woltapp / wolt_modal_sheet

This package provides a responsive modal with multiple pages, motion animation for page transitions, and scrollable content within each page.
MIT License
504 stars 64 forks source link

FloatingActionButton becomes unresponsive with hasTopBarLayer: false in WoltModalSheetPage #105

Closed motucraft closed 11 months ago

motucraft commented 11 months ago

Bug report

Describe the bug When using WoltModalSheetPage.withCustomSliverList with hasTopBarLayer: false, the FloatingActionButton in the upper left corner becomes unresponsive to taps. Removing or commenting out hasTopBarLayer: false makes the button responsive again. This behavior suggests an issue with how the hasTopBarLayer property interacts with the button's touch events.

Steps to reproduce

Steps to reproduce the behavior:

  1. Run the provided Flutter application.
  2. On the home screen, tap on the 'Modal Bottom Sheet' button to open the modal sheet.
  3. Try to tap the FloatingActionButton in the upper left corner of the modal sheet.
  4. Observe that the button is unresponsive.
  5. Comment out hasTopBarLayer: false in WoltModalSheetPage.withCustomSliverList.
  6. Repeat steps 1-3.
  7. Observe that the button is now responsive.

Expected behavior

The FloatingActionButton should be responsive to taps regardless of the hasTopBarLayer property's setting. The button should function correctly whether hasTopBarLayer is set to true, false, or is commented out.


Additional context

pubspec.yaml ```yaml name: wolt_modal_sheet_bug description: A new Flutter project. publish_to: 'none' version: 1.0.0+1 environment: sdk: '>=3.1.4 <4.0.0' dependencies: flutter: sdk: flutter cupertino_icons: ^1.0.2 hooks_riverpod: ^2.4.8 flutter_hooks: ^0.20.3 riverpod_annotation: ^2.3.2 wolt_modal_sheet: ^0.1.4 sliver_tools: ^0.2.12 go_router: ^12.1.1 cached_network_image: ^3.3.0 dev_dependencies: flutter_test: sdk: flutter flutter_lints: ^2.0.0 riverpod_generator: ^2.3.8 build_runner: ^2.4.6 custom_lint: ^0.5.7 riverpod_lint: ^2.3.6 flutter: uses-material-design: true ```
Sample Code ```dart import 'package:cached_network_image/cached_network_image.dart'; import 'package:flutter/material.dart'; import 'package:flutter_hooks/flutter_hooks.dart'; import 'package:go_router/go_router.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:riverpod_annotation/riverpod_annotation.dart'; import 'package:sliver_tools/sliver_tools.dart'; import 'package:wolt_modal_sheet/wolt_modal_sheet.dart'; part 'main.g.dart'; void main() { runApp(const ProviderScope(child: MyApp())); } class MyApp extends ConsumerWidget { const MyApp({super.key}); @override Widget build(BuildContext context, WidgetRef ref) { return MaterialApp.router(routerConfig: ref.watch(routerProvider)); } } class Home extends HookWidget { const Home({super.key}); @override Widget build(BuildContext context) { final pageBuilderNotifier = useValueNotifier((context) => []); return Scaffold( body: Center( child: ElevatedButton( onPressed: () { pageBuilderNotifier.value = (context) => [ WoltModalSheetPage.withCustomSliverList( hasTopBarLayer: false, sliverList: SliverStack( insetOnOverlap: true, children: [ SliverList( delegate: SliverChildListDelegate( [ CachedNetworkImage( imageUrl: 'https://consumer-static-assets.wolt.com/og_image_mall_web.jpg', ), const Center( child: Text('Thanks for grateful package', style: TextStyle(fontSize: 24)), ), ], ), ), SliverPositioned( left: 16, top: 16, child: FloatingActionButton( onPressed: () { print('======= onPressed ======='); context.pop(); }, child: const Icon(Icons.close, color: Colors.white, size: 30), ), ), ], ), ), ]; context.push('/bottom-sheet', extra: pageBuilderNotifier); }, child: const Text('Modal Bottom Sheet'), ), ), ); } } class SheetPage extends Page { final ValueNotifier pageListBuilderNotifier; final ValueNotifier? pageIndexNotifier; const SheetPage({ required this.pageListBuilderNotifier, this.pageIndexNotifier, }); @override Route createRoute(BuildContext context) { return WoltModalSheetRoute( pageListBuilderNotifier: pageListBuilderNotifier, pageIndexNotifier: pageIndexNotifier, onModalDismissedWithBarrierTap: () => context.pop(), routeSettings: this, ); } } @riverpod GoRouter router(RouterRef ref) { final router = GoRouter( routes: [ GoRoute( path: '/', pageBuilder: (_, __) => const MaterialPage(child: Home()), ), GoRoute( path: '/bottom-sheet', pageBuilder: (_, state) { final extra = state.extra!; if (extra is! ValueNotifier) { throw UnsupportedError('Unsupported parameter. extra=$extra'); } return SheetPage(pageListBuilderNotifier: extra); }, ), ], ); ref.onDispose(router.dispose); return router; } ```

motucraft commented 11 months ago

