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
This commit is contained in:
@@ -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<T extends Object, W extends BaseLibraryTab<T>>
|
||||
extends BaseLibraryTabState<T, W>
|
||||
with LibraryTabFocusMixin<W>, PaginatedItemLoader<T, W>, StandardPaginatedView<T, W>, SkeletonUpgradeScheduler<W> {
|
||||
with
|
||||
LibraryTabFocusMixin<W>,
|
||||
GridFocusNodeMixin<W>,
|
||||
PaginatedItemLoader<T, W>,
|
||||
StandardPaginatedView<T, W>,
|
||||
SkeletonUpgradeScheduler<W> {
|
||||
static const double _focusDecorationPadding = 3.0;
|
||||
|
||||
/// Reuses card widgets across delegate swaps so tab-level setStates
|
||||
@@ -100,12 +106,7 @@ abstract class PaginatedCardGridTabState<T extends Object, W extends BaseLibrary
|
||||
return const SkeletonMediaCard();
|
||||
}
|
||||
if (!position.isGrid) {
|
||||
return _cardMemo.widgetFor(
|
||||
index,
|
||||
item,
|
||||
epoch: position.layoutEpoch!,
|
||||
build: () => _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<T extends Object, W extends BaseLibrary
|
||||
index,
|
||||
item,
|
||||
epoch: position.layoutEpoch!,
|
||||
build: () => _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<T extends Object, W extends BaseLibrary
|
||||
@override
|
||||
void dispose() {
|
||||
disposePagination();
|
||||
disposeGridFocusNodes();
|
||||
super.dispose();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ import 'dart:convert';
|
||||
|
||||
import 'package:drift/native.dart';
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter/services.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
import 'package:http/testing.dart';
|
||||
@@ -46,6 +47,13 @@ final _jellyfinMusicLibrary = MediaLibrary(
|
||||
kind: MediaKind.artist,
|
||||
serverId: _jellyfinServerId,
|
||||
);
|
||||
final _movieLibrary = MediaLibrary(
|
||||
id: 'movies',
|
||||
backend: MediaBackend.plex,
|
||||
title: 'Movies',
|
||||
kind: MediaKind.movie,
|
||||
serverId: _serverId,
|
||||
);
|
||||
|
||||
void main() {
|
||||
TestWidgetsFlutterBinding.ensureInitialized();
|
||||
@@ -87,18 +95,110 @@ void main() {
|
||||
expect(layout.fullBleedImage, isTrue);
|
||||
expect(tester.widget<FocusableMediaCard>(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<void> _pumpTab(WidgetTester tester, {required _CollectionHarness harness, required MediaLibrary library}) async {
|
||||
Future<void> _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<int> _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<FocusableMediaCard>(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);
|
||||
|
||||
@@ -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<FocusableMediaCard>(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);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user