Skip to content

Commit 4ad47fb

Browse files
authored
Fix StretchingOverscrollIndicator not handling directional changes correctly (#116548)
* Fix overscroll behaviour of `StretchingOverscrollIndicator` on directional change while scrolling * Remove trailing spaces * Change naming of `_StretchDirection` values to `trailing` and `leading` * Remove trailing space * Apply suggestions from code review * Fix stretching overscroll indicator direction on fling * Fix issue when changing direction while recede animation is playing * Remove trailing space * Add test for changing direction during recede animation
1 parent 1f85497 commit 4ad47fb

File tree

3 files changed

+350
-21
lines changed

3 files changed

+350
-21
lines changed

AUTHORS

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,4 +101,5 @@ Junhua Lin <[email protected]>
101101
Tomasz Gucio <[email protected]>
102102
Jason C.H <[email protected]>
103103
Hubert Jóźwiak <[email protected]>
104-
Eli Albert <[email protected]>
104+
Eli Albert <[email protected]>
105+

packages/flutter/lib/src/widgets/overscroll_indicator.dart

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,15 @@ class _GlowingOverscrollIndicatorPainter extends CustomPainter {
601601
}
602602
}
603603

604+
enum _StretchDirection {
605+
/// The [trailing] direction indicates that the content will be stretched toward
606+
/// the trailing edge.
607+
trailing,
608+
/// The [leading] direction indicates that the content will be stretched toward
609+
/// the leading edge.
610+
leading,
611+
}
612+
604613
/// A Material Design visual indication that a scroll view has overscrolled.
605614
///
606615
/// A [StretchingOverscrollIndicator] listens for [ScrollNotification]s in order
@@ -689,6 +698,9 @@ class _StretchingOverscrollIndicatorState extends State<StretchingOverscrollIndi
689698
late final _StretchController _stretchController = _StretchController(vsync: this);
690699
ScrollNotification? _lastNotification;
691700
OverscrollNotification? _lastOverscrollNotification;
701+
702+
double _totalOverscroll = 0.0;
703+
692704
bool _accepted = true;
693705

694706
bool _handleScrollNotification(ScrollNotification notification) {
@@ -706,48 +718,52 @@ class _StretchingOverscrollIndicatorState extends State<StretchingOverscrollIndi
706718

707719
assert(notification.metrics.axis == widget.axis);
708720
if (_accepted) {
721+
_totalOverscroll += notification.overscroll;
722+
709723
if (notification.velocity != 0.0) {
710724
assert(notification.dragDetails == null);
711-
_stretchController.absorbImpact(notification.velocity.abs());
725+
_stretchController.absorbImpact(notification.velocity.abs(), _totalOverscroll);
712726
} else {
713727
assert(notification.overscroll != 0.0);
714728
if (notification.dragDetails != null) {
715729
// We clamp the overscroll amount relative to the length of the viewport,
716730
// which is the furthest distance a single pointer could pull on the
717731
// screen. This is because more than one pointer will multiply the
718732
// amount of overscroll - https://github.com/flutter/flutter/issues/11884
733+
719734
final double viewportDimension = notification.metrics.viewportDimension;
720-
final double distanceForPull =
721-
(notification.overscroll.abs() / viewportDimension) + _stretchController.pullDistance;
735+
final double distanceForPull = _totalOverscroll.abs() / viewportDimension;
722736
final double clampedOverscroll = clampDouble(distanceForPull, 0, 1.0);
723-
_stretchController.pull(clampedOverscroll);
737+
_stretchController.pull(clampedOverscroll, _totalOverscroll);
724738
}
725739
}
726740
}
727741
} else if (notification is ScrollEndNotification || notification is ScrollUpdateNotification) {
742+
// Since the overscrolling ended, we reset the total overscroll amount.
743+
_totalOverscroll = 0;
728744
_stretchController.scrollEnd();
729745
}
730746
_lastNotification = notification;
731747
return false;
732748
}
733749

