modal_bottom_sheet icon indicating copy to clipboard operation
modal_bottom_sheet copied to clipboard

Problem with BouncingScrollPhysics and Modal with inside navigation

Open ananevam opened this issue 4 years ago • 21 comments

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

ananevam avatar Jul 14 '20 16:07 ananevam

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

jamesblasco avatar Jul 14 '20 16:07 jamesblasco

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

passsy avatar Oct 27 '20 10:10 passsy

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)

cyberpwnn avatar Jan 27 '21 10:01 cyberpwnn

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 😓

jamesblasco avatar Jan 27 '21 13:01 jamesblasco

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 avatar Jan 27 '21 17:01 passsy

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

passsy avatar Jan 27 '21 17:01 passsy

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!

jamesblasco avatar Jan 27 '21 17:01 jamesblasco

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;
}

plavunov avatar Apr 02 '21 17:04 plavunov

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.

mcorcuera avatar Aug 11 '21 18:08 mcorcuera

Did someone find a solution for this?

joris-prl avatar Mar 16 '22 00:03 joris-prl

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

f-person avatar Jun 06 '22 16:06 f-person

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.

mprync avatar Oct 01 '22 16:10 mprync

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.

kamami avatar Nov 17 '22 13:11 kamami

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.

alvindrakes avatar Dec 08 '22 08:12 alvindrakes

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

parker-sherrill avatar Dec 09 '23 05:12 parker-sherrill

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...

  • infers the correct ScrollPhysics and hands it down using the builder pattern.
  • blocks handing on the scroll notifications in case we don't want the modal to be closed at all (2) and hands on scroll notifications in case we want the modal to be closed (3).

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

benedictstrube avatar Mar 04 '24 17:03 benedictstrube

@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

stefanschaller avatar Apr 15 '24 11:04 stefanschaller

@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.

benedictstrube avatar Apr 15 '24 12:04 benedictstrube

@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.

optdxf avatar Apr 30 '24 07:04 optdxf

@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.

benedictstrube avatar Apr 30 '24 09:04 benedictstrube

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

optdxf avatar Apr 30 '24 20:04 optdxf