Skip to content

Commit

Permalink
Fix: Modal sheet jerks when swipe to dismiss (#258)
Browse files Browse the repository at this point in the history
## Related issues (optional)

Fixes #250.

## Description

This PR modifies `_SwipeDismissibleController` to ensure that the
transition animation curve remains linear during the animation triggered
by a swipe-to-dismiss gesture. This is crucial to prevent the sheet from
jerking when the user swipes it down, as reported in #250. See the
comments in the `_handleDragEnd` method for a more detailed explanation.

## Summary (check all that apply)

- [x] Modified / added code
- [x] Modified / added tests
- [ ] Modified / added examples
- [x] Modified / added others (pubspec.yaml, workflows, etc...)
- [ ] Updated README
- [ ] Contains breaking changes
  - [ ] Created / updated migration guide
- [ ] Incremented version number
  - [ ] Updated CHANGELOG
  • Loading branch information
fujidaiti authored Oct 9, 2024
1 parent 646db64 commit 6900149
Show file tree
Hide file tree
Showing 4 changed files with 417 additions and 72 deletions.
2 changes: 1 addition & 1 deletion example/pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ packages:
path: ".."
relative: true
source: path
version: "0.9.4"
version: "0.10.0"
source_span:
dependency: transitive
description:
Expand Down
63 changes: 48 additions & 15 deletions lib/src/modal/modal_sheet.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'dart:async';
import 'dart:math';

import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';

import '../foundation/sheet_drag.dart';
Expand Down Expand Up @@ -150,12 +151,24 @@ class ModalSheetRoute<T> extends PageRoute<T> with ModalSheetRouteMixin<T> {

mixin ModalSheetRouteMixin<T> on ModalRoute<T> {
bool get swipeDismissible;

Curve get transitionCurve;

SwipeDismissSensitivity get swipeDismissSensitivity;

@override
bool get opaque => false;

/// The curve used for the transition animation.
///
/// In the middle of a dismiss gesture drag,
/// this returns [Curves.linear] to match the finger motion.
@nonVirtual
@visibleForTesting
Curve get effectiveCurve => (navigator?.userGestureInProgress ?? false)
? Curves.linear
: transitionCurve;

/// Lazily initialized in case `swipeDismissible` is set to false.
late final _swipeDismissibleController = _SwipeDismissibleController(
route: this,
Expand Down Expand Up @@ -188,13 +201,9 @@ mixin ModalSheetRouteMixin<T> on ModalRoute<T> {
Widget child,
) {
final transitionTween = Tween(begin: const Offset(0, 1), end: Offset.zero);
// In the middle of a dismiss gesture drag,
// let the transition be linear to match finger motions.
final curve =
navigator!.userGestureInProgress ? Curves.linear : this.transitionCurve;
return SlideTransition(
position: animation.drive(
transitionTween.chain(CurveTween(curve: curve)),
transitionTween.chain(CurveTween(curve: effectiveCurve)),
),
child: child,
);
Expand Down Expand Up @@ -390,19 +399,43 @@ class _SwipeDismissibleController with SheetGestureProxyMixin {
));
}

if (transitionController.isAnimating) {
// Keep the userGestureInProgress in true state so we don't change the
// curve of the page transition mid-flight since the route's transition
// depends on userGestureInProgress.
// Reset the transition animation curve back to the default from linear
// indirectly, by resetting the userGestureInProgress flag.
// It is "indirect" because ModalSheetRouteMixin.effectiveCurve returns
// the linear curve when the userGestureInProgress flag is set to true.
//
// If the transition animation has not settled at either the start or end,
// delay resetting the userGestureInProgress until the animation completes
// to ensure the effectiveCurve remains linear during the animation,
// matching the user's swipe motion. This is important to prevent the sheet
// from jerking when the user swipes it down.
// See https://github.com/fujidaiti/smooth_sheets/issues/250.
//
// Note: We cannot use AnimationController.isAnimating here to determine if
// the transition animation is running, because, in Navigator 2.0,
// the pop animation may not have started at this point even if
// Navigator.pop() is called to pop the modal route.
//
// The following sequence of events illustrates why:
// 1. Calling Navigator.pop() updates the internal page stack, triggering
// a rebuild of the Navigator. Note that the transition animation
// does not start here, so AnimationController.isAnimating returns false.
// 2. The Navigator rebuilds with the new page stack.
// 3. The modal route is removed from the Navigator's subtree.
// 4. Route.didPop() is called, initiating the pop transition animation
// by calling AnimationController.reverse().
if (transitionController.isCompleted || transitionController.isDismissed) {
_isUserGestureInProgress = false;
} else {
late final AnimationStatusListener animationStatusCallback;
animationStatusCallback = (_) {
_isUserGestureInProgress = false;
transitionController.removeStatusListener(animationStatusCallback);
animationStatusCallback = (status) {
if (status == AnimationStatus.completed ||
status == AnimationStatus.dismissed) {
_isUserGestureInProgress = false;
transitionController.removeStatusListener(animationStatusCallback);
}
};
transitionController.addStatusListener(animationStatusCallback);
} else {
// Otherwise, reset the userGestureInProgress state immediately.
_isUserGestureInProgress = false;
}

if (invokePop) {
Expand Down
1 change: 1 addition & 0 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ dev_dependencies:
build_runner: ^2.4.9
flutter_test:
sdk: flutter
go_router: ^14.2.3
mockito: ^5.4.4
pedantic_mono: ^1.23.0
Loading

0 comments on commit 6900149

Please sign in to comment.