From ba057015f44885bdf35e88b9ad9970f1d4812807 Mon Sep 17 00:00:00 2001 From: Fabian Freund Date: Thu, 8 Jan 2026 14:29:00 +0100 Subject: [PATCH] update ui logic to replace scaffold --- .../browser/presentation/screens/browser.dart | 197 +++++++++--------- .../widgets/tab_view/tab_view_header.dart | 25 +-- .../utils/profile_switch_handler.dart | 5 +- app/lib/utils/ui_helper.dart | 47 +++-- 4 files changed, 139 insertions(+), 135 deletions(-) diff --git a/app/lib/features/geckoview/features/browser/presentation/screens/browser.dart b/app/lib/features/geckoview/features/browser/presentation/screens/browser.dart index fe9738e2..db52b55d 100644 --- a/app/lib/features/geckoview/features/browser/presentation/screens/browser.dart +++ b/app/lib/features/geckoview/features/browser/presentation/screens/browser.dart @@ -300,19 +300,24 @@ class BrowserScreen extends HookConsumerWidget { [], ); - final sheetController = useState(null); - final pointerMoveEventsController = useStreamController(); + // Watch sheet state for rendering in Stack + final displayedSheet = ref.watch(bottomSheetControllerProvider); + final sheetDisplayed = displayedSheet != null; + // Compute visibility states for toolbars final appBarVisible = useValueListenable(displayAppBar); final topDismissed = ref.watch(tabBarDismissableControllerProvider); // Toolbar is visible when: sheet is shown OR (not fullscreen AND app bar visible) final topToolbarVisible = - sheetController.value != null || (!tabInFullScreen && !topDismissed); + sheetDisplayed || (!tabInFullScreen && !topDismissed); final bottomToolbarVisible = - sheetController.value != null || (!tabInFullScreen && appBarVisible); + sheetDisplayed || (!tabInFullScreen && appBarVisible); + + // Calculate relative safe area for sheet max size + final relativeSafeArea = MediaQuery.of(context).relativeSafeArea(); // Calculate bottom toolbar size for FAB positioning final bottomAppBarSize = BrowserBottomAppBar( @@ -340,15 +345,28 @@ class BrowserScreen extends HookConsumerWidget { Positioned.fill( child: _Browser( overlayController: overlayController, - sheetController: sheetController, displayAppBar: displayAppBar, tabInFullScreen: tabInFullScreen, pointerMoveEventSink: pointerMoveEventsController.sink, - bottomAppBarSize: bottomAppBarSize, + sheetDisplayed: sheetDisplayed, ), ), - // Layer 1: Bottom Toolbar (overlay, slides in/out) + // Layer 1: Sheet (when displayed) - positioned above toolbar + if (sheetDisplayed) + Positioned( + left: 0, + right: 0, + top: 0, + bottom: bottomAppBarSize.height, + child: _SheetContainer( + displayedSheet: displayedSheet, + relativeSafeArea: relativeSafeArea, + bottomAppBarSize: bottomAppBarSize, + ), + ), + + // Layer 2: Bottom Toolbar (overlay, slides in/out) - above sheet Positioned( left: 0, right: 0, @@ -367,7 +385,7 @@ class BrowserScreen extends HookConsumerWidget { ), ), - // Layer 2: Top Toolbar (overlay, slides in/out) - only when position is top + // Layer 3: Top Toolbar (overlay, slides in/out) - only when position is top if (tabBarPosition == TabBarPosition.top) Positioned( left: 0, @@ -387,10 +405,14 @@ class BrowserScreen extends HookConsumerWidget { ), ), - // Layer 3: FAB (positioned above bottom toolbar) - Positioned( + // Layer 4: FAB (animates position with toolbar visibility) + AnimatedPositioned( + duration: _AnimatedToolbar._kAnimationDuration, + curve: Curves.easeInOutQuart, right: 16, - bottom: bottomAppBarSize.height + 16, + bottom: bottomToolbarVisible + ? bottomAppBarSize.height + 16 + : 16 + MediaQuery.of(context).padding.bottom, child: const BrowserFab(), ), ], @@ -401,24 +423,80 @@ class BrowserScreen extends HookConsumerWidget { } } +/// Container widget for bottom sheets - renders in Stack for proper layering. +class _SheetContainer extends HookConsumerWidget { + final Sheet displayedSheet; + final double relativeSafeArea; + final Size bottomAppBarSize; + + const _SheetContainer({ + required this.displayedSheet, + required this.relativeSafeArea, + required this.bottomAppBarSize, + }); + + @override + Widget build(BuildContext context, WidgetRef ref) { + bool dismissOnThreshold(DraggableScrollableNotification notification) { + if (notification.extent <= 0.1) { + logger.i('Dismissing sheet, reached min extend'); + ref.read(bottomSheetControllerProvider.notifier).requestDismiss(); + return true; + } + return false; + } + + return GestureDetector( + onTap: () { + // Dismiss sheet when tapping outside + ref.read(bottomSheetControllerProvider.notifier).requestDismiss(); + }, + child: Container( + color: Colors.black54, // Scrim + child: GestureDetector( + onTap: () {}, // Prevent tap from propagating to parent + child: Align( + alignment: Alignment.bottomCenter, + child: switch (displayedSheet) { + ViewTabsSheet() => + NotificationListener( + key: UniqueKey(), + onNotification: dismissOnThreshold, + child: _ViewTabsSheet(maxChildSize: relativeSafeArea), + ), + final EditUrlSheet parameter => + NotificationListener( + key: UniqueKey(), + onNotification: dismissOnThreshold, + child: _ViewUrlSheet( + initialTabState: parameter.tabState, + maxChildSize: relativeSafeArea, + bottomAppBarSize: bottomAppBarSize, + ), + ), + }, + ), + ), + ), + ); + } +} + class _Browser extends HookConsumerWidget { Duration get _backButtonPressTimeout => const Duration(seconds: 2); final OverlayPortalController overlayController; - final ValueNotifier sheetController; final ValueNotifier displayAppBar; final StreamSink pointerMoveEventSink; - final Size bottomAppBarSize; - final bool tabInFullScreen; + final bool sheetDisplayed; const _Browser({ required this.overlayController, - required this.sheetController, required this.displayAppBar, required this.tabInFullScreen, required this.pointerMoveEventSink, - required this.bottomAppBarSize, + required this.sheetDisplayed, }); @override @@ -427,88 +505,6 @@ class _Browser extends HookConsumerWidget { final overlayBuilder = ref.watch(overlayControllerProvider); - ref.listen(bottomSheetControllerProvider, (previous, next) { - if (!context.mounted) { - logger.e('Cannot show sheet, context not mounted'); - return; - } - - WidgetsBinding.instance.addPostFrameCallback((_) { - if (!context.mounted) { - logger.e('Cannot show sheet, context not mounted (post frame)'); - return; - } - - // Close existing sheet - if (sheetController.value != null) { - try { - final existingController = sheetController.value!; - sheetController.value = null; - existingController.close(); - } catch (e) { - logger.e('Error closing existing sheet', error: e); - } - } - - // Show new sheet - if (next != null) { - try { - final relativeSafeArea = MediaQuery.of(context).relativeSafeArea(); - - final controller = Scaffold.of(context).showBottomSheet((context) { - logger.i( - 'Building bottom sheet, relativeSafeArea: $relativeSafeArea, mounted: ${context.mounted}', - ); - - bool dismissOnThreshold( - DraggableScrollableNotification notification, - ) { - if (notification.extent <= 0.1) { - logger.i('Dismissing sheet, reached min extend'); - ref - .read(bottomSheetControllerProvider.notifier) - .requestDismiss(); - return true; - } - return false; - } - - final sheet = switch (next) { - ViewTabsSheet() => - NotificationListener( - key: UniqueKey(), - onNotification: dismissOnThreshold, - child: _ViewTabsSheet(maxChildSize: relativeSafeArea), - ), - final EditUrlSheet parameter => - NotificationListener( - key: UniqueKey(), - onNotification: dismissOnThreshold, - child: _ViewUrlSheet( - initialTabState: parameter.tabState, - maxChildSize: relativeSafeArea, - bottomAppBarSize: bottomAppBarSize, - ), - ), - }; - - return sheet; - }); - - unawaited( - controller.closed.whenComplete(() { - ref.read(bottomSheetControllerProvider.notifier).closed(next); - }), - ); - - sheetController.value = controller; - } catch (e) { - debugPrint('Failed to show bottom sheet: $e'); - } - } - }); - }); - return DragTarget( onMove: (details) { ref @@ -538,7 +534,7 @@ class _Browser extends HookConsumerWidget { return overlayBuilder!.call(context); }, child: Listener( - onPointerDown: sheetController.value != null + onPointerDown: sheetDisplayed ? (_) { ref .read(bottomSheetControllerProvider.notifier) @@ -683,7 +679,8 @@ class _BrowserView extends StatelessWidget { SafeArea( top: !isFullscreen, right: !isFullscreen, - bottom: !isFullscreen, + // Bottom SafeArea is handled by the overlay toolbar (BottomAppBar) + bottom: false, left: !isFullscreen, child: Stack( children: [ diff --git a/app/lib/features/geckoview/features/browser/presentation/widgets/tab_view/tab_view_header.dart b/app/lib/features/geckoview/features/browser/presentation/widgets/tab_view/tab_view_header.dart index f994891d..e37a7ea8 100644 --- a/app/lib/features/geckoview/features/browser/presentation/widgets/tab_view/tab_view_header.dart +++ b/app/lib/features/geckoview/features/browser/presentation/widgets/tab_view/tab_view_header.dart @@ -443,31 +443,18 @@ class TabViewHeader extends HookConsumerWidget { } if (context.mounted) { - ScaffoldMessenger.of( + ui_helper.showInfoMessage( context, - ).showSnackBar( - SnackBar( - content: Text( - shouldReopenTabs - ? 'Container data cleared successfully' - : 'Container data cleared. ${tabs.length} tab(s) closed.', - ), - ), + shouldReopenTabs + ? 'Container data cleared successfully' + : 'Container data cleared. ${tabs.length} tab(s) closed.', ); } } catch (e) { if (context.mounted) { - ScaffoldMessenger.of( + ui_helper.showErrorMessage( context, - ).showSnackBar( - SnackBar( - content: Text( - 'Error clearing data: $e', - ), - backgroundColor: Theme.of( - context, - ).colorScheme.error, - ), + 'Error clearing data: $e', ); } } diff --git a/app/lib/features/user/domain/presentation/utils/profile_switch_handler.dart b/app/lib/features/user/domain/presentation/utils/profile_switch_handler.dart index 8e92ffcd..9237a667 100644 --- a/app/lib/features/user/domain/presentation/utils/profile_switch_handler.dart +++ b/app/lib/features/user/domain/presentation/utils/profile_switch_handler.dart @@ -25,6 +25,7 @@ import 'package:weblibre/core/filesystem.dart'; import 'package:weblibre/domain/entities/profile.dart'; import 'package:weblibre/features/user/domain/repositories/profile.dart'; import 'package:weblibre/utils/exit_app.dart'; +import 'package:weblibre/utils/ui_helper.dart' as ui_helper; /// Handles the profile switching flow with confirmation dialog and cache clearing options. /// @@ -43,9 +44,7 @@ Future handleSwitchProfile( // Don't allow switching to the already active profile if (isSelected) { if (context.mounted) { - ScaffoldMessenger.of(context).showSnackBar( - const SnackBar(content: Text('This profile is already active')), - ); + ui_helper.showInfoMessage(context, 'This profile is already active'); } return; } diff --git a/app/lib/utils/ui_helper.dart b/app/lib/utils/ui_helper.dart index 255acfe8..ac4ba991 100644 --- a/app/lib/utils/ui_helper.dart +++ b/app/lib/utils/ui_helper.dart @@ -22,21 +22,47 @@ import 'package:nullability/nullability.dart'; import 'package:url_launcher/url_launcher.dart'; import 'package:weblibre/utils/clipboard.dart'; +/// Default bottom margin for floating snackbars to position above toolbar. +/// This accounts for the typical browser toolbar height. +const _kSnackBarBottomMargin = 72.0; + +/// Creates a floating snackbar with proper margin for overlay toolbar layout. +SnackBar _createFloatingSnackBar({ + required Widget content, + Color? backgroundColor, + SnackBarAction? action, + Duration duration = const Duration(seconds: 4), + bool persist = false, +}) { + return SnackBar( + content: content, + backgroundColor: backgroundColor, + action: action, + duration: duration, + persist: persist, + behavior: SnackBarBehavior.floating, + margin: const EdgeInsets.only( + left: 16, + right: 16, + bottom: _kSnackBarBottomMargin, + ), + ); +} + void showErrorMessage(BuildContext context, String message) { - final snackBar = SnackBar( + final snackBar = _createFloatingSnackBar( content: Text( message, style: TextStyle(color: Theme.of(context).colorScheme.error), ), backgroundColor: Theme.of(context).colorScheme.onError, - persist: false, ); ScaffoldMessenger.of(context).showSnackBar(snackBar); } void showInfoMessage(BuildContext context, String message) { - final snackBar = SnackBar(content: Text(message), persist: false); + final snackBar = _createFloatingSnackBar(content: Text(message)); ScaffoldMessenger.of(context).showSnackBar(snackBar); } @@ -46,12 +72,11 @@ void showTabBackButtonMessage( int tabCount, Duration duration, ) { - final snackbar = SnackBar( + final snackbar = _createFloatingSnackBar( content: (tabCount > 1) ? const Text('Navigate BACK again to close current tab') : const Text('Navigate BACK again to exit app'), duration: duration, - persist: false, ); ScaffoldMessenger.of(context) @@ -70,13 +95,12 @@ void showTabOpenedMessage( null => 'New tab opened in background', }; - final snackBar = SnackBar( + final snackBar = _createFloatingSnackBar( content: Text(message), action: onShow.mapNotNull( (onPressed) => SnackBarAction(label: 'Show', onPressed: onPressed), ), duration: duration, - persist: false, ); ScaffoldMessenger.of(context).showSnackBar(snackBar); @@ -90,7 +114,7 @@ Future showSuggestNewTabMessage( final clipboardUrl = await tryGetUriFromClipboard(); if (clipboardUrl != null) { - final snackBar = SnackBar( + final snackBar = _createFloatingSnackBar( content: const Text('Want to open link from clipboard?'), action: SnackBarAction( label: 'Open', @@ -99,7 +123,6 @@ Future showSuggestNewTabMessage( }, ), duration: duration, - persist: false, ); if (context.mounted) { @@ -121,13 +144,12 @@ void showTabSwitchMessage( null => 'New tab opened', }; - final snackBar = SnackBar( + final snackBar = _createFloatingSnackBar( content: Text(message), action: onSwitch.mapNotNull( (onPressed) => SnackBarAction(label: 'Switch', onPressed: onPressed), ), duration: duration, - persist: false, ); ScaffoldMessenger.of(context).showSnackBar(snackBar); @@ -165,13 +187,12 @@ void showTabUndoClose( }) { ScaffoldMessenger.of(context).clearSnackBars(); - final snackBar = SnackBar( + final snackBar = _createFloatingSnackBar( content: (count > 1) ? Text('$count Tabs closed') : const Text('Tab closed'), action: SnackBarAction(label: 'Undo', onPressed: onUndo), duration: duration, - persist: false, ); ScaffoldMessenger.of(context).showSnackBar(snackBar);