From 7b59dac26b1dcce459f42f4e1db3f05fb7359edd Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Mon, 1 Jun 2026 08:16:35 +0200 Subject: [PATCH] fix: stabilize detail action focus close #1218 --- lib/focus/focusable_action_bar.dart | 126 +++- lib/focus/key_event_utils.dart | 15 - lib/screens/media_detail/action_buttons.dart | 692 ++++++++++--------- lib/screens/media_detail_screen.dart | 53 +- test/focus/key_event_utils_test.dart | 171 ++--- 5 files changed, 558 insertions(+), 499 deletions(-) diff --git a/lib/focus/focusable_action_bar.dart b/lib/focus/focusable_action_bar.dart index 7316cfe0..ecd4baa8 100644 --- a/lib/focus/focusable_action_bar.dart +++ b/lib/focus/focusable_action_bar.dart @@ -6,22 +6,49 @@ import 'focus_theme.dart'; import 'input_mode_tracker.dart'; import 'key_event_utils.dart'; +typedef FocusableActionBuilder = Widget Function(BuildContext context, FocusableActionBuildState state); + +class FocusableActionBuildState { + final FocusNode focusNode; + final bool isFocused; + final bool showFocus; + final bool isKeyboardMode; + final Duration animationDuration; + + const FocusableActionBuildState({ + required this.focusNode, + required this.isFocused, + required this.showFocus, + required this.isKeyboardMode, + required this.animationDuration, + }); +} + class FocusableAction { final IconData icon; final Color? iconColor; final double iconFill; + final String? debugLabel; + final FocusNode? focusNode; + final bool autofocus; + final String? tooltip; final VoidCallback? onPressed; final Widget? child; + final FocusableActionBuilder? builder; const FocusableAction({ this.icon = Icons.circle, this.iconColor, this.iconFill = 1.0, + this.debugLabel, + this.focusNode, + this.autofocus = false, this.tooltip, this.onPressed, this.child, + this.builder, }); } @@ -43,6 +70,12 @@ class FocusableActionBar extends StatefulWidget { /// Called when the user presses the back key while an action is focused. final VoidCallback? onBack; + /// Called when any action in the row gains or loses focus. + final ValueChanged? onFocusChange; + + final double spacing; + final MainAxisSize mainAxisSize; + const FocusableActionBar({ super.key, required this.actions, @@ -51,6 +84,9 @@ class FocusableActionBar extends StatefulWidget { this.onNavigateLeft, this.onNavigateRight, this.onBack, + this.onFocusChange, + this.spacing = 0, + this.mainAxisSize = MainAxisSize.min, }); @override @@ -59,7 +95,10 @@ class FocusableActionBar extends StatefulWidget { class FocusableActionBarState extends State { late List _focusNodes; + late List _ownsFocusNodes; + late List _focusListeners; late List _focusStates; + bool _hasAnyFocus = false; FocusNode? getFocusNode(int index) => index >= 0 && index < _focusNodes.length ? _focusNodes[index] : null; @@ -76,29 +115,58 @@ class FocusableActionBarState extends State { @override void didUpdateWidget(FocusableActionBar oldWidget) { super.didUpdateWidget(oldWidget); - if (oldWidget.actions.length != widget.actions.length) { + if (_shouldRebuildFocusNodes(oldWidget)) { _disposeNodes(); _initNodes(); } } + bool _shouldRebuildFocusNodes(FocusableActionBar oldWidget) { + if (oldWidget.actions.length != widget.actions.length) return true; + for (var i = 0; i < widget.actions.length; i++) { + if (oldWidget.actions[i].focusNode != widget.actions[i].focusNode) return true; + if (oldWidget.actions[i].debugLabel != widget.actions[i].debugLabel) return true; + } + return false; + } + void _initNodes() { - _focusNodes = List.generate(widget.actions.length, (i) => FocusNode(debugLabel: 'ActionBar[$i]')); - _focusStates = List.filled(widget.actions.length, false); + _focusNodes = List.generate( + widget.actions.length, + (i) => widget.actions[i].focusNode ?? FocusNode(debugLabel: widget.actions[i].debugLabel ?? 'ActionBar[$i]'), + ); + _ownsFocusNodes = List.generate(widget.actions.length, (i) => widget.actions[i].focusNode == null); + _focusListeners = []; + _focusStates = List.generate(widget.actions.length, (i) => _focusNodes[i].hasFocus); + _hasAnyFocus = _focusNodes.any((node) => node.hasFocus); for (var i = 0; i < _focusNodes.length; i++) { final idx = i; - _focusNodes[i].addListener(() { + void listener() { final hasFocus = _focusNodes[idx].hasFocus; if (_focusStates[idx] != hasFocus) { setState(() => _focusStates[idx] = hasFocus); } - }); + _notifyRowFocusIfChanged(); + } + + _focusListeners.add(listener); + _focusNodes[i].addListener(listener); } } + void _notifyRowFocusIfChanged() { + final hasAnyFocus = _focusNodes.any((node) => node.hasFocus); + if (_hasAnyFocus == hasAnyFocus) return; + _hasAnyFocus = hasAnyFocus; + widget.onFocusChange?.call(hasAnyFocus); + } + void _disposeNodes() { - for (final node in _focusNodes) { - node.dispose(); + for (var i = 0; i < _focusNodes.length; i++) { + _focusNodes[i].removeListener(_focusListeners[i]); + if (_ownsFocusNodes[i]) { + _focusNodes[i].dispose(); + } } } @@ -114,8 +182,13 @@ class FocusableActionBarState extends State { final duration = FocusTheme.getAnimationDuration(context); return Row( - mainAxisSize: MainAxisSize.min, - children: [for (var i = 0; i < widget.actions.length; i++) _buildButton(i, isKeyboard, duration)], + mainAxisSize: widget.mainAxisSize, + children: [ + for (var i = 0; i < widget.actions.length; i++) ...[ + if (i > 0 && widget.spacing > 0) SizedBox(width: widget.spacing), + _buildButton(i, isKeyboard, duration), + ], + ], ); } @@ -125,8 +198,19 @@ class FocusableActionBarState extends State { final showFocus = isFocused && isKeyboard; final opacity = isKeyboard && !isFocused ? 0.6 : 1.0; + final buildState = FocusableActionBuildState( + focusNode: _focusNodes[index], + isFocused: isFocused, + showFocus: showFocus, + isKeyboardMode: isKeyboard, + animationDuration: duration, + ); + final customChild = action.builder?.call(context, buildState); + return Focus( focusNode: _focusNodes[index], + autofocus: action.autofocus, + descendantsAreFocusable: false, onKeyEvent: (node, event) { if (widget.onBack != null) { final backResult = handleBackKeyAction(event, widget.onBack!); @@ -146,20 +230,22 @@ class FocusableActionBarState extends State { )(node, event); }, child: ClickableCursor( - enabled: action.onPressed != null || action.child != null, + enabled: action.onPressed != null || action.child != null || customChild != null, child: AnimatedOpacity( opacity: showFocus ? 1.0 : opacity, duration: duration, - child: Container( - decoration: FocusTheme.focusBackgroundDecoration(isFocused: showFocus, borderRadius: 20), - child: - action.child ?? - IconButton( - icon: AppIcon(action.icon, fill: action.iconFill, color: action.iconColor), - tooltip: action.tooltip, - onPressed: action.onPressed, - ), - ), + child: + customChild ?? + Container( + decoration: FocusTheme.focusBackgroundDecoration(isFocused: showFocus, borderRadius: 20), + child: + action.child ?? + IconButton( + icon: AppIcon(action.icon, fill: action.iconFill, color: action.iconColor), + tooltip: action.tooltip, + onPressed: action.onPressed, + ), + ), ), ), ); diff --git a/lib/focus/key_event_utils.dart b/lib/focus/key_event_utils.dart index 45f2dcdb..9d974972 100644 --- a/lib/focus/key_event_utils.dart +++ b/lib/focus/key_event_utils.dart @@ -187,21 +187,6 @@ FocusOnKeyEventCallback dpadKeyHandler({ }; } -/// Whether [container] has another focusable descendant beyond the currently -/// focused one in [direction]. Lets a row's key handler move between interior -/// items (true) but trap at the row's edge (false) so focus can't escape into -/// an off-screen "black hole" (#1181). Horizontal only. -/// -/// Relies on [FocusNode.traversalDescendants] being in reading (left→right) -/// order, which holds for a flat [Row]. -bool hasHorizontalNeighbor(FocusNode container, TraversalDirection direction) { - assert(direction == TraversalDirection.left || direction == TraversalDirection.right); - final items = container.traversalDescendants.toList(); - final index = items.indexWhere((node) => node.hasPrimaryFocus); - if (index < 0) return false; - return direction == TraversalDirection.right ? index < items.length - 1 : index > 0; -} - /// Navigator observer that automatically suppresses stray back KeyUp events /// after any route pop caused by a back key press. /// diff --git a/lib/screens/media_detail/action_buttons.dart b/lib/screens/media_detail/action_buttons.dart index 80ed0ce5..99c2e05f 100644 --- a/lib/screens/media_detail/action_buttons.dart +++ b/lib/screens/media_detail/action_buttons.dart @@ -68,7 +68,7 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { return null; // default for other states }); - ButtonStyle actionButtonStyle({Color? foregroundColor, EdgeInsetsGeometry? padding}) { + ButtonStyle actionButtonStyle({Color? foregroundColor, EdgeInsetsGeometry? padding, bool showFocus = false}) { if (!isKeyboardMode && !isTv) { if (padding != null) { return FilledButton.styleFrom(padding: padding); @@ -79,6 +79,7 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { foregroundColor: foregroundColor, ); } + return ButtonStyle( padding: padding != null ? WidgetStatePropertyAll(padding) : null, minimumSize: WidgetStatePropertyAll(padding == null ? Size.square(actionSize) : Size(0, actionSize)), @@ -87,14 +88,8 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { tapTargetSize: MaterialTapTargetSize.shrinkWrap, visualDensity: VisualDensity.compact, overlayColor: noOverlay, - backgroundColor: WidgetStateProperty.resolveWith((states) { - if (states.contains(WidgetState.focused)) return focusBg; - return idleBg; - }), - foregroundColor: WidgetStateProperty.resolveWith((states) { - if (states.contains(WidgetState.focused)) return focusFg; - return foregroundColor ?? tonalFg; - }), + backgroundColor: WidgetStatePropertyAll(showFocus ? focusBg : idleBg), + foregroundColor: WidgetStatePropertyAll(showFocus ? focusFg : foregroundColor ?? tonalFg), ); } @@ -106,73 +101,122 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { final gap = isTv ? 8.0 * tvScale : 12.0; - final playButton = SizedBox( - height: actionSize, - child: FilledButton( - focusNode: _playButtonFocusNode, - autofocus: isKeyboardMode, - onPressed: onPlayPressed, - style: actionButtonStyle( - padding: EdgeInsets.symmetric(horizontal: isTv ? 17 * tvScale : 16, vertical: isTv ? 9 * tvScale : 0), + Widget playButton(FocusableActionBuildState state) { + return SizedBox( + height: actionSize, + child: FilledButton( + onPressed: onPlayPressed, + style: actionButtonStyle( + showFocus: state.showFocus, + padding: EdgeInsets.symmetric(horizontal: isTv ? 17 * tvScale : 16, vertical: isTv ? 9 * tvScale : 0), + ), + child: playButtonLabel.isNotEmpty + ? Row( + mainAxisSize: MainAxisSize.min, + children: [ + playButtonIcon, + SizedBox(width: isTv ? 7 * tvScale : 8), + Text(playButtonLabel, style: playTextStyle), + ], + ) + : playButtonIcon, ), - child: playButtonLabel.isNotEmpty - ? Row( - mainAxisSize: MainAxisSize.min, - children: [ - playButtonIcon, - SizedBox(width: isTv ? 7 * tvScale : 8), - Text(playButtonLabel, style: playTextStyle), - ], - ) - : playButtonIcon, - ), - ); - - final trailerButton = primaryTrailer == null - ? null - : IconButton.filledTonal( - onPressed: onPlayTrailer, - icon: const AppIcon(Symbols.theaters_rounded, fill: 1), - tooltip: t.tooltips.playTrailer, - iconSize: isTv ? 21 * tvScale : 20, - style: actionButtonStyle(), - ); - - final shuffleButton = (metadata.isShow || metadata.isSeason) - ? IconButton.filledTonal( - onPressed: () async { - await _handleShufflePlayWithQueue(context, metadata); - }, - icon: const AppIcon(Symbols.shuffle_rounded, fill: 1), - tooltip: t.tooltips.shufflePlay, - iconSize: isTv ? 21 * tvScale : 20, - style: actionButtonStyle(), - ) - : null; - - final downloadButton = !widget.isOffline && !PlatformDetector.isAppleTV() - ? _buildDownloadButton(metadata, actionButtonStyle, tvScale) - : null; - final watchedButton = _buildWatchedToggleButton(metadata, actionButtonStyle, tvScale); - final moreActionsButton = widget.isOffline - ? null - : _buildMoreActionsButton(metadata, actionButtonStyle, tvScale, onPlayTrailer: onPlayTrailer); - - Row actionRow(List buttons) { - return Row( - children: [ - for (var i = 0; i < buttons.length; i++) ...[if (i > 0) SizedBox(width: gap), buttons[i]], - ], ); } - final allButtons = [ - playButton, - if (trailerButton != null) trailerButton, - if (shuffleButton != null) shuffleButton, - if (downloadButton != null) downloadButton, - watchedButton, - if (moreActionsButton != null) moreActionsButton, + Widget iconActionButton( + FocusableActionBuildState state, { + required Widget icon, + required VoidCallback? onPressed, + String? tooltip, + Color? foregroundColor, + }) { + return IconButton.filledTonal( + onPressed: onPressed, + icon: icon, + tooltip: tooltip, + iconSize: isTv ? 21 * tvScale : 20, + style: actionButtonStyle(foregroundColor: foregroundColor, showFocus: state.showFocus), + ); + } + + final playAction = FocusableAction( + debugLabel: 'detail_play', + focusNode: _playButtonFocusNode, + autofocus: isKeyboardMode, + onPressed: onPlayPressed, + builder: (context, state) => playButton(state), + ); + + final trailerAction = primaryTrailer == null + ? null + : FocusableAction( + debugLabel: 'detail_trailer', + onPressed: onPlayTrailer, + builder: (context, state) => iconActionButton( + state, + onPressed: onPlayTrailer, + icon: const AppIcon(Symbols.theaters_rounded, fill: 1), + tooltip: t.tooltips.playTrailer, + ), + ); + + final shuffleAction = (metadata.isShow || metadata.isSeason) + ? FocusableAction( + debugLabel: 'detail_shuffle', + onPressed: () async { + await _handleShufflePlayWithQueue(context, metadata); + }, + builder: (context, state) => iconActionButton( + state, + onPressed: () async { + await _handleShufflePlayWithQueue(context, metadata); + }, + icon: const AppIcon(Symbols.shuffle_rounded, fill: 1), + tooltip: t.tooltips.shufflePlay, + ), + ) + : null; + + final downloadAction = !widget.isOffline && !PlatformDetector.isAppleTV() + ? FocusableAction( + debugLabel: 'detail_download', + onPressed: () => unawaited(_handleDownloadButtonPressed(metadata)), + builder: (context, state) => + _buildDownloadButton(metadata, actionButtonStyle, tvScale, showFocus: state.showFocus), + ) + : null; + + final watchedAction = FocusableAction( + debugLabel: 'detail_watched', + onPressed: () => unawaited(_handleWatchedTogglePressed(metadata)), + builder: (context, state) => + _buildWatchedToggleButton(metadata, actionButtonStyle, tvScale, showFocus: state.showFocus), + ); + + void showMoreActions() => _contextMenuKey.currentState?.showContextMenu(context); + + final moreActionsAction = widget.isOffline + ? null + : FocusableAction( + debugLabel: 'detail_more', + onPressed: showMoreActions, + builder: (context, state) => _buildMoreActionsButton( + metadata, + actionButtonStyle, + tvScale, + onPlayTrailer: onPlayTrailer, + showFocus: state.showFocus, + ), + ); + + final allActions = [ + playAction, + if (trailerAction != null) trailerAction, + if (shuffleAction != null) shuffleAction, + if (downloadAction != null) downloadAction, + watchedAction, + if (moreActionsAction != null) moreActionsAction, ]; double playButtonWidthEstimate() { @@ -190,115 +234,121 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { } final estimatedPlayWidth = playButtonWidthEstimate(); - double estimatedRowWidth(List buttons) { - if (buttons.isEmpty) return 0; - return estimatedPlayWidth + (buttons.length - 1) * actionSize + (buttons.length - 1) * gap; + double estimatedRowWidth(List actions) { + if (actions.isEmpty) return 0; + return estimatedPlayWidth + (actions.length - 1) * actionSize + (actions.length - 1) * gap; } - List compactButtonsFor(double maxWidth) { + List compactActionsFor(double maxWidth) { if (widget.isOffline) { - final compact = [playButton, watchedButton]; - if (maxWidth.isFinite && estimatedRowWidth(compact) > maxWidth) return [playButton]; + final compact = [playAction, watchedAction]; + if (maxWidth.isFinite && estimatedRowWidth(compact) > maxWidth) return [playAction]; return compact; } - final medium = [ - playButton, - if (downloadButton != null) downloadButton, - watchedButton, - if (moreActionsButton != null) moreActionsButton, + final medium = [ + playAction, + if (downloadAction != null) downloadAction, + watchedAction, + if (moreActionsAction != null) moreActionsAction, ]; if (!maxWidth.isFinite || estimatedRowWidth(medium) <= maxWidth) return medium; - final compact = [playButton, watchedButton, if (moreActionsButton != null) moreActionsButton]; + final compact = [playAction, watchedAction, if (moreActionsAction != null) moreActionsAction]; if (estimatedRowWidth(compact) <= maxWidth) return compact; - return [playButton, if (moreActionsButton != null) moreActionsButton]; + return [playAction, if (moreActionsAction != null) moreActionsAction]; } - return Focus( - skipTraversal: true, - onFocusChange: (hasFocus) { - if (isTv) _setTvDetailActionRowFocus(hasFocus); + Widget actionBar(List actions) { + return FocusableActionBar( + actions: actions, + spacing: gap, + onFocusChange: isTv ? _setTvDetailActionRowFocus : null, + onNavigateUp: _focusAboveActionRow, + onNavigateDown: _focusBelowActionRow, + ); + } + + // TV screens are wide and D-pad focus should see every direct action. + // On smaller online screens, hidden actions remain available from ⋮. + if (isTv) return actionBar(allActions); + + return LayoutBuilder( + builder: (context, constraints) { + final maxWidth = constraints.maxWidth; + if (!maxWidth.isFinite || estimatedRowWidth(allActions) <= maxWidth) { + return actionBar(allActions); + } + return actionBar(compactActionsFor(maxWidth)); }, - onKeyEvent: _handlePlayButtonKeyEvent, - // TV screens are wide and D-pad focus should see every direct action. - // On smaller online screens, hidden actions remain available from ⋮. - child: isTv - ? actionRow(allButtons) - : LayoutBuilder( - builder: (context, constraints) { - final maxWidth = constraints.maxWidth; - if (!maxWidth.isFinite || estimatedRowWidth(allButtons) <= maxWidth) { - return actionRow(allButtons); - } - return actionRow(compactButtonsFor(maxWidth)); - }, - ), ); } + Future _handleWatchedTogglePressed(MediaItem metadata) async { + try { + final isWatched = metadata.isWatched; + if (widget.isOffline) { + // Offline mode: queue action for later sync + final offlineWatch = context.read(); + if (isWatched) { + await offlineWatch.markAsUnwatched(serverId: metadata.serverId!, itemId: metadata.id); + } else { + await offlineWatch.markAsWatched(serverId: metadata.serverId!, itemId: metadata.id); + } + if (mounted) { + showAppSnackBar(context, isWatched ? t.messages.markedAsUnwatchedOffline : t.messages.markedAsWatchedOffline); + } + } else { + // Online mode: dispatch via the right backend's neutral method so + // Jellyfin items hit /UserPlayedItems and Plex items hit /:/scrobble. + final serverId = metadata.serverId; + if (serverId == null) return; + final client = context.tryGetMediaClientForServer(serverId); + if (client == null) return; + + if (isWatched) { + await client.markUnwatched(metadata); + unawaited(TrackerCoordinator.instance.markUnwatched(metadata, client)); + } else { + await client.markWatched(metadata); + unawaited(TrackerCoordinator.instance.markWatched(metadata, client)); + } + if (mounted) { + _watchStateChanged = true; + showSuccessSnackBar(context, isWatched ? t.messages.markedAsUnwatched : t.messages.markedAsWatched); + } + } + } catch (e) { + if (mounted) { + showErrorSnackBar(context, t.messages.errorLoading(error: e.toString())); + } + } + } + Widget _buildWatchedToggleButton( MediaItem metadata, - ButtonStyle Function({Color? foregroundColor, EdgeInsetsGeometry? padding}) actionButtonStyle, - double tvScale, - ) { + ButtonStyle Function({Color? foregroundColor, EdgeInsetsGeometry? padding, required bool showFocus}) + actionButtonStyle, + double tvScale, { + bool showFocus = false, + }) { return IconButton.filledTonal( - onPressed: () async { - try { - final isWatched = metadata.isWatched; - if (widget.isOffline) { - // Offline mode: queue action for later sync - final offlineWatch = context.read(); - if (isWatched) { - await offlineWatch.markAsUnwatched(serverId: metadata.serverId!, itemId: metadata.id); - } else { - await offlineWatch.markAsWatched(serverId: metadata.serverId!, itemId: metadata.id); - } - if (mounted) { - showAppSnackBar( - context, - isWatched ? t.messages.markedAsUnwatchedOffline : t.messages.markedAsWatchedOffline, - ); - } - } else { - // Online mode: dispatch via the right backend's neutral method so - // Jellyfin items hit /UserPlayedItems and Plex items hit /:/scrobble. - final serverId = metadata.serverId; - if (serverId == null) return; - final client = context.tryGetMediaClientForServer(serverId); - if (client == null) return; - - if (isWatched) { - await client.markUnwatched(metadata); - unawaited(TrackerCoordinator.instance.markUnwatched(metadata, client)); - } else { - await client.markWatched(metadata); - unawaited(TrackerCoordinator.instance.markWatched(metadata, client)); - } - if (mounted) { - _watchStateChanged = true; - showSuccessSnackBar(context, isWatched ? t.messages.markedAsUnwatched : t.messages.markedAsWatched); - } - } - } catch (e) { - if (mounted) { - showErrorSnackBar(context, t.messages.errorLoading(error: e.toString())); - } - } - }, + onPressed: () => unawaited(_handleWatchedTogglePressed(metadata)), icon: AppIcon(metadata.isWatched ? Symbols.remove_done_rounded : Symbols.check_rounded, fill: 1), tooltip: metadata.isWatched ? t.tooltips.markAsUnwatched : t.tooltips.markAsWatched, iconSize: PlatformDetector.isTV() ? 21 * tvScale : 20, - style: actionButtonStyle(), + style: actionButtonStyle(showFocus: showFocus), ); } Widget _buildMoreActionsButton( MediaItem metadata, - ButtonStyle Function({Color? foregroundColor, EdgeInsetsGeometry? padding}) actionButtonStyle, + ButtonStyle Function({Color? foregroundColor, EdgeInsetsGeometry? padding, required bool showFocus}) + actionButtonStyle, double tvScale, { VoidCallback? onPlayTrailer, + bool showFocus = false, }) { return MediaContextMenu( key: _contextMenuKey, @@ -316,17 +366,166 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { }, icon: const AppIcon(Symbols.more_vert_rounded, fill: 1), iconSize: PlatformDetector.isTV() ? 21 * tvScale : 20, - style: actionButtonStyle(), + style: actionButtonStyle(showFocus: showFocus), ), ), ); } + Future _handleDownloadButtonPressed(MediaItem metadata) async { + final downloadProvider = context.read(); + final globalKey = metadata.globalKey; + final ruleKey = _syncRuleKeyForMetadata(context, downloadProvider, metadata); + final progress = downloadProvider.getProgress(globalKey); + + if (downloadProvider.isQueueing(globalKey) || + progress?.status == DownloadStatus.queued || + progress?.status == DownloadStatus.downloading) { + return; + } + + if (progress?.status == DownloadStatus.paused) { + final client = _getMediaClientForMetadata(context); + if (client == null) return; + await downloadProvider.resumeDownload(globalKey, client); + if (mounted) showAppSnackBar(context, 'Download resumed'); + return; + } + + if (progress?.status == DownloadStatus.failed) { + final client = _getMediaClientForMetadata(context); + if (client == null) return; + + final versionConfig = await _resolveDownloadVersion(context, metadata, client); + if (versionConfig == null || !mounted) return; + + await downloadProvider.deleteDownload(globalKey); + try { + await downloadProvider.queueDownload(metadata, client, versionConfig: versionConfig); + if (mounted) showSuccessSnackBar(context, t.downloads.downloadQueued); + } on CellularDownloadBlockedException { + if (mounted) showErrorSnackBar(context, t.settings.cellularDownloadBlocked); + } + return; + } + + if (progress?.status == DownloadStatus.cancelled) { + final retry = await showConfirmDialog( + context, + title: 'Cancelled Download', + message: 'This download was cancelled. What would you like to do?', + cancelText: t.common.delete, + confirmText: 'Retry', + ); + + if (!retry && mounted) { + await downloadProvider.deleteDownload(globalKey); + if (mounted) showSuccessSnackBar(context, t.downloads.downloadDeleted); + } else if (retry && mounted) { + final client = _getMediaClientForMetadata(context); + if (client == null) return; + + final versionConfig = await _resolveDownloadVersion(context, metadata, client); + if (versionConfig == null || !mounted) return; + + await downloadProvider.deleteDownload(globalKey); + try { + await downloadProvider.queueDownload(metadata, client, versionConfig: versionConfig); + if (mounted) showSuccessSnackBar(context, t.downloads.downloadQueued); + } on CellularDownloadBlockedException { + if (mounted) showErrorSnackBar(context, t.settings.cellularDownloadBlocked); + } + } + return; + } + + if (progress?.status == DownloadStatus.partial) { + if (downloadProvider.hasSyncRule(ruleKey)) { + await _showSyncRuleActions(context, downloadProvider, metadata, ruleKey: ruleKey, downloadGlobalKey: globalKey); + return; + } + + final client = _getMediaClientForMetadata(context); + if (client == null) return; + + final versionConfig = await _resolveDownloadVersion(context, metadata, client); + if (versionConfig == null || !mounted) return; + + final count = await downloadProvider.queueMissingEpisodes(metadata, client, versionConfig: versionConfig); + if (mounted) { + final message = count > 0 ? t.downloads.episodesQueued(count: count) : 'All episodes already downloaded'; + showAppSnackBar(context, message); + } + return; + } + + if (downloadProvider.isDownloaded(globalKey)) { + if (downloadProvider.hasSyncRule(ruleKey)) { + await _showSyncRuleActions(context, downloadProvider, metadata, ruleKey: ruleKey, downloadGlobalKey: globalKey); + return; + } + + final canDownloadMore = metadata.isShow || metadata.isSeason; + Future confirmAndDelete() async { + final confirmed = await showDeleteConfirmation( + context, + title: t.downloads.deleteDownload, + message: t.downloads.deleteConfirm(title: metadata.displayTitle), + ); + if (confirmed && mounted) { + await downloadProvider.deleteDownload(globalKey); + if (mounted) showSuccessSnackBar(context, t.downloads.downloadDeleted); + } + } + + if (!canDownloadMore) { + await confirmAndDelete(); + return; + } + + final client = _getMediaClientForMetadata(context); + if (client == null) return; + try { + final result = await showDownloadOptionsAndQueue( + context, + metadata: metadata, + client: client, + downloadProvider: downloadProvider, + onDelete: confirmAndDelete, + ); + if (result == null || !mounted) return; + showSuccessSnackBar(context, result.toSnackBarMessage()); + } on CellularDownloadBlockedException { + if (mounted) showErrorSnackBar(context, t.settings.cellularDownloadBlocked); + } + return; + } + + final client = _getMediaClientForMetadata(context); + if (client == null) return; + + try { + final result = await showDownloadOptionsAndQueue( + context, + metadata: metadata, + client: client, + downloadProvider: downloadProvider, + ); + if (result == null || !mounted) return; + + showSuccessSnackBar(context, result.toSnackBarMessage()); + } on CellularDownloadBlockedException { + if (mounted) showErrorSnackBar(context, t.settings.cellularDownloadBlocked); + } + } + Widget _buildDownloadButton( MediaItem metadata, - ButtonStyle Function({Color? foregroundColor, EdgeInsetsGeometry? padding}) actionButtonStyle, - double tvScale, - ) { + ButtonStyle Function({Color? foregroundColor, EdgeInsetsGeometry? padding, required bool showFocus}) + actionButtonStyle, + double tvScale, { + bool showFocus = false, + }) { return Consumer( builder: (context, downloadProvider, _) { final iconSize = PlatformDetector.isTV() ? 21.0 * tvScale : 20.0; @@ -346,7 +545,7 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { onPressed: null, icon: LoadingIndicatorBox(size: iconSize), iconSize: iconSize, - style: actionButtonStyle(), + style: actionButtonStyle(showFocus: showFocus), ); } @@ -362,7 +561,7 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { tooltip: tooltip, icon: const AppIcon(Symbols.schedule_rounded, fill: 1), iconSize: iconSize, - style: actionButtonStyle(), + style: actionButtonStyle(showFocus: showFocus), ); } @@ -379,100 +578,40 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { tooltip: tooltip, icon: _buildRadialProgress(progress?.progressPercent), iconSize: iconSize, - style: actionButtonStyle(), + style: actionButtonStyle(showFocus: showFocus), ); } // State 4: Paused (can resume) if (progress?.status == DownloadStatus.paused) { return IconButton.filledTonal( - onPressed: () async { - final client = _getMediaClientForMetadata(context); - if (client == null) return; - await downloadProvider.resumeDownload(globalKey, client); - if (context.mounted) { - showAppSnackBar(context, 'Download resumed'); - } - }, + onPressed: () => unawaited(_handleDownloadButtonPressed(metadata)), icon: const AppIcon(Symbols.pause_circle_outline_rounded, fill: 1), tooltip: 'Resume download', iconSize: iconSize, - style: actionButtonStyle(foregroundColor: Colors.amber), + style: actionButtonStyle(foregroundColor: Colors.amber, showFocus: showFocus), ); } // State 5: Failed (can retry) if (progress?.status == DownloadStatus.failed) { return IconButton.filledTonal( - onPressed: () async { - final client = _getMediaClientForMetadata(context); - if (client == null) return; - - final versionConfig = await _resolveDownloadVersion(context, metadata, client); - if (versionConfig == null || !context.mounted) return; - - await downloadProvider.deleteDownload(globalKey); - try { - await downloadProvider.queueDownload(metadata, client, versionConfig: versionConfig); - - if (context.mounted) { - showSuccessSnackBar(context, t.downloads.downloadQueued); - } - } on CellularDownloadBlockedException { - if (context.mounted) { - showErrorSnackBar(context, t.settings.cellularDownloadBlocked); - } - } - }, + onPressed: () => unawaited(_handleDownloadButtonPressed(metadata)), icon: const AppIcon(Symbols.error_outline_rounded, fill: 1), tooltip: 'Retry download', iconSize: iconSize, - style: actionButtonStyle(foregroundColor: Colors.red), + style: actionButtonStyle(foregroundColor: Colors.red, showFocus: showFocus), ); } // State 6: Cancelled (can delete or retry) if (progress?.status == DownloadStatus.cancelled) { return IconButton.filledTonal( - onPressed: () async { - // Show options: Delete or Retry - final retry = await showConfirmDialog( - context, - title: 'Cancelled Download', - message: 'This download was cancelled. What would you like to do?', - cancelText: t.common.delete, - confirmText: 'Retry', - ); - - if (!retry && context.mounted) { - await downloadProvider.deleteDownload(globalKey); - if (context.mounted) { - showSuccessSnackBar(context, t.downloads.downloadDeleted); - } - } else if (retry && context.mounted) { - final client = _getMediaClientForMetadata(context); - if (client == null) return; - - final versionConfig = await _resolveDownloadVersion(context, metadata, client); - if (versionConfig == null || !context.mounted) return; - - await downloadProvider.deleteDownload(globalKey); - try { - await downloadProvider.queueDownload(metadata, client, versionConfig: versionConfig); - if (context.mounted) { - showSuccessSnackBar(context, t.downloads.downloadQueued); - } - } on CellularDownloadBlockedException { - if (context.mounted) { - showErrorSnackBar(context, t.settings.cellularDownloadBlocked); - } - } - } - }, + onPressed: () => unawaited(_handleDownloadButtonPressed(metadata)), icon: const AppIcon(Symbols.cancel_rounded, fill: 1), tooltip: 'Cancelled download', iconSize: iconSize, - style: actionButtonStyle(foregroundColor: Colors.grey), + style: actionButtonStyle(foregroundColor: Colors.grey, showFocus: showFocus), ); } @@ -490,17 +629,11 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { : t.downloads.keepSynced; return IconButton.filledTonal( - onPressed: () => _showSyncRuleActions( - context, - downloadProvider, - metadata, - ruleKey: ruleKey, - downloadGlobalKey: globalKey, - ), + onPressed: () => unawaited(_handleDownloadButtonPressed(metadata)), tooltip: tooltip, icon: AppIcon(isEnabled ? Symbols.sync_rounded : Symbols.sync_disabled_rounded, fill: 1), iconSize: iconSize, - style: actionButtonStyle(foregroundColor: isEnabled ? Colors.teal : Colors.grey), + style: actionButtonStyle(foregroundColor: isEnabled ? Colors.teal : Colors.grey, showFocus: showFocus), ); } @@ -509,26 +642,11 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { : 'Partially downloaded - Click to complete'; return IconButton.filledTonal( - onPressed: () async { - final client = _getMediaClientForMetadata(context); - if (client == null) return; - - final versionConfig = await _resolveDownloadVersion(context, metadata, client); - if (versionConfig == null || !context.mounted) return; - - final count = await downloadProvider.queueMissingEpisodes(metadata, client, versionConfig: versionConfig); - - if (context.mounted) { - final message = count > 0 - ? t.downloads.episodesQueued(count: count) - : 'All episodes already downloaded'; - showAppSnackBar(context, message); - } - }, + onPressed: () => unawaited(_handleDownloadButtonPressed(metadata)), tooltip: tooltip, icon: const AppIcon(Symbols.downloading_rounded, fill: 1), iconSize: iconSize, - style: actionButtonStyle(foregroundColor: Colors.orange), + style: actionButtonStyle(foregroundColor: Colors.orange, showFocus: showFocus), ); } @@ -541,97 +659,33 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { final syncRule = downloadProvider.getSyncRule(ruleKey); final isEnabled = syncRule?.enabled ?? true; return IconButton.filledTonal( - onPressed: () => _showSyncRuleActions( - context, - downloadProvider, - metadata, - ruleKey: ruleKey, - downloadGlobalKey: globalKey, - ), + onPressed: () => unawaited(_handleDownloadButtonPressed(metadata)), icon: AppIcon(isEnabled ? Symbols.sync_rounded : Symbols.sync_disabled_rounded, fill: 1), tooltip: t.downloads.keepNUnwatched(count: syncRule?.episodeCount.toString() ?? '?'), iconSize: iconSize, - style: actionButtonStyle(foregroundColor: isEnabled ? Colors.teal : Colors.grey), + style: actionButtonStyle(foregroundColor: isEnabled ? Colors.teal : Colors.grey, showFocus: showFocus), ); } // Shows/seasons may have more episodes to fetch; movies/episodes don't. final canDownloadMore = metadata.isShow || metadata.isSeason; - Future confirmAndDelete() async { - final confirmed = await showDeleteConfirmation( - context, - title: t.downloads.deleteDownload, - message: t.downloads.deleteConfirm(title: metadata.displayTitle), - ); - if (confirmed && context.mounted) { - await downloadProvider.deleteDownload(globalKey); - if (context.mounted) { - showSuccessSnackBar(context, t.downloads.downloadDeleted); - } - } - } - return IconButton.filledTonal( - onPressed: () async { - // Movies/episodes: nothing more to download, so delete directly. - if (!canDownloadMore) { - await confirmAndDelete(); - return; - } - // Shows/seasons: reopen the download options menu so the user can - // grab more episodes (or switch to sync), with delete as a row. - final client = _getMediaClientForMetadata(context); - if (client == null) return; - try { - final result = await showDownloadOptionsAndQueue( - context, - metadata: metadata, - client: client, - downloadProvider: downloadProvider, - onDelete: confirmAndDelete, - ); - if (result == null || !context.mounted) return; - showSuccessSnackBar(context, result.toSnackBarMessage()); - } on CellularDownloadBlockedException { - if (context.mounted) { - showErrorSnackBar(context, t.settings.cellularDownloadBlocked); - } - } - }, + onPressed: () => unawaited(_handleDownloadButtonPressed(metadata)), icon: const AppIcon(Symbols.download_rounded, fill: 1), tooltip: canDownloadMore ? t.downloads.manage : t.downloads.deleteDownload, iconSize: iconSize, - style: actionButtonStyle(foregroundColor: Colors.orange), + style: actionButtonStyle(foregroundColor: Colors.orange, showFocus: showFocus), ); } // State 9: Not downloaded (default - can download) return IconButton.filledTonal( - onPressed: () async { - final client = _getMediaClientForMetadata(context); - if (client == null) return; - - try { - final result = await showDownloadOptionsAndQueue( - context, - metadata: metadata, - client: client, - downloadProvider: downloadProvider, - ); - if (result == null || !context.mounted) return; - - showSuccessSnackBar(context, result.toSnackBarMessage()); - } on CellularDownloadBlockedException { - if (context.mounted) { - showErrorSnackBar(context, t.settings.cellularDownloadBlocked); - } - } - }, + onPressed: () => unawaited(_handleDownloadButtonPressed(metadata)), icon: const AppIcon(Symbols.download_rounded, fill: 1), tooltip: t.downloads.downloadNow, iconSize: iconSize, - style: actionButtonStyle(), + style: actionButtonStyle(showFocus: showFocus), ); }, ); diff --git a/lib/screens/media_detail_screen.dart b/lib/screens/media_detail_screen.dart index 13ed2d00..fbe47b3a 100644 --- a/lib/screens/media_detail_screen.dart +++ b/lib/screens/media_detail_screen.dart @@ -16,6 +16,7 @@ import '../widgets/collapsible_text.dart'; import '../widgets/rating_bottom_sheet.dart'; import '../focus/dpad_navigator.dart'; +import '../focus/focusable_action_bar.dart'; import '../focus/focusable_wrapper.dart'; import '../focus/key_event_utils.dart'; import '../focus/input_mode_tracker.dart'; @@ -1996,79 +1997,57 @@ class _MediaDetailScreenState extends State scrollContextToCenter(key.currentContext); } - /// Intercept DOWN from the play button row to focus the first available section - KeyEventResult _handlePlayButtonKeyEvent(FocusNode node, KeyEvent event) { - final key = event.logicalKey; - if (!event.isActionable) return KeyEventResult.ignored; - final isTv = PlatformDetector.isTV(); - - if (isTv && key.isUpKey) { - return KeyEventResult.handled; - } - - // UP: focus the rating chip if available - if (key.isUpKey) { - if (!widget.isOffline) { - _ratingChipFocusNode.requestFocus(); - return KeyEventResult.handled; - } - return KeyEventResult.handled; - } - - // LEFT/RIGHT: let the framework move between buttons in the row, but trap - // at the row's edges so focus can't fall off into a black hole (#1181). - if (key.isLeftKey || key.isRightKey) { - final dir = key.isRightKey ? TraversalDirection.right : TraversalDirection.left; - return hasHorizontalNeighbor(node, dir) ? KeyEventResult.ignored : KeyEventResult.handled; - } - - if (!key.isDownKey) return KeyEventResult.ignored; + /// Focus the first available section above the primary action row. + void _focusAboveActionRow() { + if (PlatformDetector.isTV()) return; + if (!widget.isOffline) _ratingChipFocusNode.requestFocus(); + } + /// Focus the first available section below the primary action row. + void _focusBelowActionRow() { final metadata = _fullMetadata ?? _metadata; - if (isTv) { + if (PlatformDetector.isTV()) { _tvDetailRailKey.currentState?.requestFocus(); - return KeyEventResult.handled; + return; } // DOWN order: overview → seasons → cast → extras if (!PlatformDetector.isTV() && metadata.summary != null && metadata.summary!.isNotEmpty) { _overviewFocusNode.requestFocus(); _scrollSectionIntoView(_overviewSectionKey); - return KeyEventResult.handled; + return; } if (metadata.isShow && !_showEpisodesDirectly && _seasons.isNotEmpty && _seasonTabFocusNodes.isNotEmpty) { // Focus the selected season tab chip _seasonTabFocusNodes[_selectedSeasonIndex].requestFocus(); _scrollSectionIntoView(_seasonsSectionKey); - return KeyEventResult.handled; + return; } if (_episodes.isNotEmpty) { _firstEpisodeFocusNode.requestFocus(); _scrollSectionIntoView(_seasonsSectionKey); - return KeyEventResult.handled; + return; } if (metadata.roles != null && metadata.roles!.isNotEmpty) { _castFocusNode.requestFocus(); _scrollSectionIntoView(_castSectionKey); - return KeyEventResult.handled; + return; } if (_extras != null && _extras!.isNotEmpty) { _extrasFocusNode.requestFocus(); _scrollSectionIntoView(_extrasSectionKey); - return KeyEventResult.handled; + return; } if (_relatedHubs.isNotEmpty) { _relatedHubKeys.first.currentState?.requestFocusFromMemory(); - return KeyEventResult.handled; + return; } - - return KeyEventResult.handled; // consume to prevent unwanted traversal } /// Get the responsive card width used by seasons/extras/cast rows. diff --git a/test/focus/key_event_utils_test.dart b/test/focus/key_event_utils_test.dart index 2011f5fd..0d410527 100644 --- a/test/focus/key_event_utils_test.dart +++ b/test/focus/key_event_utils_test.dart @@ -43,114 +43,6 @@ void main() { expect(backs, 1); }); - group('hasHorizontalNeighbor', () { - testWidgets('reports interior vs. edge neighbors for a flat button row', (tester) async { - final container = FocusNode(debugLabel: 'row', skipTraversal: true); - final play = FocusNode(debugLabel: 'play'); - final download = FocusNode(debugLabel: 'download'); - final more = FocusNode(debugLabel: 'more'); - for (final node in [container, play, download, more]) { - addTearDown(node.dispose); - } - - await tester.pumpWidget( - MaterialApp( - home: Scaffold( - body: Focus( - focusNode: container, - skipTraversal: true, - child: Row( - mainAxisSize: MainAxisSize.min, - children: [ - FilledButton(focusNode: play, onPressed: () {}, child: const Text('Play')), - IconButton(focusNode: download, onPressed: () {}, icon: const Icon(Icons.download)), - IconButton(focusNode: more, onPressed: () {}, icon: const Icon(Icons.more_vert)), - ], - ), - ), - ), - ), - ); - await tester.pump(); - - // The ordinal helper assumes one traversal node per button, in reading - // order. Assert it so a framework change that breaks the assumption fails - // loudly here rather than silently re-opening the black hole. - expect(container.traversalDescendants.length, 3); - - play.requestFocus(); - await tester.pump(); - expect(hasHorizontalNeighbor(container, TraversalDirection.left), isFalse); - expect(hasHorizontalNeighbor(container, TraversalDirection.right), isTrue); - - download.requestFocus(); - await tester.pump(); - expect(hasHorizontalNeighbor(container, TraversalDirection.left), isTrue); - expect(hasHorizontalNeighbor(container, TraversalDirection.right), isTrue); - - more.requestFocus(); - await tester.pump(); - expect(hasHorizontalNeighbor(container, TraversalDirection.right), isFalse); - expect(hasHorizontalNeighbor(container, TraversalDirection.left), isTrue); - }); - - testWidgets('single-item row has no horizontal neighbor either way (rating-chip case)', (tester) async { - final container = FocusNode(debugLabel: 'row', skipTraversal: true); - final only = FocusNode(debugLabel: 'chip'); - addTearDown(container.dispose); - addTearDown(only.dispose); - - await tester.pumpWidget( - MaterialApp( - home: Scaffold( - body: Focus( - focusNode: container, - skipTraversal: true, - child: Focus(focusNode: only, child: const SizedBox(width: 50, height: 50)), - ), - ), - ), - ); - await tester.pump(); - only.requestFocus(); - await tester.pump(); - - expect(hasHorizontalNeighbor(container, TraversalDirection.left), isFalse); - expect(hasHorizontalNeighbor(container, TraversalDirection.right), isFalse); - }); - - testWidgets('returns false when focus is outside the container', (tester) async { - final container = FocusNode(debugLabel: 'row'); - final inside = FocusNode(debugLabel: 'inside'); - final outside = FocusNode(debugLabel: 'outside'); - for (final node in [container, inside, outside]) { - addTearDown(node.dispose); - } - - await tester.pumpWidget( - MaterialApp( - home: Scaffold( - body: Row( - children: [ - Focus( - focusNode: container, - child: Focus(focusNode: inside, child: const SizedBox(width: 40, height: 40)), - ), - Focus(focusNode: outside, child: const SizedBox(width: 40, height: 40)), - ], - ), - ), - ), - ); - await tester.pump(); - outside.requestFocus(); - await tester.pump(); - - expect(hasHorizontalNeighbor(container, TraversalDirection.left), isFalse); - expect(hasHorizontalNeighbor(container, TraversalDirection.right), isFalse); - }); - }); - group('dpadKeyHandler trapHorizontalEdges', () { testWidgets('consumes edge LEFT/RIGHT so focus cannot escape the group', (tester) async { final trapped = FocusNode(debugLabel: 'trapped'); @@ -301,5 +193,68 @@ void main() { expect(navigatedLeft, isTrue); expect(FocusManager.instance.primaryFocus?.debugLabel, 'left-target'); }); + + testWidgets('moves through detail actions when trailer is inserted before shuffle', (tester) async { + final play = FocusNode(debugLabel: 'detail_play'); + final outside = FocusNode(debugLabel: 'outside'); + addTearDown(play.dispose); + addTearDown(outside.dispose); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Row( + children: [ + FocusableActionBar( + actions: [ + FocusableAction( + debugLabel: 'unused_play_label', + focusNode: play, + icon: Icons.play_arrow, + onPressed: () {}, + ), + FocusableAction(debugLabel: 'detail_trailer', icon: Icons.theaters, onPressed: () {}), + FocusableAction(debugLabel: 'detail_shuffle', icon: Icons.shuffle, onPressed: () {}), + FocusableAction(debugLabel: 'detail_download', icon: Icons.download, onPressed: () {}), + FocusableAction(debugLabel: 'detail_watched', icon: Icons.check, onPressed: () {}), + FocusableAction(debugLabel: 'detail_more', icon: Icons.more_vert, onPressed: () {}), + ], + ), + Focus(focusNode: outside, child: const SizedBox(width: 50, height: 50)), + ], + ), + ), + ), + ); + await tester.pump(); + + play.requestFocus(); + await tester.pump(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'detail_play'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'detail_trailer'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'detail_shuffle'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'detail_download'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'detail_watched'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'detail_more'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'detail_more'); + }); }); }