jamesblasco / modal_bottom_sheet

Flutter | Create advanced modal bottom sheets. Material, Cupertino or your own style
https://pub.dev/packages/modal_bottom_sheet
MIT License
1.86k stars 466 forks source link

Problem with BouncingScrollPhysics and Modal with inside navigation #47

Open ananevam opened 4 years ago

ananevam commented 4 years ago

1 When you are using a BouncingScrollPhysics and scroll down the screen you see that the scroll startet to go under the borders. I understand why it's going. How I can get around it for make "as native" behawiour? For when I swipe down the window scroll stay at his place

2 If you make Cupertino Modal with inside navigation there will no way to create the next screen in stack with scroll which will afford you to swipe down all the navigation stack. Because scrollController which I receive from builder I can attach to only the first window at navigation stack

22

jamesblasco commented 4 years ago

You can use ClamplingScrollPhysics for the moment, I was planning to create a custom physics that solves this.

And you are making a really good point there about the scrollController. I will have to think about how to approach this

passsy commented 3 years ago

I have the same problem. With navigation inside the modal, the second Route using controller: ModalScrollController.of(context), isn't able to close the bottom sheet with a swipe

cyberpwnn commented 3 years ago

I have the same problem. With navigation inside the modal, the second Route using controller: ModalScrollController.of(context), isn't able to close the bottom sheet with a swipe

Did you figure this out? It has nothing to do with bouncy physics, It's just that if there is actually something that requires scrolling, then the bottomSheet wont dismiss or even animate downwards when trying to swipe it down (at the top of whatever scrollable the scroll controller is attached to)

jamesblasco commented 3 years ago

sorry @passsy, I never got to see your comment. Are you still having the same issue? This is probably because the ScrollController is used in multiple scrollviews at the same time.

https://github.com/jamesblasco/modal_bottom_sheet/blob/ab1dc56e17c1ff15300bc32a9a66ef43694947df/lib/src/bottom_sheet.dart#L270

Could you share your specific case with some reproducible code so I can check the issue? I think it could be better to create a new issue for this

Maybe doing something like this might work

scrollController: ModalRoute.of(context).iscurrent ? ModalScrollController.of(context) : null,

Sorry I don't have much time lately to focus on this 😓

passsy commented 3 years ago

I can still reproduce it in the "Modal with Nested Scroll" example on the current website.

https://user-images.githubusercontent.com/1096485/106029523-77d5e200-60cd-11eb-967e-60b103d71f97.mp4

passsy commented 3 years ago

