Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ScrollableSheet with maxPosition broken #265

Open
Clon1998 opened this issue Oct 15, 2024 · 6 comments
Open

ScrollableSheet with maxPosition broken #265

Clon1998 opened this issue Oct 15, 2024 · 6 comments
Assignees
Labels
bug Something isn't working P2

Comments

@Clon1998
Copy link

Hey,
when using a ScrollableSheet with maxPosition < proportional(1), there are a few problems:

  1. The sheet will ignore the maxPosition and open fully as long as the scrollable is "large" enough
  2. When starting to scroll, reaching the end of the scrollable will cause the sheet to adjust to the intended max position. However this leads to :
  3. The First few and last few elements of the scrollable cant be scrolled to after the sheet adjusted its size

Example:
I am using GoRouter and this route definition:

GoRoute(
        path: '/modal-sheet',
        name: AppRoute.modal_sheet.name,
        pageBuilder: (context, state) {
          // Use ModalSheetPage to show a modal sheet with Navigator 2.0.
          // It works with any *Sheet provided by this package!
          return ModalSheetPage(
            key: state.pageKey,
            // Enable the swipe-to-dismiss behavior.
            swipeDismissible: true,
            // Use `SwipeDismissSensitivity` to tweak the sensitivity of the swipe-to-dismiss behavior.
            swipeDismissSensitivity: const SwipeDismissSensitivity(
              minFlingVelocityRatio: 2.0,
              minDragDistance: 200.0,
            ),
            child: _MySheet(),
          );
        },
      ),
      ....
      
class _MySheet extends StatelessWidget {
  const _MySheet();

  @override
  Widget build(BuildContext context) {
    // Create a content whatever you want.
    // ScrollableSheet works with any scrollable widget such as
    // ListView, GridView, CustomScrollView, etc.
    final content = ListView.builder(
      itemCount: 50,
      itemBuilder: (context, index) {
        return ListTile(
          title: Text('Item $index'),
        );
      },
    );

    // Just wrap the content in a ScrollableSheet!
    final sheet = ScrollableSheet(
      maxPosition: SheetAnchor.proportional(0.8),
      // initialPosition: SheetAnchor.proportional(0.4),
      child: buildSheetBackground(context, content),
      // Optional: Comment out the following lines to add multiple stop positions.
      //
      // minPosition: const SheetAnchor.proportional(0.2),
      // physics: BouncingSheetPhysics(
      //   parent: SnappingSheetPhysics(
      //     snappingBehavior: SnapToNearest(
      //       snapTo: [
      //         const SheetAnchor.proportional(0.2),
      //         const SheetAnchor.proportional(0.5),
      //         const SheetAnchor.proportional(1),
      //       ],
      //     ),
      //   ),
      // ),
    );

    return SafeArea(bottom: false, child: sheet);
  }

  Widget buildSheetBackground(BuildContext context, Widget content) {
    // Add background color, circular corners and material shadow to the sheet.
    // This is just an example, you can customize it however you want.
    return Material(
      color: Theme.of(context).colorScheme.secondaryContainer,
      borderRadius: BorderRadius.circular(16),
      clipBehavior: Clip.antiAlias,
      child: content,
    );
  }
}
Bildschirmaufnahme.2024-10-15.um.20.35.17.mov
@fujidaiti
Copy link
Owner

fujidaiti commented Oct 16, 2024

Hi @Clon1998,

maxPosition < proportional(1)

In what situations is this needed? I have always thought that exposing maxPosition was a flaw in the package’s API design during the early stages of development, as it is confusing and would be proportional(1) in almost all cases. I'm planning to remove that parameter from the sheet's constructor in a future release.

@Clon1998
Copy link
Author

Hi @Clon1998,

maxPosition < proportional(1)

In what situations is this needed? I have always thought that exposing maxPosition was a flaw in the package’s API design during the early stages of development, as it is confusing and would be proportional(1) in almost all cases. I'm planning to remove that parameter from the sheet's constructor in a future release.

It can be useful in cases where some of the content of the current page is useful to the user as reference.
I am currently using the flutter sheets and always limit the extent to 0.8.

@fujidaiti
Copy link
Owner

@Clon1998 Make sure maxPosition >= minPosition is always true.

ScrollableSheet(
  maxPosition: const SheetAnchor.proportional(0.8),
  minPosition: const SheetAnchor.proportional(0.8), // The default value is SheetAnchor.proportional(1.0)
  initialPosition: const SheetAnchor.proportional(0.8),
);

@Clon1998
Copy link
Author

Clon1998 commented Oct 19, 2024

@Clon1998 Make sure maxPosition >= minPosition is always true.

ScrollableSheet(
  maxPosition: const SheetAnchor.proportional(0.8),
  minPosition: const SheetAnchor.proportional(0.8), // The default value is SheetAnchor.proportional(1.0)
  initialPosition: const SheetAnchor.proportional(0.8),
);

This solves some of the issues but still the last few items are unreachable:

      maxPosition: SheetAnchor.proportional(0.8),
      initialPosition: SheetAnchor.proportional(0.8),
      minPosition: SheetAnchor.proportional(0.8),
Bildschirmaufnahme.2024-10-19.um.11.33.20.mov

However if I set different values for max, initial and min I can still observe the same issue as described in the initial reporting:

      maxPosition: SheetAnchor.proportional(0.8),
      initialPosition: SheetAnchor.proportional(0.5),
      minPosition: SheetAnchor.proportional(0.2),
Bildschirmaufnahme.2024-10-19.um.11.36.19.mov

@fujidaiti
Copy link
Owner

fujidaiti commented Oct 19, 2024

This solves some of the issues but still the last few items are unreachable:

That is the expected behavior when specifying maxPosition: SheetAnchor.proportional(0.8).
maxPosition: SheetAnchor.proportional(0.8) means that only 80% of the content is visible at maximum (in your case, the content is the ListView).

If you want the sheet’s height to be 80% of the screen height, you can use LayoutBuilder like this:

ScrollableSheet(
  child: LayoutBuilder(
    builder: (context, constraints) {
      return SizedBox(
        width: double.infinity,
        height: constraints.maxHeight * 0.8,
          child: ...,
      );
    },
  ),
);

@Clon1998
Copy link
Author

This solves some of the issues but still the last few items are unreachable:

That is the expected behavior when specifying maxPosition: SheetAnchor.proportional(0.8). maxPosition: SheetAnchor.proportional(0.8) means that only 80% of the content is visible at maximum (in your case, the content is the ListView).

If you want the sheet’s height to be 80% of the screen height, you can use LayoutBuilder like this:

ScrollableSheet(
  child: LayoutBuilder(
    builder: (context, constraints) {
      return SizedBox(
        width: double.infinity,
        height: constraints.maxHeight * 0.8,
          child: ...,
      );
    },
  ),
);

Mhhh. In that case, I don't understand why this property exists and why you intended to remove it, as there should be close to no use case in which a developer would want the content to be only 80% visible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P2
Projects
None yet
Development

No branches or pull requests

2 participants