From d2b5039bb3f6e02e8fb37740e336c30e737c920a Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Mon, 31 Aug 2026 20:41:33 +0200 Subject: [PATCH] fix(explore): stop d-pad on detail prose instead of skipping to the next button On TV, pressing down on the Explore detail screen jumped straight from the action bar to the next focusable button, scrolling the description and background prose past unread. The overview and background sections are now collapsible focus stops wired into the d-pad chain in both directions, so a viewer can stop, read, and expand them before moving on. close #2199 --- lib/screens/catalog_item_detail_screen.dart | 95 +++++++++++++-- .../catalog_item_detail_screen_test.dart | 110 +++++++++++++++++- 2 files changed, 191 insertions(+), 14 deletions(-) diff --git a/lib/screens/catalog_item_detail_screen.dart b/lib/screens/catalog_item_detail_screen.dart index db8b30880..034129517 100644 --- a/lib/screens/catalog_item_detail_screen.dart +++ b/lib/screens/catalog_item_detail_screen.dart @@ -45,6 +45,7 @@ import '../widgets/app_bar_back_button.dart'; import '../widgets/app_icon.dart'; import '../widgets/backend_badge.dart'; import '../widgets/cast_member_strip.dart'; +import '../widgets/collapsible_text.dart'; import '../widgets/focusable_list_tile.dart'; import '../widgets/hub_section.dart'; import '../widgets/optimized_media_image.dart'; @@ -75,6 +76,8 @@ class _CatalogItemDetailScreenState extends State { final _hubFocusMemory = HubFocusMemory(); final ScrollController _scrollController = ScrollController(); final _spoilerTagFocusNode = FocusNode(debugLabel: 'catalog_spoiler_tags'); + final _overviewFocusNode = FocusNode(debugLabel: 'catalog_overview'); + final _backgroundFocusNode = FocusNode(debugLabel: 'catalog_background'); List _linkFocusNodes = const []; List _relationFocusNodes = const []; List _orderedTags = const []; @@ -144,6 +147,8 @@ class _CatalogItemDetailScreenState extends State { void dispose() { _backButtonFocusNode.dispose(); _spoilerTagFocusNode.dispose(); + _overviewFocusNode.dispose(); + _backgroundFocusNode.dispose(); for (final node in _linkFocusNodes) { node.dispose(); } @@ -308,6 +313,13 @@ class _CatalogItemDetailScreenState extends State { bool get _hasDetailActions => _hasSpoilerReveal || _linkFocusNodes.isNotEmpty; + bool get _hasOverview => _item.overview?.trim().isNotEmpty ?? false; + + bool get _hasBackground => _item.background?.trim().isNotEmpty ?? false; + + /// Prose sections rendered as dpad stops (overview, MAL background prose). + bool get _hasTextStops => _hasOverview || _hasBackground; + bool get _hasCast => _cast?.isNotEmpty ?? false; bool get _hasRelations => _relationEntries.isNotEmpty; @@ -343,6 +355,18 @@ class _CatalogItemDetailScreenState extends State { _revealFocusNode(node); } + /// Prose stops are top-aligned: an expanded overview should read from its + /// first line, not start 30% down with its opening already scrolled past. + void _requestOverviewFocus() { + _overviewFocusNode.requestFocus(); + _revealFocusNode(_overviewFocusNode, alignment: 0.1); + } + + void _requestBackgroundFocus() { + _backgroundFocusNode.requestFocus(); + _revealFocusNode(_backgroundFocusNode, alignment: 0.1); + } + void _requestFirstDetailActionFocus() { if (_hasSpoilerReveal) { _spoilerTagFocusNode.requestFocus(); @@ -424,6 +448,22 @@ class _CatalogItemDetailScreenState extends State { } void _focusSectionBelowActions() { + if (_hasOverview) { + _requestOverviewFocus(); + } else { + _focusSectionBelowOverview(); + } + } + + void _focusSectionBelowOverview() { + if (_hasBackground) { + _requestBackgroundFocus(); + } else { + _focusSectionBelowTextStops(); + } + } + + void _focusSectionBelowTextStops() { if (_hasDetailActions) { _requestFirstDetailActionFocus(); } else { @@ -431,11 +471,27 @@ class _CatalogItemDetailScreenState extends State { } } + void _focusSectionAboveBackground() { + if (_hasOverview) { + _requestOverviewFocus(); + } else { + _requestActionBarFocus(); + } + } + + void _focusSectionAboveDetailActions() { + if (_hasBackground) { + _requestBackgroundFocus(); + } else { + _focusSectionAboveBackground(); + } + } + void _focusSectionAboveLibraryMatches() { if (_hasDetailActions) { _requestLastDetailActionFocus(); } else { - _requestActionBarFocus(); + _focusSectionAboveDetailActions(); } } @@ -445,7 +501,7 @@ class _CatalogItemDetailScreenState extends State { } else if (_hasDetailActions) { _requestLastDetailActionFocus(); } else { - _requestActionBarFocus(); + _focusSectionAboveDetailActions(); } } @@ -471,7 +527,7 @@ class _CatalogItemDetailScreenState extends State { if (key.isUpKey) { if (index > 0) { _requestLibraryMatchFocus(index - 1); - } else if (_hasDetailActions || _hasActions) { + } else if (_hasDetailActions || _hasTextStops || _hasActions) { _focusSectionAboveLibraryMatches(); } else { return KeyEventResult.ignored; @@ -943,7 +999,7 @@ class _CatalogItemDetailScreenState extends State { FocusableButton( focusNode: _spoilerTagFocusNode, onPressed: _revealSpoilerTags, - onNavigateUp: _hasActions ? _requestActionBarFocus : null, + onNavigateUp: _hasActions || _hasTextStops ? _focusSectionAboveDetailActions : null, onNavigateDown: _linkFocusNodes.isNotEmpty ? () => _requestLinkFocus(0) : _focusSectionBelowDetailActions, child: OutlinedButton.icon( onPressed: _revealSpoilerTags, @@ -963,7 +1019,7 @@ class _CatalogItemDetailScreenState extends State { _spoilerTagFocusNode.requestFocus(); _revealFocusNode(_spoilerTagFocusNode); } else { - _requestActionBarFocus(); + _focusSectionAboveDetailActions(); } } @@ -1008,13 +1064,21 @@ class _CatalogItemDetailScreenState extends State { ); } - Widget _buildBackgroundSection(ThemeData theme, String background) { + Widget _buildBackgroundSection(ThemeData theme, String background, {required bool isMobile}) { return Column( crossAxisAlignment: .start, children: [ Text(t.explore.detail.background, style: theme.textTheme.titleMedium), const SizedBox(height: 8), - Text(background, style: theme.textTheme.bodyLarge), + CollapsibleText( + text: background, + maxLines: isMobile ? 6 : 4, + style: theme.textTheme.bodyLarge, + focusNode: _backgroundFocusNode, + skipTraversal: false, + onNavigateUp: _hasOverview || _hasActions ? _focusSectionAboveBackground : null, + onNavigateDown: _focusSectionBelowTextStops, + ), ], ); } @@ -1039,7 +1103,9 @@ class _CatalogItemDetailScreenState extends State { members: [ for (final member in cast) (name: member.name, secondary: member.secondary, imagePath: member.imageUrl), ], - onNavigateUp: _hasLibraryMatches || _hasDetailActions || _hasActions ? _focusSectionAboveCast : null, + onNavigateUp: _hasLibraryMatches || _hasDetailActions || _hasTextStops || _hasActions + ? _focusSectionAboveCast + : null, onNavigateDown: _hasSectionsBelowCast ? _focusSectionBelowCast : null, debugLabel: 'catalog_cast_row', ), @@ -1196,6 +1262,7 @@ class _CatalogItemDetailScreenState extends State { // the 320px slot height. final backdropUrl = item.backdropFor((MediaQuery.sizeOf(context).width * artworkDpr).ceil()); final posterUrl = item.posterFor((140 * artworkDpr).ceil()); + final isMobile = PlatformDetector.isMobile(context); final viewInsets = MediaQuery.paddingOf(context); final blockSystemBack = PlatformDetector.isTV() || InputModeTracker.shouldBlockSystemBack(context); @@ -1346,11 +1413,19 @@ class _CatalogItemDetailScreenState extends State { if (_buildStatsChips() case final Widget chips) ...[const SizedBox(height: 20), chips], if (item.overview?.trim() case final overview? when overview.isNotEmpty) ...[ const SizedBox(height: 24), - Text(overview, style: theme.textTheme.bodyLarge), + CollapsibleText( + text: overview, + maxLines: isMobile ? 6 : 4, + style: theme.textTheme.bodyLarge, + focusNode: _overviewFocusNode, + skipTraversal: false, + onNavigateUp: _hasActions ? _requestActionBarFocus : null, + onNavigateDown: _focusSectionBelowOverview, + ), ], if (item.background?.trim() case final background? when background.isNotEmpty) ...[ const SizedBox(height: 24), - _buildBackgroundSection(theme, background), + _buildBackgroundSection(theme, background, isMobile: isMobile), ], if (_buildFactsSection(theme) case final Widget facts) ...[ const SizedBox(height: 24), diff --git a/test/screens/catalog_item_detail_screen_test.dart b/test/screens/catalog_item_detail_screen_test.dart index 7e586af29..d5578c41b 100644 --- a/test/screens/catalog_item_detail_screen_test.dart +++ b/test/screens/catalog_item_detail_screen_test.dart @@ -42,9 +42,14 @@ import '../test_helpers/prefs.dart'; class _FakeCatalogSource implements CatalogSource { final WatchlistChangeNotifier _watchlistChanges = WatchlistChangeNotifier(); - _FakeCatalogSource({bool watchlistLoading = false, this.detail, this.detailError, this.detailCompleter}) - : _watchlistValue = watchlistLoading ? null : false, - _watchlistLoad = watchlistLoading ? Completer() : null; + _FakeCatalogSource({ + bool watchlistLoading = false, + this.supportsWatchlist = true, + this.detail, + this.detailError, + this.detailCompleter, + }) : _watchlistValue = watchlistLoading ? null : false, + _watchlistLoad = watchlistLoading ? Completer() : null; bool? _watchlistValue; final Completer? _watchlistLoad; @@ -61,7 +66,7 @@ class _FakeCatalogSource implements CatalogSource { String get displayName => 'Trakt'; @override - bool get supportsWatchlist => true; + final bool supportsWatchlist; @override Listenable get watchlistChanges => _watchlistChanges; @@ -923,6 +928,10 @@ void main() { await _pumpDetail(tester, source); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); + await tester.pumpAndSettle(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_overview'); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); await tester.pumpAndSettle(); expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_cast_row'); @@ -1080,6 +1089,10 @@ void main() { await _pumpDetail(tester, _FakeCatalogSource()); expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_watchlist'); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); + await tester.pumpAndSettle(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_overview'); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); await tester.pumpAndSettle(); expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_cast_row'); @@ -1096,6 +1109,10 @@ void main() { await tester.pumpAndSettle(); expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_cast_row'); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); + await tester.pumpAndSettle(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_overview'); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); await tester.pumpAndSettle(); expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_watchlist'); @@ -1126,6 +1143,10 @@ void main() { ]; await _pumpDetail(tester, _FakeCatalogSource(), matches: matches); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); + await tester.pumpAndSettle(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_overview'); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); await tester.pumpAndSettle(); expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_match_1'); @@ -1141,6 +1162,87 @@ void main() { await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); await tester.pumpAndSettle(); expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_match_2'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); + await tester.pumpAndSettle(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_match_1'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); + await tester.pumpAndSettle(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_overview'); + }); + + testWidgets('D-pad stops on the overview and expands it before moving on to the buttons', (tester) async { + tester.view.devicePixelRatio = 1; + tester.view.physicalSize = const Size(1280, 720); + addTearDown(tester.view.resetDevicePixelRatio); + addTearDown(tester.view.resetPhysicalSize); + + // #2199: down from the action bar used to be thrown straight to the next + // button, scrolling long prose past unread. + final item = CatalogItem( + source: CatalogSourceId.trakt, + kind: MediaKind.movie, + title: 'Wordy Movie', + overview: '${'A very long establishing sentence about the movie. ' * 30}Closing line of the overview.', + ids: const CatalogItemIds(tmdb: 31), + links: const [CatalogLink(label: 'StreamCo', url: 'https://example.com/watch', isStreaming: true)], + ); + final source = _FakeCatalogSource(detail: CatalogDetail(item: item)); + + await _pumpDetail(tester, source, item: item); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_watchlist'); + expect(find.textContaining('Closing line'), findsNothing); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); + await tester.pumpAndSettle(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_overview'); + + await tester.sendKeyEvent(LogicalKeyboardKey.select); + await tester.pumpAndSettle(); + expect(find.textContaining('Closing line'), findsOneWidget, reason: 'select expands the collapsed overview'); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_overview'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); + await tester.pumpAndSettle(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_external_link_0'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); + await tester.pumpAndSettle(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_overview'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); + await tester.pumpAndSettle(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_watchlist'); + }); + + testWidgets('up from the first library copy reaches the overview when the item has no action bar', (tester) async { + // No watchlist support, trailer, or Seerr: nothing above the copies is a + // button, but the overview is still a stop rather than a dead end. + final source = _FakeCatalogSource(supportsWatchlist: false); + await _pumpDetail( + tester, + source, + matches: [ + testMediaItem( + id: 'match_1', + libraryTitle: 'Movies', + serverName: 'Living Room', + mediaVersions: const [MediaVersion(id: 'v1', videoResolution: '4k')], + ), + ], + ); + expect(find.byType(FocusableActionBar), findsNothing); + + final tile = tester.widget( + find.ancestor(of: find.text('Movies'), matching: find.byType(FocusableListTile)), + ); + tile.focusNode!.requestFocus(); + await tester.pump(); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); + await tester.pumpAndSettle(); + expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_overview'); }); testWidgets('pending watchlist action keeps initial focus and its press retries the snapshot', (tester) async {