The fix I'm currently using is this:

  void _handleScrollUpdate(ScrollNotification notification) {
    //Check if scrollController is used
    if (!_scrollController.hasClients) return;
-    //Check if there is more than 1 attached ScrollController e.g. swiping page in PageView
-    // ignore: invalid_use_of_protected_member
-    if (_scrollController.positions.length > 1) return;

    if (_scrollController !=
        Scrollable.of(notification.context).widget.controller) return;

-    final scrollPosition = _scrollController.position;
+    final scrollPosition = _scrollController.positions
+        .firstWhere((it) => it.isScrollingNotifier.value);

    if (scrollPosition.axis == Axis.horizontal) return;

It works for me but I'm not sure it works for all cases

jamesblasco commented 3 years ago

Oh I see, then this issue is not about the navigator, and more Nested scroll views.

The problem here is with multiple scroll views using the same scroll controller (Right now ModalScrollController.of(context) is the default PrimaryScrollController.of(context) of the modal).

For the moment this can be easily fixed by creating another scroll controller or adding a Scaffold in between. I will take a deeper look at it. thanks!

plavunov commented 3 years ago

I combined Flutter's BouncingScrollPhysics() and ClampingPhysics() to overcome this issue. So it clamps at the top, but bounces at the bottom. Seems to work, at least for my purposes, but use at your own risk!

import 'dart:math' as math;
import 'package:flutter/material.dart';
import 'package:flutter/gestures.dart';
import 'package:flutter/physics.dart';

class BottomBouncingScrollPhysics extends ScrollPhysics {
  const BottomBouncingScrollPhysics({ScrollPhysics? parent})
      : super(parent: parent);

  @override
  BottomBouncingScrollPhysics applyTo(ScrollPhysics? ancestor) {
    return BottomBouncingScrollPhysics(parent: buildParent(ancestor));
  }

  double frictionFactor(double overscrollFraction) =>
      0.52 * math.pow(1 - overscrollFraction, 2);

  @override
  double applyPhysicsToUserOffset(ScrollMetrics position, double offset) {
    assert(offset != 0.0);
    assert(position.minScrollExtent <= position.maxScrollExtent);

    if (!position.outOfRange) return offset;

    //final double overscrollPastStart = math.max(position.minScrollExtent - position.pixels, 0.0);
    final double overscrollPastEnd =
        math.max(position.pixels - position.maxScrollExtent, 0.0);
    final double overscrollPast =
        overscrollPastEnd; //math.max(overscrollPastStart, overscrollPastEnd);
    final bool easing = (overscrollPastEnd > 0.0 && offset > 0.0);

    final double friction = easing
        // Apply less resistance when easing the overscroll vs tensioning.
        ? frictionFactor(
            (overscrollPast - offset.abs()) / position.viewportDimension)
        : frictionFactor(overscrollPast / position.viewportDimension);
    final double direction = offset.sign;

    return direction * _applyFriction(overscrollPast, offset.abs(), friction);
  }

  static double _applyFriction(
      double extentOutside, double absDelta, double gamma) {
    assert(absDelta > 0);
    double total = 0.0;
    if (extentOutside > 0) {
      final double deltaToLimit = extentOutside / gamma;
      if (absDelta < deltaToLimit) return absDelta * gamma;
      total += extentOutside;
      absDelta -= deltaToLimit;
    }
    return total + absDelta;
  }

  @override
  double applyBoundaryConditions(ScrollMetrics position, double value) {
    if (value < position.pixels &&
        position.pixels <= position.minScrollExtent) // underscroll
      return value - position.pixels;
    // if (position.maxScrollExtent <= position.pixels && position.pixels < value) // overscroll
    //   return value - position.pixels;
    if (value < position.minScrollExtent &&
        position.minScrollExtent < position.pixels) // hit top edge
      return value - position.minScrollExtent;
    // if (position.pixels < position.maxScrollExtent && position.maxScrollExtent < value) // hit bottom edge
    //   return value - position.maxScrollExtent;
    return 0.0;
  }

  @override
  Simulation? createBallisticSimulation(
      ScrollMetrics position, double velocity) {
    final Tolerance tolerance = this.tolerance;
    if (velocity.abs() >= tolerance.velocity || position.outOfRange) {
      return BouncingScrollSimulation(
        spring: spring,
        position: position.pixels,
        velocity: velocity,
        leadingExtent: position.minScrollExtent,
        trailingExtent: position.maxScrollExtent,
        tolerance: tolerance,
      );
    }
    return null;
  }

  @override
  double get minFlingVelocity => kMinFlingVelocity * 2.0;

  @override
  double carriedMomentum(double existingVelocity) {
    return existingVelocity.sign *
        math.min(0.000816 * math.pow(existingVelocity.abs(), 1.967).toDouble(),
            40000.0);
  }

  // Eyeballed from observation to counter the effect of an unintended scroll
  // from the natural motion of lifting the finger after a scroll.
  @override
  double get dragStartDistanceMotionThreshold => 3.5;
}
mcorcuera commented 3 years ago

I have been able to solve this issue by setting shrinkWrap: true on my list view. This removes the over-scrolling on the top of the list but not on the bottom.

joris-prl commented 2 years ago

Did someone find a solution for this?

f-person commented 2 years ago

I created a scroll physics which solves this issue too. Just use TopBlockedBouncingScrollPhysics for your scrollable's physics and you should be good to go

https://github.com/qyre-ab/flutter_top_blocked_bouncing_scroll_physics

mprync commented 1 year ago

I can still reproduce it in the "Modal with Nested Scroll" example on the current website.

Screen-Recording-2021-01-27-18-28-30.mp4

Does anyone have a solution for this? Nothing I've tried fixes it. Using clamping physics create a new problem and when you pull down the sheet slowly it's fine, but when you start to scroll back up it snaps instantly back to the top of the screen and starts to scroll the inner scroll view.

I'm using a CustomScrollView with slivers.

kamami commented 1 year ago

Same issue with CustomScrollView and slivers. @f-person Your Scroll Physics does not solve the problem for me. The sheet still snaps to the top when scrolling back up.

alvindrakes commented 1 year ago

Same issue with CustomScrollView and slivers. @f-person Your Scroll Physics does not solve the problem for me. The sheet still snaps to the top when scrolling back up.

Yeap, having the same issue here. I was trying to replicate Airbnb bottom sheet UI where the bottom sheet won't snap instantly when dragging up.

Got no choice but to disable enableDrag for now.

parker-sherrill commented 9 months ago

Was there ever a solution for this added? Currently on the 3.0.0 pre release, but still having the weird scroll bug when closing the cupertino modal @jamesblasco. Have tried multiple fixes

benedictstrube commented 6 months ago

I finally found a solution that works good for me.

Goal

My ultimate goal for the scroll behaviour of a modally presented screen with a scroll view was:

  1. Have bouncing behaviour at the bottom always.
  2. Have bouncing behaviour at the top when scrolling up not starting at the top edge (bounce back behaviour).
  3. Have clamping behaviour at the top when scrolling down starts at the top edge.
  4. Dragging down the modal should only be possible in case (3) and especially not in case (2).

This is the common behaviour for iOS modals we know from Apples own apps, Instagram, Trade Republic etc.

Implementation Idea

To allow for dragging the modal down, the plugin listens for scroll notifications of the inner scroll view and scrolls the whole modal down, once we over scroll the inner scroll view. This hurts (2). I implemented a listener, that...

My Wrapper

import 'package:flutter/material.dart';

/// Wrapper for screens that are presented in a modal by the package
/// modal_bottom_sheet.
///
/// Allows for determining the correct `ScrollPhysics` to use inside the screen
/// to have a clamping behaviour at the right time.
class ModalBottomSheetWrapper extends StatefulWidget {

  /// `scrollPhysics` are the recommended `ScrollPhysics` to be used for any
  /// scroll view inside.
  final Widget Function(BuildContext context, ScrollPhysics scrollPhysics) builder;

  const ModalBottomSheetWrapper({
    super.key,
    required this.builder,
  });

  @override
  State<ModalBottomSheetWrapper> createState() => _ModalBottomSheetWrapperState();
}

class _ModalBottomSheetWrapperState extends State<ModalBottomSheetWrapper> {
  bool _clamp = false;

  @override
  Widget build(BuildContext context) {
    return NotificationListener(
      onNotification: (ScrollNotification notification) {
        // don't care about horizontal scrolling
        if (notification.metrics.axis != Axis.vertical) {
          return false;
        }

        // examine new value
        bool clamp = false;

        // TODO: (04/03/24) handle inverted
        final atTopEdge = notification.metrics.pixels == notification.metrics.minScrollExtent;

        // if scrolling starts, exactly clamp when we start to drag at the top
        if (notification is ScrollStartNotification) {
          clamp = atTopEdge;
          setState(() {
            _clamp = clamp;
          });
        }

        // if scrolling ends, exactly clamp if we end on the edge
        if (notification is ScrollEndNotification) {
          clamp = atTopEdge;
          setState(() {
            _clamp = clamp;
          });
        }

        // when we are scrolling, enable bouncing again if we dragged away from
        // the edge
        if (notification is ScrollUpdateNotification) {
          if (!atTopEdge) {
            clamp = false;
            setState(() {
              _clamp = clamp;
            });
          }
        }

        // only pass on scroll events if we are clamping (only then we want
        // the modal to be closed potentially)
        return !_clamp;
      },
      child: widget.builder(
        context,
        _clamp
            ? const ClampingScrollPhysics()
            : const BouncingScrollPhysics(),
      ),
    );
  }
}

and then use it in the screen you present modally:

ModalBottomSheetWrapper(
  builder: (context, physics) {
    return ListView(
      // important: use the provided physics
      physics: physics,
      children: [
        ...
      ],
    );
  },
);

Demonstration

https://github.com/jamesblasco/modal_bottom_sheet/assets/23528911/c613d5dd-9508-4194-88d2-c033765ad227

stefanschaller commented 5 months ago

@benedictstrube Looks good, but it could be way more simple:

// State varaible: 
bool _overridePhysics = false;

// build method: 
            return Listener(
              onPointerMove: (_) {
                final atTopEdge = controller.offset <= 0;

                final shouldOverride = atTopEdge;

                if (_overridePhysics == shouldOverride) return;

                setState(() => _overridePhysics = shouldOverride);
              },
              onPointerUp: (details) {
                if (!_overridePhysics) return;

                setState(() => _overridePhysics = false);
              },
              child: ListView(
                physics: _overridePhysics ? const ClampingScrollPhysics() : null,
                ...
              ),
            )

You otherwise lose the native feeling

benedictstrube commented 5 months ago

@stefanschaller I think your solution does infer the correct physics to use, but it won't block scroll notifications from bubbling up to the plugin implementation. If you now scroll past the top edge, your list would bounce correctly but the modal would still be closing while you were dragging further downwards. This behaviour is also displayed in the initial issue description. Native feeling while dragging down past the top edge specifically was a goal for my implementation and was achieved by the NotificationListener which only forwards events if the scroll physics have a clamping behaviour (at the top) while dragging downwards.

optdxf commented 4 months ago

@benedictstrube I like your solution, but one annoyance/bug is that when you scroll down starting at the top edge (which pulls down the modal sheet) and instead of just letting go of the modal sheet (to let it bounce up) you attempt to drag the modal sheet up, it springs up immediately (triggers the _handleDragEnd call inside of the _handleScrollUpdate function) and no longer tracks your scroll. Do you know of a fix for this? Spent a bit trying to fix this bug to no avail -- will continue to investigate though.

benedictstrube commented 4 months ago

@optdxf This actually does not seem to be bound to my solution or this issue here at all. I tested it with a scrollable widget inside a modally opened screen and the behaviour was as you described (without using my wrapper). So this seems to need a fix outside of my implementation. Maybe it's reasonable to open another issue to attract attention? Definitely needs addressing as it hurts the "native feeling".

Edit: you need to set the physics to ClampingScrollPhysics in order to reproduce.

optdxf commented 4 months ago

@benedictstrube Yep you're right. I'll try and investigate further before opening another issue.