From ec57cd436e9a2242857393a08bb097fb93aff3b3 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Sat, 24 Jan 2026 00:22:10 +0100 Subject: [PATCH] fix: browse library chips focus --- lib/focus/focusable_chip_mixin.dart | 12 +- lib/focus/focusable_wrapper.dart | 22 +++- lib/screens/libraries/libraries_screen.dart | 15 +++ .../libraries/tabs/library_browse_tab.dart | 119 +++++++++--------- lib/widgets/focusable_filter_chip.dart | 10 ++ 5 files changed, 112 insertions(+), 66 deletions(-) diff --git a/lib/focus/focusable_chip_mixin.dart b/lib/focus/focusable_chip_mixin.dart index 7ca45f61..0a57e2d0 100644 --- a/lib/focus/focusable_chip_mixin.dart +++ b/lib/focus/focusable_chip_mixin.dart @@ -114,15 +114,15 @@ mixin FocusableChipStateMixin on State { return KeyEventResult.handled; } - // LEFT arrow - if (key.isLeftKey && callbacks.onNavigateLeft != null) { - callbacks.onNavigateLeft!(); + // LEFT arrow - always consume to prevent default focus traversal + if (key.isLeftKey) { + callbacks.onNavigateLeft?.call(); return KeyEventResult.handled; } - // RIGHT arrow - if (key.isRightKey && callbacks.onNavigateRight != null) { - callbacks.onNavigateRight!(); + // RIGHT arrow - always consume to prevent default focus traversal + if (key.isRightKey) { + callbacks.onNavigateRight?.call(); return KeyEventResult.handled; } diff --git a/lib/focus/focusable_wrapper.dart b/lib/focus/focusable_wrapper.dart index 6a667330..bc68dab9 100644 --- a/lib/focus/focusable_wrapper.dart +++ b/lib/focus/focusable_wrapper.dart @@ -246,10 +246,24 @@ class _FocusableWrapperState extends State with SingleTickerPr } } - // Scroll to alignment - Scrollable.ensureVisible( - context, - alignment: widget.scrollAlignment, + // Calculate target scroll offset for the immediate scrollable only. + // This avoids Scrollable.ensureVisible which scrolls ALL ancestor scrollables, + // which can cause issues with nested scroll views (e.g., chips bar scrolling + // out of view when focusing grid items in library browse tab). + final position = scrollable.position; + final currentOffset = position.pixels; + + // Target: item center should be at scrollAlignment of viewport + final targetViewportY = viewportHeight * widget.scrollAlignment; + final scrollDelta = itemVerticalCenter - targetViewportY; + + final targetOffset = (currentOffset + scrollDelta).clamp( + position.minScrollExtent, + position.maxScrollExtent, + ); + + position.animateTo( + targetOffset, duration: const Duration(milliseconds: 200), curve: Curves.easeInOut, ); diff --git a/lib/screens/libraries/libraries_screen.dart b/lib/screens/libraries/libraries_screen.dart index dbfc097a..b91b9b8d 100644 --- a/lib/screens/libraries/libraries_screen.dart +++ b/lib/screens/libraries/libraries_screen.dart @@ -115,6 +115,9 @@ class _LibrariesScreenState extends State final _collectionsTabChipFocusNode = FocusNode(debugLabel: 'tab_chip_collections'); final _playlistsTabChipFocusNode = FocusNode(debugLabel: 'tab_chip_playlists'); + // Scroll controller for the outer CustomScrollView + final ScrollController _outerScrollController = ScrollController(); + @override void initState() { super.initState(); @@ -183,6 +186,11 @@ class _LibrariesScreenState extends State }); } + // Scroll outer view to top to ensure tab content (including chips bar) is visible + if (_outerScrollController.hasClients && _outerScrollController.offset > 0) { + _outerScrollController.jumpTo(0); + } + WidgetsBinding.instance.addPostFrameCallback((_) { if (!mounted) return; @@ -221,6 +229,11 @@ class _LibrariesScreenState extends State /// Focus without additional frame delay (used for retry) void _focusCurrentTabImmediate() { + // Scroll outer view to top to ensure tab content (including chips bar) is visible + if (_outerScrollController.hasClients && _outerScrollController.offset > 0) { + _outerScrollController.jumpTo(0); + } + State? tabState; switch (_tabController.index) { case 0: @@ -313,6 +326,7 @@ class _LibrariesScreenState extends State _tabController.removeListener(_onTabChanged); _tabController.dispose(); _cancelToken?.cancel(); + _outerScrollController.dispose(); _recommendedTabChipFocusNode.dispose(); _browseTabChipFocusNode.dispose(); _collectionsTabChipFocusNode.dispose(); @@ -1077,6 +1091,7 @@ class _LibrariesScreenState extends State return Scaffold( body: CustomScrollView( + controller: _outerScrollController, slivers: [ DesktopSliverAppBar( title: _buildAppBarTitle(visibleLibraries), diff --git a/lib/screens/libraries/tabs/library_browse_tab.dart b/lib/screens/libraries/tabs/library_browse_tab.dart index 0a4cde49..adc6780c 100644 --- a/lib/screens/libraries/tabs/library_browse_tab.dart +++ b/lib/screens/libraries/tabs/library_browse_tab.dart @@ -88,9 +88,13 @@ class _LibraryBrowseTabState extends BaseLibraryTabState _gridItemFocusNodes = {}; + // Scroll controller for the CustomScrollView + final ScrollController _scrollController = ScrollController(); + @override void dispose() { _cancelToken?.cancel(); + _scrollController.dispose(); _groupingChipFocusNode.dispose(); _filtersChipFocusNode.dispose(); _sortChipFocusNode.dispose(); @@ -161,6 +165,9 @@ class _LibraryBrowseTabState extends BaseLibraryTabState _loadContent() async { @@ -501,7 +500,13 @@ class _LibraryBrowseTabState extends BaseLibraryTabState( onNotification: (notification) { if (notification.metrics.pixels >= notification.metrics.maxScrollExtent - 300 && _hasMoreItems && !isLoading) { @@ -551,21 +569,23 @@ class _LibraryBrowseTabState extends BaseLibraryTabState _filters.isNotEmpty && _selectedGrouping != 'folders'; + + /// Whether the sort chip is visible + bool get _isSortChipVisible => _sortOptions.isNotEmpty && _selectedGrouping != 'folders'; + /// Builds the chips bar widget Widget _buildChipsBar() { return Container( - padding: const EdgeInsets.fromLTRB(16, 0, 16, 4), + color: Theme.of(context).scaffoldBackgroundColor, + padding: const EdgeInsets.fromLTRB(16, 8, 16, 8), alignment: Alignment.centerLeft, child: Row( mainAxisSize: MainAxisSize.min, @@ -578,11 +598,16 @@ class _LibraryBrowseTabState extends BaseLibraryTabState _filtersChipFocusNode.requestFocus() + : _isSortChipVisible + ? () => _sortChipFocusNode.requestFocus() + : null, onBack: widget.onBack, ), const SizedBox(width: 8), // Filters chip - if (_filters.isNotEmpty && _selectedGrouping != 'folders') + if (_isFiltersChipVisible) FocusableFilterChip( focusNode: _filtersChipFocusNode, icon: Symbols.filter_alt_rounded, @@ -592,11 +617,13 @@ class _LibraryBrowseTabState extends BaseLibraryTabState _groupingChipFocusNode.requestFocus(), + onNavigateRight: _isSortChipVisible ? () => _sortChipFocusNode.requestFocus() : null, onBack: widget.onBack, ), - if (_filters.isNotEmpty && _selectedGrouping != 'folders') const SizedBox(width: 8), + if (_isFiltersChipVisible) const SizedBox(width: 8), // Sort chip - if (_sortOptions.isNotEmpty && _selectedGrouping != 'folders') + if (_isSortChipVisible) FocusableFilterChip( focusNode: _sortChipFocusNode, icon: Symbols.sort_rounded, @@ -604,6 +631,9 @@ class _LibraryBrowseTabState extends BaseLibraryTabState _filtersChipFocusNode.requestFocus() + : () => _groupingChipFocusNode.requestFocus(), onBack: widget.onBack, ), ], @@ -711,26 +741,3 @@ class _LibraryBrowseTabState extends BaseLibraryTabState 40.0; // Height of chips bar - - @override - double get minExtent => 40.0; // Same as max (no shrinking) - - @override - bool shouldRebuild(covariant _ChipsHeaderDelegate oldDelegate) { - return child != oldDelegate.child; - } -} diff --git a/lib/widgets/focusable_filter_chip.dart b/lib/widgets/focusable_filter_chip.dart index 0cea883b..d8a4a61b 100644 --- a/lib/widgets/focusable_filter_chip.dart +++ b/lib/widgets/focusable_filter_chip.dart @@ -23,6 +23,12 @@ class FocusableFilterChip extends StatefulWidget { /// Called when the user presses UP from this chip. final VoidCallback? onNavigateUp; + /// Called when the user presses LEFT from this chip. + final VoidCallback? onNavigateLeft; + + /// Called when the user presses RIGHT from this chip. + final VoidCallback? onNavigateRight; + /// Called when the user presses BACK from this chip. final VoidCallback? onBack; @@ -34,6 +40,8 @@ class FocusableFilterChip extends StatefulWidget { this.focusNode, this.onNavigateDown, this.onNavigateUp, + this.onNavigateLeft, + this.onNavigateRight, this.onBack, }); @@ -74,6 +82,8 @@ class _FocusableFilterChipState extends State with Focusabl onSelect: widget.onPressed, onNavigateDown: widget.onNavigateDown, onNavigateUp: widget.onNavigateUp, + onNavigateLeft: widget.onNavigateLeft, + onNavigateRight: widget.onNavigateRight, onBack: widget.onBack, ), );