From 4712414b3d1de6be476dfdeda72ae973eb7ff465 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Sun, 6 Sep 2026 20:21:11 +0200 Subject: [PATCH] fix(focus): reclaim detached grid nodes without stealing live focus Historical focus contexts kept detached grid nodes alive, while stale menu captures could queue focus after their row was gone. Track actual attachment lifetime, preserve pending and remembered destinations, and restore captured focus only to a live eligible leaf without overriding a covering route. --- lib/mixins/grid_focus_node_mixin.dart | 53 +++- lib/screens/libraries/folder_tree_item.dart | 9 +- .../libraries/tabs/library_browse_tab.dart | 19 +- lib/utils/focus_utils.dart | 15 + lib/widgets/media_context_menu.dart | 9 +- .../captured_focus_restoration_test.dart | 180 ++++++++++++ test/focus/grid_focus_node_mixin_test.dart | 262 +++++++++++++++++- test/widgets/folder_tree_item_test.dart | 49 ++++ test/widgets/media_context_menu_test.dart | 93 ++++++- 9 files changed, 645 insertions(+), 44 deletions(-) create mode 100644 test/focus/captured_focus_restoration_test.dart diff --git a/lib/mixins/grid_focus_node_mixin.dart b/lib/mixins/grid_focus_node_mixin.dart index 252025235..0b99af85f 100644 --- a/lib/mixins/grid_focus_node_mixin.dart +++ b/lib/mixins/grid_focus_node_mixin.dart @@ -1,5 +1,46 @@ import 'package:flutter/material.dart'; +/// A detached FocusNode retains its last context, even after that host unmounts. +/// Keep the attachment token so eviction can distinguish history from borrowers. +class _GridFocusNode extends FocusNode { + _GridFocusNode({super.debugLabel}); + + FocusAttachment? _gridAttachment; + bool _hasPendingFocusRequest = false; + + bool get _isAttached => _gridAttachment?.isAttached ?? false; + + @override + FocusAttachment attach( + BuildContext? context, { + FocusOnKeyEventCallback? onKeyEvent, + // ignore: deprecated_member_use + FocusOnKeyCallback? onKey, + }) { + // Forward the legacy argument because this still implements FocusNode.attach. + // ignore: deprecated_member_use + final attachment = super.attach(context, onKeyEvent: onKeyEvent, onKey: onKey); + _gridAttachment = attachment; + _hasPendingFocusRequest = false; + return attachment; + } + + @override + void requestFocus([FocusNode? node]) { + if (node == null && !_isAttached && parent == null && canRequestFocus) { + _hasPendingFocusRequest = true; + } + super.requestFocus(node); + } + + @override + void dispose() { + _hasPendingFocusRequest = false; + super.dispose(); + _gridAttachment = null; + } +} + /// Owns grid nodes independently of their current spatial index. A content /// commit reconciles every realized item's node, including scope history and /// references captured by menus; it never requests focus behind a cover. @@ -13,7 +54,7 @@ mixin GridFocusNodeMixin on State { int lastFocusedGridContentVersion = 0; FocusNode getGridItemFocusNode(int index, {String prefix = 'grid_item', String? debugLabel}) { - return gridItemFocusNodes.putIfAbsent(index, () => FocusNode(debugLabel: debugLabel ?? '${prefix}_$index')); + return gridItemFocusNodes.putIfAbsent(index, () => _GridFocusNode(debugLabel: debugLabel ?? '${prefix}_$index')); } /// [firstNode] must resolve the current index-zero node owned by this mixin, @@ -66,6 +107,7 @@ mixin GridFocusNodeMixin on State { // Disable stale menu/route destinations immediately, without issuing a // replacement request. The owning scope supplies its normal fallback. node.canRequestFocus = false; + if (node is _GridFocusNode) node._hasPendingFocusRequest = false; _retiredGridFocusNodes.add(node); if (_gridRetirementScheduled) return; _gridRetirementScheduled = true; @@ -88,8 +130,8 @@ mixin GridFocusNodeMixin on State { if (lastFocusedGridIndex != null && lastFocusedGridIndex! >= itemCount) lastFocusedGridIndex = null; } - /// Evict detached nodes far from the viewport, never a mounted or remembered - /// leaf. Those nodes may still be borrowed by a card or a covering menu. + /// Evict detached nodes far from the viewport, never live attachments or + /// remembered/pending destinations. The budget is soft while those survive. void evictDistantFocusNodes(int centerIndex, {int keepCount = 200}) { if (gridItemFocusNodes.length <= keepCount) return; final halfKeep = keepCount ~/ 2; @@ -99,7 +141,10 @@ mixin GridFocusNodeMixin on State { for (final entry in gridItemFocusNodes.entries) { final node = entry.value; if ((entry.key < keepStart || entry.key > keepEnd) && - node.context == null && + node is _GridFocusNode && + !node._isAttached && + !node._hasPendingFocusRequest && + !(shouldRestoreGridFocus && entry.key == lastFocusedGridIndex) && !node.hasFocus && node.enclosingScope?.focusedChild != node) { keysToRemove.add(entry.key); diff --git a/lib/screens/libraries/folder_tree_item.dart b/lib/screens/libraries/folder_tree_item.dart index 5c738a314..d0df1ca49 100644 --- a/lib/screens/libraries/folder_tree_item.dart +++ b/lib/screens/libraries/folder_tree_item.dart @@ -14,6 +14,7 @@ import '../../mixins/context_menu_tap_mixin.dart'; import '../../services/settings_service.dart'; import '../../widgets/settings_builder.dart'; import '../../utils/formatters.dart'; +import '../../utils/focus_utils.dart'; import '../../utils/provider_extensions.dart'; import '../../widgets/app_menu.dart'; import '../../widgets/media_context_menu.dart'; @@ -161,12 +162,8 @@ class _FolderTreeItemState extends State with ContextMenuTapMixi if (selected == null) { // Dismissed — restore focus to the row. Play/Shuffle hand off to the // player instead (mirrors MediaContextMenu's didNavigate handling). - if (previousFocus != null && previousFocus.canRequestFocus) { - WidgetsBinding.instance.addPostFrameCallback((_) { - if (previousFocus.canRequestFocus) { - previousFocus.requestFocus(); - } - }); + if (previousFocus != null) { + FocusUtils.restoreFocusAfterBuild(this, previousFocus); } return; } diff --git a/lib/screens/libraries/tabs/library_browse_tab.dart b/lib/screens/libraries/tabs/library_browse_tab.dart index f124dc855..101550a14 100644 --- a/lib/screens/libraries/tabs/library_browse_tab.dart +++ b/lib/screens/libraries/tabs/library_browse_tab.dart @@ -1438,15 +1438,8 @@ class _LibraryBrowseTabState extends BaseLibraryTabState= totalSize) return; final node = _cardFocusNode(targetIndex); - if (node.context != null) { - node.requestFocus(); - } else { - WidgetsBinding.instance.addPostFrameCallback((_) { - if (mounted) node.requestFocus(); - }); - } + node.requestFocus(); } } diff --git a/lib/utils/focus_utils.dart b/lib/utils/focus_utils.dart index 51a6dc81e..f161472cc 100644 --- a/lib/utils/focus_utils.dart +++ b/lib/utils/focus_utils.dart @@ -17,4 +17,19 @@ class FocusUtils { } }); } + + /// Restore a captured menu destination only while its owner and live leaf + /// still allow it. Unlike initial focus, this must not queue a detached node. + static void restoreFocusAfterBuild(State state, FocusNode focusNode) { + // A post-frame callback may not run until this node is attached again. + // Do not let an already-detached capture become a future focus request. + if (focusNode.parent == null || !focusNode.canRequestFocus) return; + WidgetsBinding.instance.addPostFrameCallback((_) { + if (!state.mounted) return; + final destination = focusNode.context; + if (destination == null || !destination.mounted || focusNode.parent == null || !focusNode.canRequestFocus) return; + if (!(ModalRoute.of(state.context)?.isCurrent ?? true)) return; + focusNode.requestFocus(); + }); + } } diff --git a/lib/widgets/media_context_menu.dart b/lib/widgets/media_context_menu.dart index 5797e36a1..b27ba777f 100644 --- a/lib/widgets/media_context_menu.dart +++ b/lib/widgets/media_context_menu.dart @@ -23,6 +23,7 @@ import '../services/catalog/library_watchlist_candidates.dart'; import '../utils/content_utils.dart'; import '../utils/delete_impact.dart'; import '../utils/download_utils.dart'; +import '../utils/focus_utils.dart'; import '../utils/global_key_utils.dart'; import '../providers/download_provider.dart'; import '../providers/multi_server_provider.dart'; @@ -959,12 +960,8 @@ class MediaContextMenuState extends State { // Restore focus to the previously focused item after the menu closes, // but only if no navigation occurred and the focus node is still valid - if (!didNavigate && previousFocus != null && previousFocus.canRequestFocus) { - WidgetsBinding.instance.addPostFrameCallback((_) { - if (previousFocus.canRequestFocus) { - previousFocus.requestFocus(); - } - }); + if (!didNavigate && previousFocus != null) { + FocusUtils.restoreFocusAfterBuild(this, previousFocus); } } } diff --git a/test/focus/captured_focus_restoration_test.dart b/test/focus/captured_focus_restoration_test.dart new file mode 100644 index 000000000..ec4369b8c --- /dev/null +++ b/test/focus/captured_focus_restoration_test.dart @@ -0,0 +1,180 @@ +import 'dart:async'; + +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/focus/covered_route_focus_boundary.dart'; +import 'package:plezy/utils/focus_utils.dart'; + +void main() { + testWidgets('a captured detached node cannot queue focus through its still-mounted former host', (tester) async { + final key = GlobalKey<_RestorationHarnessState>(); + await tester.pumpWidget(MaterialApp(home: _RestorationHarness(key: key))); + final state = key.currentState!; + state.original.requestFocus(); + await tester.pump(); + state.swapHost(true); + state.replacement.requestFocus(); + await tester.pump(); + await tester.pump(); + expect(state.original.context!.mounted, isTrue); + expect(state.original.parent, isNull); + expect(state.original.canRequestFocus, isTrue); + + FocusUtils.restoreFocusAfterBuild(state, state.original); + await tester.pump(); + await tester.pump(); + expect(state.replacement.hasPrimaryFocus, isTrue); + state.guard.requestFocus(); + await tester.pump(); + expect(state.guard.hasPrimaryFocus, isTrue); + state.swapHost(false); + await tester.pump(); + await tester.pump(); + expect(state.guard.hasPrimaryFocus, isTrue, reason: 'a stale restoration must not become a preattachment request'); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + expect(state.selected, 'guard'); + }); + + testWidgets('profile subtree disposal invalidates a queued captured restoration', (tester) async { + final key = GlobalKey<_RestorationHarnessState>(); + await tester.pumpWidget(MaterialApp(home: _RestorationHarness(key: key))); + final state = key.currentState!; + state.original.requestFocus(); + await tester.pump(); + FocusUtils.restoreFocusAfterBuild(state, state.original); + final nextProfile = FocusNode(); + addTearDown(nextProfile.dispose); + var selected = false; + await tester.pumpWidget( + MaterialApp( + home: Focus( + focusNode: nextProfile, + autofocus: true, + onKeyEvent: (_, event) { + if (event is KeyDownEvent && event.logicalKey == LogicalKeyboardKey.enter) { + selected = true; + return KeyEventResult.handled; + } + return KeyEventResult.ignored; + }, + child: const Text('Next profile'), + ), + ), + ); + await tester.pump(); + expect(key.currentState, isNull); + expect(nextProfile.hasPrimaryFocus, isTrue); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + expect(selected, isTrue); + expect(tester.takeException(), isNull); + }); + + testWidgets('captured restoration does not steal from a newly covering route', (tester) async { + final key = GlobalKey<_RestorationHarnessState>(); + await tester.pumpWidget(MaterialApp(home: _RestorationHarness(key: key))); + final state = key.currentState!; + state.original.requestFocus(); + await tester.pump(); + final cover = FocusNode(); + addTearDown(cover.dispose); + unawaited( + showDialog( + context: state.context, + builder: (_) => AlertDialog( + content: Focus(focusNode: cover, autofocus: true, child: const Text('Cover')), + ), + ), + ); + await tester.pumpAndSettle(); + expect(state.original.canRequestFocus, isTrue, reason: 'ordinary covered routes only skip traversal'); + FocusUtils.restoreFocusAfterBuild(state, state.original); + await tester.pump(); + await tester.pump(); + expect(cover.hasPrimaryFocus, isTrue); + Navigator.of(cover.context!).pop(); + await tester.pumpAndSettle(); + expect(state.original.hasPrimaryFocus, isTrue); + }); + + testWidgets('captured nested-route restoration respects the outer covered boundary', (tester) async { + final key = GlobalKey<_RestorationHarnessState>(); + await tester.pumpWidget( + MaterialApp( + home: CoveredRouteFocusBoundary( + child: Navigator( + onGenerateRoute: (_) => MaterialPageRoute(builder: (_) => _RestorationHarness(key: key)), + ), + ), + ), + ); + final state = key.currentState!; + state.original.requestFocus(); + await tester.pump(); + final cover = FocusNode(); + addTearDown(cover.dispose); + unawaited( + showDialog( + context: state.context, + builder: (_) => AlertDialog( + content: Focus(focusNode: cover, autofocus: true, child: const Text('Profile picker')), + ), + ), + ); + await tester.pumpAndSettle(); + expect(ModalRoute.of(state.context)!.isCurrent, isTrue); + expect(state.original.canRequestFocus, isFalse); + FocusUtils.restoreFocusAfterBuild(state, state.original); + await tester.pump(); + await tester.pump(); + expect(cover.hasPrimaryFocus, isTrue); + Navigator.of(cover.context!).pop(); + await tester.pumpAndSettle(); + expect(state.original.hasPrimaryFocus, isTrue); + }); +} + +class _RestorationHarness extends StatefulWidget { + const _RestorationHarness({super.key}); + + @override + State<_RestorationHarness> createState() => _RestorationHarnessState(); +} + +class _RestorationHarnessState extends State<_RestorationHarness> { + final original = FocusNode(debugLabel: 'original'); + final replacement = FocusNode(debugLabel: 'replacement'); + final guard = FocusNode(debugLabel: 'guard'); + bool useReplacement = false; + String? selected; + + void swapHost(bool value) => setState(() => useReplacement = value); + + @override + void dispose() { + original.dispose(); + replacement.dispose(); + guard.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) => Scaffold( + body: Column( + children: [ + Focus(focusNode: useReplacement ? replacement : original, child: const Text('Item')), + Focus( + focusNode: guard, + onKeyEvent: (_, event) { + if (event is KeyDownEvent && event.logicalKey == LogicalKeyboardKey.enter) { + selected = 'guard'; + return KeyEventResult.handled; + } + return KeyEventResult.ignored; + }, + child: const Text('Guard'), + ), + ], + ), + ); +} diff --git a/test/focus/grid_focus_node_mixin_test.dart b/test/focus/grid_focus_node_mixin_test.dart index ddf695c34..9ebc25abf 100644 --- a/test/focus/grid_focus_node_mixin_test.dart +++ b/test/focus/grid_focus_node_mixin_test.dart @@ -1,8 +1,12 @@ import 'package:flutter/material.dart'; +import 'package:flutter/rendering.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/focus/focusable_wrapper.dart'; +import 'package:plezy/focus/input_mode_tracker.dart'; import 'package:plezy/mixins/grid_focus_node_mixin.dart'; import 'package:plezy/mixins/library_tab_focus_mixin.dart'; +import 'package:plezy/utils/focus_utils.dart'; void main() { testWidgets('covered item keeps captured restoration across first and nonfirst positions', (tester) async { @@ -29,7 +33,8 @@ void main() { } Navigator.of(cover.context!).pop(); await tester.pumpAndSettle(); - captured.requestFocus(); + FocusUtils.restoreFocusAfterBuild(state, captured); + await tester.pump(); await tester.pump(); await tester.sendKeyEvent(LogicalKeyboardKey.enter); expect(state.selected, 'A'); @@ -61,6 +66,10 @@ void main() { await tester.pumpAndSettle(); state.focusFirstItem(); await tester.pump(); + FocusUtils.restoreFocusAfterBuild(state, captured); + await tester.pump(); + await tester.pump(); + expect(tester.takeException(), isNull); await tester.sendKeyEvent(LogicalKeyboardKey.enter); expect(state.selected, 'X'); }); @@ -72,6 +81,9 @@ void main() { state.firstItemFocusNode.requestFocus(); await tester.pump(); final captured = FocusManager.instance.primaryFocus!; + state.evictDistantFocusNodes(20, keepCount: 1); + await tester.pump(); + expect(captured.hasPrimaryFocus, isTrue, reason: 'eviction must not disturb a live focused leaf'); final cover = FocusNode(); addTearDown(cover.dispose); unawaitedDialog(state.context, cover); @@ -86,6 +98,193 @@ void main() { await tester.sendKeyEvent(LogicalKeyboardKey.enter); expect(state.selected, 'A'); }); + + testWidgets('lazy grid reclaims historical attachments and remains traversable after returning', (tester) async { + final key = GlobalKey<_LazyGridHarnessState>(); + await tester.pumpWidget( + InputModeTracker( + child: MaterialApp(home: _LazyGridHarness(key: key)), + ), + ); + final state = key.currentState!; + final historical = state.getGridItemFocusNode(0); + + for (var index = 12; index <= 336; index += 12) { + state.scrollTo(index); + await tester.pump(); + state.evictDistantFocusNodes(index); + await tester.pump(); + } + + expect(state.realizedItems, containsAll(List.generate(336, (index) => index))); + expect(historical.context, isNotNull, reason: 'Flutter retains the last attachment context'); + expect(historical.context!.mounted, isFalse); + expect(historical.canRequestFocus, isFalse, reason: 'the detached historical resource was retired'); + expect(state.gridItemFocusNodes.length, lessThanOrEqualTo(200)); + + for (var index = 324; index >= 0; index -= 12) { + state.scrollTo(index); + await tester.pump(); + state.evictDistantFocusNodes(index); + await tester.pump(); + } + final returned = state.getGridItemFocusNode(0); + expect(returned, isNot(same(historical))); + expect(returned.rect, tester.getRect(find.byKey(const ValueKey('lazy_0')))); + expect(state.gridItemFocusNodes.length, lessThanOrEqualTo(200)); + + await tester.tap(find.text('Item 0')); + expect(state.selected, 0); + returned.requestFocus(); + await tester.pump(); + await tester.sendKeyEvent(LogicalKeyboardKey.tab); + await tester.pump(); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + expect(state.selected, 1, reason: 'keyboard traversal uses the returned item identities'); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown); + await tester.pump(); + await tester.sendKeyEvent(LogicalKeyboardKey.select); + expect(state.selected, 5, reason: 'd-pad navigation still reaches the next row'); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + await tester.sendKeyEvent(LogicalKeyboardKey.gameButtonA); + expect(state.selected, 6, reason: 'gamepad selection activates the newly focused item'); + }); + + testWidgets('detached remembered destination survives until its content version expires', (tester) async { + final key = GlobalKey<_LazyGridHarnessState>(); + await tester.pumpWidget(MaterialApp(home: _LazyGridHarness(key: key))); + final state = key.currentState!; + final remembered = state.getGridItemFocusNode(0); + remembered.requestFocus(); + await tester.pump(); + state.toolbar.requestFocus(); + await tester.pump(); + state.scrollTo(300); + await tester.pump(); + expect(remembered.parent, isNull); + expect(remembered.context!.mounted, isFalse); + state.evictDistantFocusNodes(300, keepCount: 4); + await tester.pump(); + expect(state.getGridItemFocusNode(0), same(remembered)); + expect(state.toolbar.hasPrimaryFocus, isTrue, reason: 'retaining history does not restore focus'); + + state.scrollTo(0); + await tester.pump(); + remembered.requestFocus(); + await tester.pump(); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + expect(state.selected, 0); + state.toolbar.requestFocus(); + await tester.pump(); + state.scrollTo(300); + await tester.pump(); + state.gridContentVersion++; + state.evictDistantFocusNodes(300, keepCount: 4); + await tester.pump(); + expect(remembered.canRequestFocus, isFalse, reason: 'an obsolete remembered version no longer pins the node'); + }); + + testWidgets('preattachment destination survives eviction and releases its pin after attaching', (tester) async { + final key = GlobalKey<_LazyGridHarnessState>(); + await tester.pumpWidget(MaterialApp(home: _LazyGridHarness(key: key))); + final state = key.currentState!; + final pending = state.getGridItemFocusNode(500); + pending.requestFocus(); + state.evictDistantFocusNodes(0, keepCount: 4); + await tester.pump(); + expect(state.getGridItemFocusNode(500), same(pending)); + state.scrollTo(500); + await tester.pump(); + await tester.pump(); + expect(pending.hasPrimaryFocus, isTrue, reason: 'Flutter completes the retained preattachment request'); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + expect(state.selected, 500); + + state.toolbar.requestFocus(); + await tester.pump(); + state.scrollTo(0); + await tester.pump(); + state.getGridItemFocusNode(0).requestFocus(); + await tester.pump(); + state.evictDistantFocusNodes(0, keepCount: 4); + await tester.pump(); + expect(pending.canRequestFocus, isFalse, reason: 'the completed request must not pin detached history forever'); + }); + + testWidgets('attachment tokens distinguish detached mounted contexts from unparented live borrowers', (tester) async { + final key = GlobalKey<_GridFocusHarnessState>(); + await tester.pumpWidget(MaterialApp(home: _GridFocusHarness(key: key))); + final state = key.currentState!; + final detached = state.getGridItemFocusNode(300); + final oldAttachment = detached.attach(state.context); + oldAttachment.reparent(); + oldAttachment.detach(); + expect(detached.context!.mounted, isTrue); + expect(oldAttachment.isAttached, isFalse); + + final borrower = state.getGridItemFocusNode(301); + final liveAttachment = borrower.attach( + state.context, + onKeyEvent: (_, event) { + if (event is KeyDownEvent && event.logicalKey == LogicalKeyboardKey.enter) { + state.selected = 'borrower'; + return KeyEventResult.handled; + } + return KeyEventResult.ignored; + }, + ); + expect(borrower.parent, isNull); + state.evictDistantFocusNodes(0, keepCount: 1); + await tester.pump(); + expect(detached.canRequestFocus, isFalse); + expect(liveAttachment.isAttached, isTrue); + expect(state.getGridItemFocusNode(301), same(borrower)); + liveAttachment.reparent(); + borrower.requestFocus(); + await tester.pump(); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + expect(state.selected, 'borrower'); + liveAttachment.detach(); + }); + + testWidgets('offstage and globally reparented grid hosts keep their live nodes', (tester) async { + final key = GlobalKey<_GridFocusHarnessState>(); + var moved = false; + late StateSetter rebuild; + await tester.pumpWidget( + MaterialApp( + home: StatefulBuilder( + builder: (_, setState) { + rebuild = setState; + return Row( + children: [ + Expanded( + child: Offstage(offstage: true, child: moved ? const SizedBox() : _GridFocusHarness(key: key)), + ), + Expanded(child: moved ? _GridFocusHarness(key: key) : const SizedBox()), + ], + ); + }, + ), + ), + ); + final state = key.currentState!; + final captured = state.firstItemFocusNode; + state.evictDistantFocusNodes(500, keepCount: 1); + await tester.pump(); + expect(state.firstItemFocusNode, same(captured)); + rebuild(() => moved = true); + await tester.pump(); + state.evictDistantFocusNodes(500, keepCount: 1); + await tester.pump(); + expect(state.firstItemFocusNode, same(captured)); + captured.requestFocus(); + await tester.pump(); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + expect(state.selected, 'A'); + expect(captured.rect, tester.getRect(find.text('A'))); + }); } void unawaitedDialog(BuildContext context, FocusNode node) { @@ -152,3 +351,64 @@ class _GridFocusHarnessState extends State<_GridFocusHarness> ), ); } + +class _LazyGridHarness extends StatefulWidget { + const _LazyGridHarness({super.key}); + + @override + State<_LazyGridHarness> createState() => _LazyGridHarnessState(); +} + +class _LazyGridHarnessState extends State<_LazyGridHarness> with GridFocusNodeMixin<_LazyGridHarness> { + final controller = ScrollController(); + final toolbar = FocusNode(debugLabel: 'toolbar'); + final realizedItems = {}; + int? selected; + + void scrollTo(int index) => controller.jumpTo((index ~/ 4) * 60.0); + + @override + void dispose() { + controller.dispose(); + toolbar.dispose(); + disposeGridFocusNodes(); + super.dispose(); + } + + @override + Widget build(BuildContext context) => Scaffold( + body: Column( + children: [ + Focus(focusNode: toolbar, child: const Text('Toolbar')), + SizedBox( + width: 400, + height: 180, + child: GridView.builder( + scrollCacheExtent: ScrollCacheExtent.pixels(0), + controller: controller, + addAutomaticKeepAlives: false, + gridDelegate: const SliverGridDelegateWithFixedCrossAxisCount(crossAxisCount: 4, mainAxisExtent: 60), + itemCount: 650, + itemBuilder: (_, index) { + realizedItems.add(index); + return FocusableWrapper( + key: ValueKey('lazy_$index'), + focusNode: getGridItemFocusNode(index), + autoScroll: false, + disableScale: true, + onFocusChange: (focused) => trackGridItemFocus(index, focused), + onSelect: () => selected = index, + onNavigateDown: () => getGridItemFocusNode(index + 4).requestFocus(), + onNavigateRight: () => getGridItemFocusNode(index + 1).requestFocus(), + child: GestureDetector( + onTap: () => selected = index, + child: Center(child: Text('Item $index')), + ), + ); + }, + ), + ), + ], + ), + ); +} diff --git a/test/widgets/folder_tree_item_test.dart b/test/widgets/folder_tree_item_test.dart index 3125182df..dfa6f5fa9 100644 --- a/test/widgets/folder_tree_item_test.dart +++ b/test/widgets/folder_tree_item_test.dart @@ -1,6 +1,9 @@ import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/i18n/strings.g.dart'; import 'package:plezy/media/media_backend.dart'; +import 'package:plezy/theme/mono_theme.dart'; import 'package:plezy/media/media_kind.dart'; import 'package:plezy/screens/libraries/folder_tree_item.dart'; @@ -61,4 +64,50 @@ void main() { expect(played, isTrue); expect(shuffled, isTrue); }); + + testWidgets('folder menu cancellation restores the row but play preserves its focus handoff', (tester) async { + final row = FocusNode(); + final player = FocusNode(); + addTearDown(row.dispose); + addTearDown(player.dispose); + var selected = false; + final item = testMediaItem(id: 'folder-1', backend: MediaBackend.jellyfin, title: 'Videos'); + await tester.pumpWidget( + MaterialApp( + theme: monoTheme(dark: true).copyWith(platform: TargetPlatform.windows), + home: Material( + child: Column( + children: [ + FolderTreeItem( + item: item, + depth: 0, + isFolder: true, + focusNode: row, + onTap: () => selected = true, + onPlayAll: player.requestFocus, + ), + Focus(focusNode: player, child: const Text('Player')), + ], + ), + ), + ), + ); + row.requestFocus(); + await tester.pump(); + await tester.sendKeyEvent(LogicalKeyboardKey.contextMenu); + await tester.pumpAndSettle(); + expect(find.text(t.common.play), findsOneWidget); + Navigator.of(tester.element(find.byType(FolderTreeItem))).pop(); + await tester.pumpAndSettle(); + expect(row.hasPrimaryFocus, isTrue); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + expect(selected, isTrue); + + await tester.sendKeyEvent(LogicalKeyboardKey.contextMenu); + await tester.pumpAndSettle(); + await tester.tap(find.text(t.common.play)); + await tester.pumpAndSettle(); + expect(player.hasPrimaryFocus, isTrue); + expect(row.hasFocus, isFalse); + }); } diff --git a/test/widgets/media_context_menu_test.dart b/test/widgets/media_context_menu_test.dart index 420406635..c92b51a83 100644 --- a/test/widgets/media_context_menu_test.dart +++ b/test/widgets/media_context_menu_test.dart @@ -11,6 +11,7 @@ import 'package:http/testing.dart'; import 'package:material_symbols_icons/symbols.dart'; import 'package:plezy/connection/connection.dart'; import 'package:plezy/database/app_database.dart'; +import 'package:plezy/focus/focusable_wrapper.dart'; import 'package:plezy/i18n/strings.g.dart'; import 'package:plezy/navigation/profile_navigation_scope.dart'; import 'package:plezy/media/ids.dart'; @@ -64,6 +65,78 @@ import '../test_helpers/profile_stack.dart'; import '../test_helpers/stub_music_playback_service.dart'; void main() { + testWidgets('cancelled media menu restores its live captured item', (tester) async { + final destination = FocusNode(); + addTearDown(destination.dispose); + var selected = false; + final menuKey = await _pumpPlexMovieMenu( + tester, + const [], + wrapMenu: (menu) => FocusableWrapper(focusNode: destination, onSelect: () => selected = true, child: menu), + ); + destination.requestFocus(); + await tester.pump(); + menuKey.currentState!.showContextMenu(tester.element(find.text('picker target'))); + await tester.pumpAndSettle(); + expect(destination.hasPrimaryFocus, isFalse); + Navigator.of(menuKey.currentContext!).pop(); + await tester.pumpAndSettle(); + expect(destination.hasPrimaryFocus, isTrue); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + expect(selected, isTrue); + }); + + testWidgets('queued media menu restoration cannot outlive its profile owner', (tester) async { + final destination = FocusNode(); + final nextProfile = FocusNode(); + final showOwner = ValueNotifier(true); + addTearDown(destination.dispose); + addTearDown(nextProfile.dispose); + addTearDown(showOwner.dispose); + String? selected; + final menuKey = await _pumpPlexMovieMenu( + tester, + const [], + wrapMenu: (menu) => ValueListenableBuilder( + valueListenable: showOwner, + builder: (_, visible, _) => Column( + mainAxisSize: MainAxisSize.min, + children: [ + FocusableWrapper( + focusNode: destination, + onSelect: () => selected = 'old profile', + child: visible ? menu : const Text('Removed profile menu'), + ), + FocusableWrapper( + focusNode: nextProfile, + onSelect: () => selected = 'new profile', + child: const Text('New profile'), + ), + ], + ), + ), + ); + destination.requestFocus(); + await tester.pump(); + menuKey.currentState!.showContextMenu(tester.element(find.text('picker target'))); + await tester.pumpAndSettle(); + Navigator.of(menuKey.currentContext!).pop(); + await tester.idle(); // The menu has queued its post-frame restore. + showOwner.value = false; + nextProfile.requestFocus(); + await tester.pumpAndSettle(); + expect(menuKey.currentState, isNull); + expect( + destination.context!.mounted, + isTrue, + reason: 'the external destination survives, but its menu owner does not', + ); + expect(nextProfile.hasPrimaryFocus, isTrue); + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + expect(selected, 'new profile'); + expect(tester.takeException(), isNull); + }); + TestWidgetsFlutterBinding.ensureInitialized(); group('isAdminActionAllowedForMediaItem', () { @@ -1404,8 +1477,9 @@ void main() { Future> _pumpPlexMovieMenu( WidgetTester tester, - List<({String id, String title})> playlists, -) async { + List<({String id, String title})> playlists, { + Widget Function(Widget menu)? wrapMenu, +}) async { LocaleSettings.setLocaleSync(AppLocale.en); TvDetectionService.debugSetAppleTVOverride(true); addTearDown(() => TvDetectionService.debugSetAppleTVOverride(null)); @@ -1464,6 +1538,11 @@ Future> _pumpPlexMovieMenu( title: 'Movie', serverId: 'plex-1', ); + final menu = MediaContextMenu( + key: menuKey, + item: item, + child: const SizedBox(width: 120, height: 80, child: Text('picker target')), + ); await tester.pumpWidget( TranslationProvider( child: MultiProvider( @@ -1473,15 +1552,7 @@ Future> _pumpPlexMovieMenu( ], child: MaterialApp( theme: monoTheme(dark: true), - home: Scaffold( - body: Center( - child: MediaContextMenu( - key: menuKey, - item: item, - child: const SizedBox(width: 120, height: 80, child: Text('picker target')), - ), - ), - ), + home: Scaffold(body: Center(child: wrapMenu?.call(menu) ?? menu)), ), ), ),