From a2b3377d91f5e175f60da099fdd76d82fb550e69 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Mon, 17 Aug 2026 08:58:00 +0200 Subject: [PATCH] fix(libraries): stop d-pad up snapping collection and playlist grids to top On TV, pressing UP in the library Collections or Playlists grid snapped the list back to the top and dropped focus on the tab chips, making long lists impossible to navigate. Default directional focus traversal scrolls the found card into view via Scrollable.ensureVisible, whose outer-scrollable pass routes through the NestedScrollView coordinator and resets the inner grid position to zero on every UP press. Give the shared paginated card grid explicit per-card d-pad navigation, matching the browse tab: managed per-index focus nodes, row/column moves that request focus directly, first-row UP to the tab bar, and first-column LEFT to the sidebar. Focus changes now scroll only through FocusableWrapper's delta-based auto-scroll. close #1977 --- .../tabs/paginated_card_grid_tab.dart | 71 ++++++--- .../library_collections_tab_test.dart | 140 +++++++++++++++++- .../libraries/library_playlists_tab_test.dart | 22 ++- 3 files changed, 202 insertions(+), 31 deletions(-) diff --git a/lib/screens/libraries/tabs/paginated_card_grid_tab.dart b/lib/screens/libraries/tabs/paginated_card_grid_tab.dart index 4ef05476f..7e9f1474e 100644 --- a/lib/screens/libraries/tabs/paginated_card_grid_tab.dart +++ b/lib/screens/libraries/tabs/paginated_card_grid_tab.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import '../../../focus/input_mode_tracker.dart'; import '../../../media/media_item.dart'; +import '../../../mixins/grid_focus_node_mixin.dart'; import '../../../mixins/library_tab_focus_mixin.dart'; import '../../../mixins/paginated_item_loader.dart'; import '../../../mixins/standard_paginated_view.dart'; @@ -26,7 +27,12 @@ import 'base_library_tab.dart'; /// [BaseLibraryTabState]. abstract class PaginatedCardGridTabState> extends BaseLibraryTabState - with LibraryTabFocusMixin, PaginatedItemLoader, StandardPaginatedView, SkeletonUpgradeScheduler { + with + LibraryTabFocusMixin, + GridFocusNodeMixin, + PaginatedItemLoader, + StandardPaginatedView, + SkeletonUpgradeScheduler { static const double _focusDecorationPadding = 3.0; /// Reuses card widgets across delegate swaps so tab-level setStates @@ -100,12 +106,7 @@ abstract class PaginatedCardGridTabState _buildCard(index, isFirstRow: position.isFirstRow, isFirstColumn: true, disableScale: true), - ); + return _cardMemo.widgetFor(index, item, epoch: position.layoutEpoch!, build: () => _buildCard(position)); } final cached = _cardMemo.tryGet(index, item, epoch: position.layoutEpoch!); @@ -120,44 +121,65 @@ abstract class PaginatedCardGridTabState _buildCard( - index, - isFirstRow: position.isFirstRow, - isFirstColumn: position.isFirstColumn, - fullBleedImage: useFullCardLayout, - ), + build: () => _buildCard(position, fullBleedImage: useFullCardLayout), ); }, ); } - Widget _buildCard( - int index, { - required bool isFirstRow, - required bool isFirstColumn, - bool disableScale = false, - bool fullBleedImage = false, - }) { + Widget _buildCard(MediaCardSliverPosition position, {bool fullBleedImage = false}) { + final index = position.index; final item = loadedItems[index]; if (item == null) { ensureIndexLoaded(index, pageSize: pageSize); return const SkeletonMediaCard(); } + // Explicit navigation instead of default directional traversal. This grid + // lives inside the libraries screen's NestedScrollView (floating chips + // header); framework traversal scrolls the found node into view via + // Scrollable.ensureVisible, whose outer-scrollable pass routes through the + // nested-scroll coordinator and resets the inner position to zero on UP — + // snapping the list back to the top and dropping focus onto the header. + final columnCount = position.columnCount; + final navigateUp = position.isFirstRow ? widget.onBack : () => _focusGridItem(index - columnCount); + final navigateDown = index + columnCount < totalSize ? () => _focusGridItem(index + columnCount) : null; + final navigateLeft = position.isFirstColumn ? _navigateToSidebar : () => _focusGridItem(index - 1); + final navigateRight = !position.isLastColumn && index + 1 < totalSize ? () => _focusGridItem(index + 1) : null; + return FocusableMediaCard( key: Key(idOf(item)), item: item, - focusNode: index == 0 ? firstItemFocusNode : null, - disableScale: disableScale, + focusNode: _cardFocusNode(index), + disableScale: position.disableScale, fullBleedImage: fullBleedImage, cardShapeOverride: usesSquareCards ? CardShape.square : null, onListRefresh: loadItems, - onNavigateUp: isFirstRow ? widget.onBack : null, + onNavigateUp: navigateUp, + onNavigateDown: navigateDown, + onNavigateLeft: navigateLeft, + onNavigateRight: navigateRight, onBack: widget.onBack, - onNavigateLeft: isFirstColumn ? _navigateToSidebar : null, ); } + FocusNode _cardFocusNode(int index) => focusNodeForIndex(index, firstItemFocusNode, prefix: 'paginated_grid_item'); + + /// Move focus to the grid item at [targetIndex]. When the target card is + /// not yet mounted (being built this frame, or still an unloaded skeleton), + /// the pending request lands once its card attaches the node. + void _focusGridItem(int targetIndex) { + if (targetIndex < 0 || targetIndex >= totalSize) return; + final node = _cardFocusNode(targetIndex); + if (node.context != null) { + node.requestFocus(); + } else { + WidgetsBinding.instance.addPostFrameCallback((_) { + if (mounted) node.requestFocus(); + }); + } + } + void _navigateToSidebar() { MainScreenFocusScope.focusSidebarOf(context); } @@ -165,6 +187,7 @@ abstract class PaginatedCardGridTabState(find.byType(FocusableMediaCard)).cardShapeOverride, isNull); }); + + group('D-pad grid navigation', () { + testWidgets('UP moves one row up without resetting the scroll position', (tester) async { + final harness = _CollectionHarness.plexMovies(collectionCount: 60); + addTearDown(harness.dispose); + + await _pumpTab(tester, harness: harness, library: _movieLibrary); + final columns = await _enterGridAndFocusFirstCard(tester); + + for (var i = 0; i < 4; i++) { + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); + await tester.pumpAndSettle(); + } + expect(_primaryFocusLabel(), 'paginated_grid_item_${4 * columns}'); + + final scrollable = Scrollable.of(FocusManager.instance.primaryFocus!.context!); + final pixelsBeforeUp = scrollable.position.pixels; + expect(pixelsBeforeUp, greaterThan(0)); + + // Regression #1977: default directional traversal ran + // Scrollable.ensureVisible through the NestedScrollView coordinator, + // which reset the inner position to 0 and bounced focus to the header. + await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); + await tester.pumpAndSettle(); + + expect(_primaryFocusLabel(), 'paginated_grid_item_${3 * columns}'); + expect(scrollable.position.pixels, greaterThan(0)); + }); + + testWidgets('UP from the first row hands focus to onBack', (tester) async { + final harness = _CollectionHarness.plexMovies(collectionCount: 60); + addTearDown(harness.dispose); + var backCalls = 0; + + await _pumpTab(tester, harness: harness, library: _movieLibrary, onBack: () => backCalls++); + await _enterGridAndFocusFirstCard(tester); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); + await tester.pumpAndSettle(); + + expect(backCalls, 1); + }); + + testWidgets('LEFT and RIGHT move within a row; first column LEFT reaches the sidebar', (tester) async { + final harness = _CollectionHarness.plexMovies(collectionCount: 60); + addTearDown(harness.dispose); + var sidebarCalls = 0; + + await _pumpTab(tester, harness: harness, library: _movieLibrary, focusSidebar: () => sidebarCalls++); + await _enterGridAndFocusFirstCard(tester); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); + await tester.pumpAndSettle(); + expect(_primaryFocusLabel(), 'paginated_grid_item_1'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft); + await tester.pumpAndSettle(); + expect(_primaryFocusLabel(), 'collections_first_item'); + + await tester.sendKeyEvent(LogicalKeyboardKey.arrowLeft); + await tester.pumpAndSettle(); + expect(sidebarCalls, 1); + }); + }); } -Future _pumpTab(WidgetTester tester, {required _CollectionHarness harness, required MediaLibrary library}) async { +Future _pumpTab( + WidgetTester tester, { + required _CollectionHarness harness, + required MediaLibrary library, + VoidCallback? onBack, + VoidCallback? focusSidebar, +}) async { await pumpLibraryTab( tester, provider: harness.provider, - tab: LibraryCollectionsTab(library: library, suppressAutoFocus: true, onBack: () {}), + tab: LibraryCollectionsTab(library: library, suppressAutoFocus: true, onBack: onBack ?? () {}), size: const Size(800, 600), + focusSidebar: focusSidebar, ); await tester.pumpAndSettle(); } +String? _primaryFocusLabel() => FocusManager.instance.primaryFocus?.debugLabel; + +/// Switches to keyboard input mode, focuses the first card, and returns the +/// grid's column count (cards sharing the first realized row's dy). +Future _enterGridAndFocusFirstCard(WidgetTester tester) async { + await tester.sendKeyEvent(LogicalKeyboardKey.tab); + await tester.pump(); + + final cards = find.byType(FocusableMediaCard); + final firstRowDy = tester.getTopLeft(cards.first).dy; + var columns = 0; + for (final element in cards.evaluate()) { + if (tester.getTopLeft(find.byWidget(element.widget)).dy == firstRowDy) columns++; + } + + tester.widget(cards.first).focusNode!.requestFocus(); + await tester.pumpAndSettle(); + expect(_primaryFocusLabel(), 'collections_first_item'); + return columns; +} + class _CollectionHarness { final AppDatabase database; late final MultiServerManager manager; @@ -143,6 +243,42 @@ class _CollectionHarness { return _CollectionHarness._(database: database, client: client); } + /// Movie library with [collectionCount] collections, served as one page. + factory _CollectionHarness.plexMovies({required int collectionCount}) { + final database = AppDatabase.forTesting(NativeDatabase.memory()); + PlexApiCache.initialize(database); + final client = testPlexClient( + config: PlexConfig( + baseUrl: 'https://plex.example.com', + token: 'token', + clientIdentifier: 'client-id', + product: 'Plezy', + version: 'test', + ), + serverId: _serverId, + httpClient: MockClient((request) async { + if (request.url.path != '/library/sections/movies/collections') { + return http.Response('not found', 404); + } + return http.Response( + jsonEncode({ + 'MediaContainer': { + 'size': collectionCount, + 'totalSize': collectionCount, + 'Metadata': [ + for (var i = 0; i < collectionCount; i++) + {'ratingKey': 'collection-$i', 'type': 'collection', 'title': 'Collection $i', 'childCount': 2}, + ], + }, + }), + 200, + headers: {'content-type': 'application/json'}, + ); + }), + ); + return _CollectionHarness._(database: database, client: client); + } + factory _CollectionHarness.jellyfin() { final database = AppDatabase.forTesting(NativeDatabase.memory()); JellyfinApiCache.initialize(database); diff --git a/test/screens/libraries/library_playlists_tab_test.dart b/test/screens/libraries/library_playlists_tab_test.dart index 2ecc6c5ae..035abda46 100644 --- a/test/screens/libraries/library_playlists_tab_test.dart +++ b/test/screens/libraries/library_playlists_tab_test.dart @@ -85,18 +85,27 @@ void main() { expect(first.onNavigateUp, isNotNull); expect(first.onNavigateLeft, isNotNull); expect(first.onBack, isNotNull); + // Every card now carries explicit navigation; default directional + // traversal is bypassed (it resets the NestedScrollView on UP). expect(second.onNavigateUp, isNotNull); - expect(second.onNavigateLeft, isNull); - - final firstColumnBelowTop = cards.firstWhere((card) => card.onNavigateUp == null && card.onNavigateLeft != null); - expect((firstColumnBelowTop.item as MediaPlaylist).id, isNot('playlist-0')); + expect(second.onNavigateLeft, isNotNull); + // First row: UP and BACK hand off to the tab bar, first-column LEFT to + // the sidebar. Second column's LEFT moves within the row instead. first.onNavigateUp!(); first.onBack!(); first.onNavigateLeft!(); + second.onNavigateLeft!(); expect(backCalls, 2); expect(sidebarCalls, 1); + // Below the top row, UP moves focus up a row rather than leaving the grid. + final columns = cards.where((card) => identical(card.onNavigateUp, first.onNavigateUp)).length; + final firstColumnBelowTop = _cardFor(cards, columns); + expect(firstColumnBelowTop.onNavigateUp, isNotNull); + firstColumnBelowTop.onNavigateUp!(); + expect(backCalls, 2); + final firstWidget = tester.widget(find.byKey(const Key('playlist-0'))); harness.rebuild.value++; await tester.pump(); @@ -143,11 +152,14 @@ void main() { expect(first.disableScale, isTrue); expect(first.onNavigateUp, isNotNull); expect(first.onNavigateLeft, isNotNull); - expect(second.onNavigateUp, isNull); + // Rows below the first navigate up explicitly; LEFT always reaches the + // sidebar in the single-column list. + expect(second.onNavigateUp, isNotNull); expect(second.onNavigateLeft, isNotNull); first.onNavigateUp!(); second.onNavigateLeft!(); + second.onNavigateUp!(); expect(backCalls, 1); expect(sidebarCalls, 1); });