734-
AlignmentGeometry _getAlignmentForAxisDirection(double overscroll) {
750+
AlignmentGeometry _getAlignmentForAxisDirection(_StretchDirection stretchDirection) {
735751
// Accounts for reversed scrollables by checking the AxisDirection
736752
switch (widget.axisDirection) {
737753
case AxisDirection.up:
738-
return overscroll > 0
754+
return stretchDirection == _StretchDirection.trailing
739755
? AlignmentDirectional.topCenter
740756
: AlignmentDirectional.bottomCenter;
741757
case AxisDirection.right:
742-
return overscroll > 0
758+
return stretchDirection == _StretchDirection.trailing
743759
? Alignment.centerRight
744760
: Alignment.centerLeft;
745761
case AxisDirection.down:
746-
return overscroll > 0
762+
return stretchDirection == _StretchDirection.trailing
747763
? AlignmentDirectional.bottomCenter
748764
: AlignmentDirectional.topCenter;
749765
case AxisDirection.left:
750-
return overscroll > 0
766+
return stretchDirection == _StretchDirection.trailing
751767
? Alignment.centerLeft
752768
: Alignment.centerRight;
753769
}
@@ -784,7 +800,7 @@ class _StretchingOverscrollIndicatorState extends State<StretchingOverscrollIndi
784800
}
785801

786802
final AlignmentGeometry alignment = _getAlignmentForAxisDirection(
787-
_lastOverscrollNotification?.overscroll ?? 0.0
803+
_stretchController.stretchDirection,
788804
);
789805

790806
final double viewportDimension = _lastOverscrollNotification?.metrics.viewportDimension ?? mainAxisSize;
@@ -836,6 +852,9 @@ class _StretchController extends ChangeNotifier {
836852
double get pullDistance => _pullDistance;
837853
double _pullDistance = 0.0;
838854

855+
_StretchDirection get stretchDirection => _stretchDirection;
856+
_StretchDirection _stretchDirection = _StretchDirection.trailing;
857+
839858
// Constants from Android.
840859
static const double _exponentialScalar = math.e / 0.33;
841860
static const double _stretchIntensity = 0.016;
@@ -847,23 +866,35 @@ class _StretchController extends ChangeNotifier {
847866
/// Handle a fling to the edge of the viewport at a particular velocity.
848867
///
849868
/// The velocity must be positive.
850-
void absorbImpact(double velocity) {
869+
void absorbImpact(double velocity, double totalOverscroll) {
851870
assert(velocity >= 0.0);
852871
velocity = clampDouble(velocity, 1, 10000);
853872
_stretchSizeTween.begin = _stretchSize.value;
854873
_stretchSizeTween.end = math.min(_stretchIntensity + (_flingFriction / velocity), 1.0);
855874
_stretchController.duration = Duration(milliseconds: (velocity * 0.02).round());
856875
_stretchController.forward(from: 0.0);
857876
_state = _StretchState.absorb;
877+
_stretchDirection = totalOverscroll > 0 ? _StretchDirection.trailing : _StretchDirection.leading;
858878
}
859879

860880
/// Handle a user-driven overscroll.
861881
///
862882
/// The `normalizedOverscroll` argument should be the absolute value of the
863883
/// scroll distance in logical pixels, divided by the extent of the viewport
864884
/// in the main axis.
865-
void pull(double normalizedOverscroll) {
885+
void pull(double normalizedOverscroll, double totalOverscroll) {
866886
assert(normalizedOverscroll >= 0.0);
887+
888+
final _StretchDirection newStretchDirection = totalOverscroll > 0 ? _StretchDirection.trailing : _StretchDirection.leading;
889+
if (_stretchDirection != newStretchDirection && _state == _StretchState.recede) {
890+
// When the stretch direction changes while we are in the recede state, we need to ignore the change.
891+
// If we don't, the stretch will instantly jump to the new direction with the recede animation still playing, which causes
892+
// a unwanted visual abnormality (https://github.com/flutter/flutter/pull/116548#issuecomment-1414872567).
893+
// By ignoring the directional change until the recede state is finished, we can avoid this.
894+
return;
895+
}
896+
897+
_stretchDirection = newStretchDirection;
867898
_pullDistance = normalizedOverscroll;
868899
_stretchSizeTween.begin = _stretchSize.value;
869900
final double linearIntensity =_stretchIntensity * _pullDistance;

0 commit comments

Comments
 (0)