From b11c63f5574b6c6b0c8fbd85d8b02a72de11b531 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Thu, 18 Jun 2026 04:29:34 +0200 Subject: [PATCH] fix: restore sidebar focus visibility --- lib/widgets/side_navigation_rail.dart | 70 ++++++++++++++++----- test/widgets/side_navigation_rail_test.dart | 65 ++++++++++++++++--- 2 files changed, 112 insertions(+), 23 deletions(-) diff --git a/lib/widgets/side_navigation_rail.dart b/lib/widgets/side_navigation_rail.dart index 929faf3a..bf8b9425 100644 --- a/lib/widgets/side_navigation_rail.dart +++ b/lib/widgets/side_navigation_rail.dart @@ -17,6 +17,7 @@ import '../providers/hidden_libraries_provider.dart'; import '../providers/libraries_provider.dart'; import '../services/settings_service.dart'; import '../utils/platform_detector.dart'; +import '../utils/scroll_utils.dart'; import '../utils/library_grouping.dart'; import '../providers/multi_server_provider.dart'; import '../services/fullscreen_state_manager.dart'; @@ -335,14 +336,60 @@ class SideNavigationRailState extends State with MountedSetS /// If [targetKey] is provided, try it first (used when the caller captured /// the intended target before a focus-scope switch overwrote it). void focusActiveItem({String? targetKey}) { - if (targetKey != null) { - final node = _focusTracker.nodeFor(targetKey); - if (node != null) { - node.requestFocus(); - return; - } + final node = _resolveFocusNode(targetKey) ?? _mountedFocusNodeFor(_kHome); + if (node == null) return; + _requestFocusAndReveal(node); + } + + /// Resolve the best mounted focus node in priority order: + /// 1. Explicit [targetKey] (captured before scope switch) + /// 2. Last focused key still in the tracker + /// 3. Currently selected navigation item (tab / library) + /// 4. Home fallback + FocusNode? _resolveFocusNode(String? targetKey) { + return _mountedFocusNodeFor(targetKey) ?? + _mountedFocusNodeFor(_focusTracker.lastFocusedKey) ?? + _mountedFocusNodeFor(_resolveSelectedFocusKey()); + } + + FocusNode? _mountedFocusNodeFor(String? key) { + if (key == null) return null; + final node = _focusTracker.nodeFor(key); + return node?.context == null ? null : node; + } + + /// Derive a focus key from the current selection state (tab + library). + /// Returns null if no meaningful selected item exists. + String? _resolveSelectedFocusKey() { + switch (widget.selectedTab) { + case NavigationTabId.discover: + return _kHome; + case NavigationTabId.libraries: + final libKey = widget.selectedLibraryKey; + if (libKey != null && _librariesExpanded) { + final visibleKey = '$_kLibraryItemPrefix:${_LibraryNavSection.visible.name}:$libKey'; + if (_mountedFocusNodeFor(visibleKey) != null) return visibleKey; + if (_hiddenLibrariesExpanded) { + final hiddenKey = '$_kLibraryItemPrefix:${_LibraryNavSection.hidden.name}:$libKey'; + if (_mountedFocusNodeFor(hiddenKey) != null) return hiddenKey; + } + } + return _kLibraries; + case NavigationTabId.search: + return _kSearch; + case NavigationTabId.downloads: + return _showDownloads ? _kDownloads : null; + case NavigationTabId.settings: + return _kSettings; + case NavigationTabId.liveTv: + return 'liveTv'; } - _focusTracker.restoreFocus(fallbackKey: _kHome); + } + + /// Request focus on [node] and scroll it into view after the next frame. + void _requestFocusAndReveal(FocusNode node) { + node.requestFocus(); + scrollContextToCenter(node.context); } String _serverHeaderFocusKey(_LibraryNavSection section, ServerId serverId) => @@ -497,14 +544,7 @@ class SideNavigationRailState extends State with MountedSetS final nextNode = _focusTracker.nodeFor(focusOrder[nextIndex]); if (nextNode == null) return KeyEventResult.ignored; - nextNode.requestFocus(); - WidgetsBinding.instance.addPostFrameCallback((_) { - if (!mounted) return; - final ctx = nextNode.context; - if (ctx != null) { - Scrollable.ensureVisible(ctx, alignment: 0.5, duration: const Duration(milliseconds: 200)); - } - }); + _requestFocusAndReveal(nextNode); return KeyEventResult.handled; } diff --git a/test/widgets/side_navigation_rail_test.dart b/test/widgets/side_navigation_rail_test.dart index 72204338..047e42fe 100644 --- a/test/widgets/side_navigation_rail_test.dart +++ b/test/widgets/side_navigation_rail_test.dart @@ -75,12 +75,19 @@ AnimatedOpacity _railSurfaceOpacity(WidgetTester tester) { Future _pumpBasicRail( WidgetTester tester, { GlobalKey? sideNavKey, + NavigationTabId selectedTab = NavigationTabId.discover, + String? selectedLibraryKey, + List libraries = const [], bool isSidebarFocused = false, bool alwaysExpanded = false, + double? height, }) async { await SettingsService.getInstance(); final librariesProvider = LibrariesProvider(); + if (libraries.isNotEmpty) { + await librariesProvider.updateLibraryOrder(libraries); + } addTearDown(librariesProvider.dispose); final hiddenLibrariesProvider = HiddenLibrariesProvider(); @@ -92,6 +99,16 @@ Future _pumpBasicRail( final multiServerProvider = MultiServerProvider(manager, aggregation); addTearDown(multiServerProvider.dispose); + final rail = SideNavigationRail( + key: sideNavKey, + selectedTab: selectedTab, + selectedLibraryKey: selectedLibraryKey, + isSidebarFocused: isSidebarFocused, + alwaysExpanded: alwaysExpanded, + onDestinationSelected: (_) {}, + onLibrarySelected: (_) {}, + ); + await tester.pumpWidget( TranslationProvider( child: MultiProvider( @@ -103,14 +120,7 @@ Future _pumpBasicRail( child: MaterialApp( theme: ThemeData(extensions: const [_testTokens]), home: Scaffold( - body: SideNavigationRail( - key: sideNavKey, - selectedTab: NavigationTabId.discover, - isSidebarFocused: isSidebarFocused, - alwaysExpanded: alwaysExpanded, - onDestinationSelected: (_) {}, - onLibrarySelected: (_) {}, - ), + body: height == null ? rail : SizedBox(height: height, child: rail), ), ), ), @@ -262,6 +272,45 @@ void main() { expect(_railItemDecoration(tester, selectedItem)?.color, isNull); }); + testWidgets('focusActiveItem focuses selected library and scrolls it into view', (tester) async { + final sideNavKey = GlobalKey(); + final libraries = List.generate( + 18, + (index) => _library(id: '$index', title: 'Library $index', serverId: ServerId('server'), serverName: 'Server'), + ); + final targetLibrary = libraries.last; + + await _pumpBasicRail( + tester, + sideNavKey: sideNavKey, + selectedTab: NavigationTabId.libraries, + selectedLibraryKey: targetLibrary.globalKey, + libraries: libraries, + isSidebarFocused: true, + alwaysExpanded: true, + height: 260, + ); + + final scrollable = find.descendant(of: find.byType(SideNavigationRail), matching: find.byType(Scrollable)).first; + final scrollableState = tester.state(scrollable); + expect(scrollableState.position.pixels, 0); + + sideNavKey.currentState!.focusActiveItem(); + await tester.pump(); + await tester.pumpAndSettle(); + + final targetItemFinder = find.widgetWithText(NavigationRailItem, targetLibrary.title); + expect(targetItemFinder, findsOneWidget); + final targetItem = tester.widget(targetItemFinder); + expect(targetItem.focusNode.hasFocus, isTrue); + expect(scrollableState.position.pixels, greaterThan(0)); + + final railRect = tester.getRect(find.byType(SideNavigationRail)); + final targetRect = tester.getRect(find.text(targetLibrary.title)); + expect(targetRect.top, greaterThanOrEqualTo(railRect.top)); + expect(targetRect.bottom, lessThanOrEqualTo(railRect.bottom)); + }); + testWidgets('reports interaction expansion for shell content push', (tester) async { await SettingsService.getInstance();