diff --git a/lib/screens/main_shell.dart b/lib/screens/main_shell.dart index 13866453..e96168c6 100644 --- a/lib/screens/main_shell.dart +++ b/lib/screens/main_shell.dart @@ -29,6 +29,7 @@ import 'package:spotiflac_android/widgets/update_dialog.dart'; import 'package:spotiflac_android/widgets/animation_utils.dart'; import 'package:spotiflac_android/widgets/settings_group.dart'; import 'package:spotiflac_android/widgets/mini_player.dart'; +import 'package:spotiflac_android/widgets/selection_bottom_bar.dart'; import 'package:spotiflac_android/utils/logger.dart'; final _log = AppLogger('MainShell'); @@ -808,107 +809,114 @@ class _MainShellState extends ConsumerState ), ); - return BackButtonListener( - onBackButtonPressed: () async { - await _handleBackPress(); - return true; - }, - child: Scaffold( - extendBody: true, - // The page view keeps one element across the rail<->bar structure - // swap via _pageViewKey; without it a rotation past the 600dp - // breakpoint remounts the PageView and snaps back to the first tab. - body: useNavigationRail - ? Row( - children: [ - SafeArea( - right: false, - bottom: false, - // The rail needs ~300dp of height for four labeled - // destinations; on short viewports (landscape phone with - // the mini player showing) it must scroll, not overflow. - child: LayoutBuilder( - builder: (context, constraints) => SingleChildScrollView( - child: ConstrainedBox( - constraints: BoxConstraints( - minHeight: constraints.maxHeight, - ), - child: IntrinsicHeight( - child: NavigationRail( - selectedIndex: _currentIndex.clamp(0, maxIndex), - onDestinationSelected: _onNavTap, - labelType: NavigationRailLabelType.all, - backgroundColor: Theme.of( - context, - ).colorScheme.surfaceContainer, - destinations: [ - for (final destination in destinations) - NavigationRailDestination( - icon: destination.icon, - selectedIcon: destination.selectedIcon, - label: Text(destination.label), + return SelectionOverlayHost( + child: BackButtonListener( + onBackButtonPressed: () async { + await _handleBackPress(); + return true; + }, + child: Scaffold( + extendBody: true, + // The page view keeps one element across the rail<->bar structure + // swap via _pageViewKey; without it a rotation past the 600dp + // breakpoint remounts the PageView and snaps back to the first tab. + body: useNavigationRail + ? Row( + children: [ + SafeArea( + right: false, + bottom: false, + // The rail needs ~300dp of height for four labeled + // destinations; on short viewports (landscape phone with + // the mini player showing) it must scroll, not overflow. + child: LayoutBuilder( + builder: (context, constraints) => + SingleChildScrollView( + child: ConstrainedBox( + constraints: BoxConstraints( + minHeight: constraints.maxHeight, + ), + child: IntrinsicHeight( + child: NavigationRail( + selectedIndex: _currentIndex.clamp( + 0, + maxIndex, + ), + onDestinationSelected: _onNavTap, + labelType: NavigationRailLabelType.all, + backgroundColor: Theme.of( + context, + ).colorScheme.surfaceContainer, + destinations: [ + for (final destination in destinations) + NavigationRailDestination( + icon: destination.icon, + selectedIcon: + destination.selectedIcon, + label: Text(destination.label), + ), + ], ), - ], + ), + ), ), + ), + ), + const VerticalDivider(width: 1), + Expanded(child: pageView), + ], + ) + : pageView, + bottomNavigationBar: Builder( + builder: (context) { + final bottomBar = Column( + mainAxisSize: MainAxisSize.min, + children: [ + const MiniPlayer(), + if (!useNavigationRail) + DecoratedBox( + position: DecorationPosition.foreground, + decoration: BoxDecoration( + border: Border( + top: BorderSide( + color: Theme.of( + context, + ).colorScheme.outlineVariant.withValues(alpha: 0.5), ), ), ), - ), - ), - const VerticalDivider(width: 1), - Expanded(child: pageView), - ], - ) - : pageView, - bottomNavigationBar: Builder( - builder: (context) { - final bottomBar = Column( - mainAxisSize: MainAxisSize.min, - children: [ - const MiniPlayer(), - if (!useNavigationRail) - DecoratedBox( - position: DecorationPosition.foreground, - decoration: BoxDecoration( - border: Border( - top: BorderSide( - color: Theme.of( - context, - ).colorScheme.outlineVariant.withValues(alpha: 0.5), - ), + child: NavigationBar( + selectedIndex: _currentIndex.clamp(0, maxIndex), + onDestinationSelected: _onNavTap, + animationDuration: const Duration(milliseconds: 500), + elevation: 0, + height: 64, + backgroundColor: settingsGroupColor( + context, + ).withValues(alpha: 0.72), + destinations: destinations, ), ), - child: NavigationBar( - selectedIndex: _currentIndex.clamp(0, maxIndex), - onDestinationSelected: _onNavTap, - animationDuration: const Duration(milliseconds: 500), - elevation: 0, - height: 64, - backgroundColor: settingsGroupColor( - context, - ).withValues(alpha: 0.72), - destinations: destinations, - ), - ), - ], - ); - // The backdrop blur re-filters everything scrolling underneath on - // every frame; low-end devices get an opaque base instead unless - // the user forces blur on in appearance settings. - if (!ref.watch(backdropBlurEnabledProvider)) { - return ColoredBox( - color: settingsGroupColor(context), - child: bottomBar, + ], ); - } - return ClipRect( - child: BackdropFilter( - filter: ImageFilter.blur(sigmaX: 10, sigmaY: 10), - blendMode: BlendMode.src, - child: bottomBar, - ), - ); - }, + // The backdrop blur re-filters everything scrolling underneath on + // every frame; low-end devices get an opaque base instead unless + // the user forces blur on in appearance settings. + if (!ref.watch(backdropBlurEnabledProvider)) { + return ColoredBox( + color: settingsGroupColor(context), + child: bottomBar, + ); + } + return ClipRect( + child: BackdropFilter( + filter: ImageFilter.blur(sigmaX: 10, sigmaY: 10), + blendMode: BlendMode.src, + child: bottomBar, + ), + ); + }, + ), ), ), ); diff --git a/lib/widgets/selection_bottom_bar.dart b/lib/widgets/selection_bottom_bar.dart index 43f3120d..49ce6bb1 100644 --- a/lib/widgets/selection_bottom_bar.dart +++ b/lib/widgets/selection_bottom_bar.dart @@ -3,24 +3,110 @@ import 'package:spotiflac_android/theme/app_tokens.dart'; import 'package:spotiflac_android/widgets/app_bottom_sheet.dart'; import 'package:spotiflac_android/l10n/l10n.dart'; -/// Mounts a selection bar in the **root** overlay. -/// -/// The bar has to float above the shell's navigation bar, which lives outside -/// the per-tab navigators, so an in-tree `Positioned` inside the screen is not -/// enough. Five screens each solved this differently (two hand-rolled -/// `OverlayEntry` blocks, two `AnimatedPositioned` stacks and one bespoke -/// `Positioned` container); they now all go through this controller, which also -/// guarantees the same entrance animation everywhere. -class SelectionOverlayController { - OverlayEntry? _entry; +/// Hosts selection bars above the shell navigation while keeping modal routes +/// above the bar. A raw entry in the root [Overlay] stays above routes pushed +/// later, which made download pickers appear behind the selection toolbar. +class SelectionOverlayHost extends StatefulWidget { + const SelectionOverlayHost({super.key, required this.child}); + + final Widget child; + + static _SelectionOverlayHostState? _maybeOf(BuildContext context) => + context.findAncestorStateOfType<_SelectionOverlayHostState>(); + + @override + State createState() => _SelectionOverlayHostState(); +} + +class _SelectionOverlayHostState extends State { + SelectionOverlayController? _owner; WidgetBuilder? _builder; - bool get isVisible => _entry != null; + bool owns(SelectionOverlayController owner) => + _owner == owner && _builder != null; + + void show(SelectionOverlayController owner, WidgetBuilder builder) { + if (!mounted) return; + setState(() { + _owner = owner; + _builder = builder; + }); + } + + void hide(SelectionOverlayController owner) { + if (!mounted || _owner != owner) return; + setState(() { + _owner = null; + _builder = null; + }); + } + + @override + Widget build(BuildContext context) { + final owner = _owner; + final builder = _builder; + // A root modal makes the shell route non-current. Removing the toolbar + // while that route is covered both prevents visual overlap and avoids the + // hidden toolbar intercepting taps during the modal transition. The stored + // builder is kept so the selection returns if the modal is dismissed. + final routeIsCurrent = ModalRoute.of(context)?.isCurrent ?? true; + return Stack( + fit: StackFit.expand, + children: [ + widget.child, + if (routeIsCurrent && owner != null && builder != null) + Positioned( + left: 0, + right: 0, + bottom: 0, + child: KeyedSubtree( + key: ObjectKey(owner), + child: AnimatedSelectionBottomBar( + child: Material( + color: Colors.transparent, + child: Builder(builder: builder), + ), + ), + ), + ), + ], + ); + } +} + +/// Mounts a selection bar in [SelectionOverlayHost]. Screens rendered outside +/// the main shell (including isolated widget tests) retain a root-overlay +/// fallback. +class SelectionOverlayController { + OverlayEntry? _entry; + _SelectionOverlayHostState? _host; + WidgetBuilder? _builder; + + bool get isVisible => _entry != null || (_host?.owns(this) ?? false); /// Shows the bar, or rebuilds it in place when already visible so the /// entrance animation does not replay on every selection change. void show(BuildContext context, WidgetBuilder builder) { + if (!TickerMode.valuesOf(context).enabled) { + hide(); + return; + } + _builder = builder; + final host = SelectionOverlayHost._maybeOf(context); + if (host != null) { + _entry?.remove(); + _entry = null; + if (_host != null && _host != host) { + _host!.hide(this); + } + _host = host; + host.show(this, builder); + return; + } + + _host?.hide(this); + _host = null; if (_entry != null) { _entry!.markNeedsBuild(); return; @@ -42,6 +128,8 @@ class SelectionOverlayController { } void hide() { + _host?.hide(this); + _host = null; _entry?.remove(); _entry = null; _builder = null; diff --git a/test/design_system_test.dart b/test/design_system_test.dart index 8bb7ed1b..c5088bd4 100644 --- a/test/design_system_test.dart +++ b/test/design_system_test.dart @@ -10,6 +10,7 @@ import 'package:spotiflac_android/widgets/app_bottom_sheet.dart'; import 'package:spotiflac_android/widgets/app_search_field.dart'; import 'package:spotiflac_android/widgets/app_sliver_header.dart'; import 'package:spotiflac_android/widgets/collection_scaffold.dart'; +import 'package:spotiflac_android/widgets/selection_bottom_bar.dart'; import 'package:spotiflac_android/widgets/settings_group.dart'; import 'package:spotiflac_android/widgets/track_card.dart'; @@ -476,6 +477,73 @@ void main() { 'they animate identically and clear the shell navigation bar.', ); }); + + testWidgets('shell host keeps modal routes above the selection bar', ( + tester, + ) async { + await tester.binding.setSurfaceSize(const Size(430, 900)); + addTearDown(() => tester.binding.setSurfaceSize(null)); + + final controller = SelectionOverlayController(); + addTearDown(controller.dispose); + BuildContext? pageContext; + var selectionTaps = 0; + var sheetTaps = 0; + + await tester.pumpWidget( + MaterialApp( + home: SelectionOverlayHost( + child: Builder( + builder: (context) { + pageContext = context; + return const Scaffold(body: SizedBox.expand()); + }, + ), + ), + ), + ); + + controller.show( + pageContext!, + (_) => GestureDetector( + key: const ValueKey('selection-hit-target'), + behavior: HitTestBehavior.opaque, + onTap: () => selectionTaps++, + child: const SizedBox(height: 200), + ), + ); + await tester.pumpAndSettle(); + + final sheetClosed = showModalBottomSheet( + context: pageContext!, + useRootNavigator: true, + enableDrag: false, + builder: (_) => GestureDetector( + key: const ValueKey('sheet-hit-target'), + behavior: HitTestBehavior.opaque, + onTap: () => sheetTaps++, + child: const SizedBox(height: 300), + ), + ); + await tester.pumpAndSettle(); + + expect(find.byKey(const ValueKey('sheet-hit-target')), findsOneWidget); + expect(find.byKey(const ValueKey('selection-hit-target')), findsNothing); + expect(selectionTaps, 0); + expect(sheetTaps, 0); + + Navigator.of(pageContext!, rootNavigator: true).pop(); + await tester.pumpAndSettle(); + await sheetClosed; + + expect( + find.byKey(const ValueKey('selection-hit-target')), + findsOneWidget, + ); + await tester.tap(find.byKey(const ValueKey('selection-hit-target'))); + await tester.pump(); + expect(selectionTaps, 1); + }); }); group('CollectionScaffold', () {