I found that setting enableDrag: false makes the FloatingActionButton responsive to taps. However, this is not a viable solution for me as the drag functionality is also required for my use case. (Additionally, I would like to note that I am not planning to use the heroImage property as it results in the images being cropped, which is not suitable for my application's requirements. This further limits my options for working around the issue with the FloatingActionButton.)

ulusoyca commented 11 months ago

Thanks for reporting the issue. This is a valid bug, and a draft PR is created to fix this. Hopefully, we will make a new release during the week.

motucraft commented 11 months ago

Your prompt action is greatly appreciated.

motucraft commented 11 months ago

@ulusoyca Will the resolution of this Issue take more time?

ulusoyca commented 11 months ago

Hi @motucraft After going through careful consideration, I realized that this looks a bit difficult to resolve.

Have you considered using the leadingNavBarWidget instead of SliverPositioned? It should do the same thing. You can also utilize heroImage field instead of placing it in a Sliver.

Here is why we need to bloc the Gesture. There is a scrollable sliver list in the index z = 0. The drag should disable the propagation going down.

Semantics(
      label: MaterialLocalizations.of(context).modalBarrierDismissLabel,
      container: true,
      child: SizedBox.square(
        dimension: _minInteractiveDimension,
        // By setting behavior to HitTestBehavior.opaque, the GestureDetector will capture touch
        // events even if its child (the drag handle) isn't the exact size of the gesture.
        // This will prevent the scrollable content below from receiving the touch events,
        // effectively allowing the handler to capture drag gestures.
        child: GestureDetector(
          behavior: HitTestBehavior.opaque,
          child: Align(
            alignment: Alignment.topCenter,
            child: Container(
              margin: const EdgeInsets.only(top: 8.0),
              height: handleSize.height,
              width: handleSize.width,
              decoration: BoxDecoration(
                borderRadius: BorderRadius.circular(handleSize.height / 2),
                color: handleColor,
              ),
            ),
          ),
        ),
      ),
    );
motucraft commented 11 months ago

@ulusoyca Using leadingNavBarWidget results in an empty space above the image as shown in the attached picture.

Regarding heroImage, won’t the image be cropped? I want to always display the entire image, but is that possible with heroImage?

ulusoyca commented 11 months ago

@ulusoyca Using leadingNavBarWidget results in an empty space above the image as shown in the attached picture.

Regarding heroImage, won’t the image be cropped? I want to always display the entire image, but is that possible with heroImage?

Yes this is the point of the hero image field. Please try it. Here is an example how you can use it. It should be used in this case. Please let me know if this answers your needs.

motucraft commented 11 months ago

@ulusoyca I tried using the heroImage. As shown below, it is clear that the top and bottom are being cropped. I am using BoxFit.cover for the heroImage.

Does this differ from what you expected? Are you suggesting that I should increase the heroImageHeight in WoltModalSheetDefaultThemeData?

use SliverStack version use heroImage version
Code for using hero image ```dart import 'package:cached_network_image/cached_network_image.dart'; import 'package:flutter/material.dart'; import 'package:flutter_hooks/flutter_hooks.dart'; import 'package:go_router/go_router.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:riverpod_annotation/riverpod_annotation.dart'; import 'package:wolt_modal_sheet/wolt_modal_sheet.dart'; part 'main.g.dart'; void main() { runApp(const ProviderScope(child: MyApp())); } class MyApp extends ConsumerWidget { const MyApp({super.key}); @override Widget build(BuildContext context, WidgetRef ref) { return MaterialApp.router( routerConfig: ref.watch(routerProvider), theme: ThemeData(useMaterial3: false), ); } } class Home extends HookWidget { const Home({super.key}); @override Widget build(BuildContext context) { final pageBuilderNotifier = useValueNotifier((context) => []); return Scaffold( body: Center( child: ElevatedButton( onPressed: () { pageBuilderNotifier.value = (context) => [ WoltModalSheetPage.withCustomSliverList( hasTopBarLayer: false, leadingNavBarWidget: FloatingActionButton( child: const Icon(Icons.close, color: Colors.white, size: 30), onPressed: () {}, ), heroImage: CachedNetworkImage( imageUrl: 'https://consumer-static-assets.wolt.com/og_image_mall_web.jpg', fit: BoxFit.cover, ), sliverList: SliverList( delegate: SliverChildListDelegate( [ const Center( child: Text('Thanks for grateful package', style: TextStyle(fontSize: 24)), ), ], ), ), ), ]; context.push('/bottom-sheet', extra: pageBuilderNotifier); }, child: const Text('Modal Bottom Sheet'), ), ), ); } } class SheetPage extends Page { final ValueNotifier pageListBuilderNotifier; final ValueNotifier? pageIndexNotifier; const SheetPage({ required this.pageListBuilderNotifier, this.pageIndexNotifier, }); @override Route createRoute(BuildContext context) { return WoltModalSheetRoute( pageListBuilderNotifier: pageListBuilderNotifier, pageIndexNotifier: pageIndexNotifier, onModalDismissedWithBarrierTap: () => context.pop(), routeSettings: this, ); } } @riverpod GoRouter router(RouterRef ref) { final router = GoRouter( routes: [ GoRoute( path: '/', pageBuilder: (_, __) => const MaterialPage(child: Home()), ), GoRoute( path: '/bottom-sheet', pageBuilder: (_, state) { final extra = state.extra!; if (extra is! ValueNotifier) { throw UnsupportedError('Unsupported parameter. extra=$extra'); } return SheetPage(pageListBuilderNotifier: extra); }, ), ], ); ref.onDispose(router.dispose); return router; } ```
ulusoyca commented 11 months ago

@ulusoyca I tried using the heroImage. As shown below, it is clear that the top and bottom are being cropped. I am using BoxFit.cover for the heroImage.

Does this differ from what you expected? Are you suggesting that I should increase the heroImageHeight in WoltModalSheetDefaultThemeData?

use SliverStack version use heroImage version Code for using hero image

I think we are getting closer to your needs. You can play with the BoxFit and the heroImageHeight properties to achieve what you want in my opinion.

motucraft commented 11 months ago

Thanks for your help. I will try.

motucraft commented 11 months ago

I'd like to clarify one thing. Can the heroImageHeight be adjusted for each image? I'm concerned that fixing the height might leave unwanted vertical space.

ulusoyca commented 11 months ago

Yes, every page has a field for heroImageHeight.