Fix `StretchingOverscrollIndicator` not handling directional changes correctly #116548

jankuss · 2022-12-05T21:38:25Z

This PR fixes #116522
Fixes #88741
Fixes #118145

When using a StrechingOverscrollIndicator in combination with AlwaysScrollableScrollPhysics, in the case the content doesn't fill the viewport, there is some kind of jump when changing the scroll direction while overscrolling.

This video shows this unwanted behaviour:

Screen.Recording.2022-12-05.at.22.20.39.mov

This PR makes changes on how distanceForPull is calculated, as well as how the alignment for the transform is determined.

After this change is made, the StrechingOverscrollIndicator will behave like this:

Screen.Recording.2022-12-05.at.22.25.10.mov

This behaves much smoother, and is much more in line with how it works in native android apps on Android 12.

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is [test-exempt].
All existing and new tests are passing.

jankuss · 2022-12-07T11:39:21Z

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

@@ -612,6 +612,11 @@ class _GlowingOverscrollIndicatorPainter extends CustomPainter {
  }
 }

+enum _StretchDirection {
+  down,
+  up


I'm not entirely sure about the naming here (up and down), since this implies verticality, even though it applies to vertical and horizontal scrolling. Any suggestions?

Typically we use terms like leading and trailing so it can describe both vertical and horizontal cases.

Piinks

Thanks for the PR, and for your patience! Can you update the up/down to use leading and trailing terms?

Piinks · 2022-12-21T23:28:24Z

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

@@ -612,6 +612,11 @@ class _GlowingOverscrollIndicatorPainter extends CustomPainter {
  }
 }

+enum _StretchDirection {
+  down,
+  up


Typically we use terms like leading and trailing so it can describe both vertical and horizontal cases.

AUTHORS

packages/flutter/test/widgets/overscroll_stretch_indicator_test.dart

Piinks · 2022-12-22T22:22:26Z

It looks like this also fixes #88741 :)

jankuss · 2022-12-23T00:37:27Z

Thanks for the review! I added a test for horizontal scrolling, feel free to check again.

Piinks

LGTM, thank you! This will need a second review, I will see about finding someone. :)

Piinks

Marking for changes here to just hold this for a sec while we investigate #118145. This PR may already resolve that issue, I'd like to check.

jankuss · 2023-01-18T15:06:31Z

@Piinks I was able to test #118145 with this fix, and it looks like this PR will resolve this issue as well.

Screen.Recording.2023-01-18.at.16.03.07.mov

JoanSchi · 2023-01-22T20:36:21Z

@jankuss Issue #104798 also happens only with material 3 overscroll. Is this issue also solved with this fix. (I tried to test it, but I was not able to get it right.)

elliotkhd · 2023-01-28T08:42:56Z

It seems there comes up another issue with this commit, when swiping up a list and let the list auto scroll to the end, the stretch direction goes oppesite.

screen-20230128.mp4

jankuss · 2023-01-28T13:37:56Z

It seems there comes up another issue with this commit, when swiping up a list and let the list auto scroll to the end, the stretch direction goes oppesite.

screen-20230128.mp4

Thanks for the info! I will check this out

jankuss · 2023-01-30T17:08:34Z

It seems there comes up another issue with this commit, when swiping up a list and let the list auto scroll to the end, the stretch direction goes oppesite.

screen-20230128.mp4

@elliotkhd
I believe I found the problem - could you check if the issue is fixed for you in my latest commit?

In the meantime, I'll try writing tests to cover this.

elliotkhd · 2023-01-31T03:03:19Z

It seems there comes up another issue with this commit, when swiping up a list and let the list auto scroll to the end, the stretch direction goes oppesite.
screen-20230128.mp4

@elliotkhd I believe I found the problem - could you check if the issue is fixed for you in my latest commit?

In the meantime, I'll try writing tests to cover this.

It works for me now!

Piinks

Hey @jankuss thank you for your patience, I was out of town for Flutter Forward. The failing checks here appear unrelated to your change.
Can you update your branch with the latest from the master branch? That should resolve the failures.

jankuss · 2023-02-02T11:40:55Z

@Piinks I rebased the branch and added two more tests covering the bug @elliotkhd found. Feel free to review again

Piinks

Thank you for contributing! I will need to find another reviewer for a second approval. Seeking one out now.

jankuss · 2023-02-15T15:34:58Z

@Piinks Thanks for testing! I added the test. The failing checks seem to be unrelated.

Piinks

Excellent, thank you! LGTM - again. :)

Piinks · 2023-02-15T19:57:48Z

As far as the test failures, I have seen this happen before. Rebasing with the master branch (not merging) should resolve the issue.

…ional change while scrolling

…l changes correctly (flutter/flutter#116548)

* 3ad7ea3c9 Roll Plugins from 9c312d4 to 2ce625f (5 revisions) (flutter/flutter#120793) * 786571368 Roll Flutter Engine from 1328c4bc6299 to 4db9673d48d6 (2 revisions) (flutter/flutter#120796) * 541a8bfd9 Fix switching from scrollable and non-scrollable tab bars throws (flutter/flutter#120771) * ab1390e0a Use black30 for CupertinoTabBar's border (flutter/flutter#119509) * a513d4e7b Fix `flutter_localizations` README references (flutter/flutter#120800) * a664f08a5 In test of --(no-)fatal-infos analyzer flags, pin missing_return to info (flutter/flutter#120797) * ef49f5661 Add Android unit tests to plugin template (flutter/flutter#120720) * a12e242c0 Improve CupertinoContextMenu to match native more (flutter/flutter#117698) * a9f43665c Fix the `flutter run -d linux` tests (flutter/flutter#120721) * dff09558d 09da59a5a Roll Dart SDK from c022d475e9d8 to 5d17a336bdfe (1 revision) (flutter/engine#39649) (flutter/flutter#120816) * f35de0c80 Adds wide gamut saveLayer integration test (flutter/flutter#120131) * 99dcaa2d9 Roll Flutter Engine from 09da59a5adcf to a8b3d1af55b6 (3 revisions) (flutter/flutter#120821) * 8d150833b Use the impellerc GLES output flag when compiling shaders for Android (flutter/flutter#120647) * c6b636fa5 [flutter_tools] Replace Future.catchError() with Future.then(onError: ...) (flutter/flutter#120637) * 2b7d709fd Add `@widgetFactory` annotation (flutter/flutter#117455) * e65dfba8e Add Linux unit tests to plugin template (flutter/flutter#120814) * dccec41d5 5de007b90 Remove "bringup: true" from "Linux Fuchsia FEMU" (flutter/engine#39651) (flutter/flutter#120826) * d6de6bc68 9f3b061b7 Roll buildroot to 64b0c3deecaff8e66c2deb74e2171e8297b2bfcd (flutter/engine#39653) (flutter/flutter#120830) * da2508c9f bb1ff84b6 Add a white background to app anatomy diagram (flutter/engine#39638) (flutter/flutter#120832) * 1f85497ef [flutter_tools] Add the NoProfile parameter to the PowerShell execution statement (flutter/flutter#120786) * 4ad47fb47 Fix `StretchingOverscrollIndicator` not handling directional changes correctly (flutter/flutter#116548) * 9a721c456 Update AndroidManifest.xml.tmpl (flutter/flutter#120527) * c0b7d2ddd Roll Flutter Engine from bb1ff84b6c4f to 02a379db1d38 (4 revisions) (flutter/flutter#120845) * a10e295a0 Added identical(a,b) short circuit to Material Library lerp methods (flutter/flutter#120829) * efde35081 Roll Flutter Engine from 02a379db1d38 to a966cf878ffd (2 revisions) (flutter/flutter#120846) * cc473e4f1 Roll Flutter Engine from a966cf878ffd to 3fc40ca5beb9 (3 revisions) (flutter/flutter#120850) * d1252428c Roll Flutter Engine from 3fc40ca5beb9 to 9fa2a5c3cfbd (2 revisions) (flutter/flutter#120856) * 22e17bb71 ea1d087c4 Roll Skia from b8b36146c7a0 to 7b3fb04bc3d4 (3 revisions) (flutter/engine#39673) (flutter/flutter#120860) * f85438bfa c8b1d2ffa Roll Fuchsia Mac SDK from YpQKlqmyn8r_snx06... to xl9Y8o-9FDyvPogki... (flutter/engine#39675) (flutter/flutter#120887) * 174a562a6 d699b4a Roll Flutter from e3471f08d1d3 to df41e58f6f4e (83 revisions) (flutter/plugins#7184) (flutter/flutter#120888) * 170539f83 Roll Flutter Engine from c8b1d2ffaec8 to 0d8d93306822 (2 revisions) (flutter/flutter#120891)

* 3ad7ea3c9 Roll Plugins from 9c312d4 to 2ce625f (5 revisions) (flutter/flutter#120793) * 786571368 Roll Flutter Engine from 1328c4bc6299 to 4db9673d48d6 (2 revisions) (flutter/flutter#120796) * 541a8bfd9 Fix switching from scrollable and non-scrollable tab bars throws (flutter/flutter#120771) * ab1390e0a Use black30 for CupertinoTabBar's border (flutter/flutter#119509) * a513d4e7b Fix `flutter_localizations` README references (flutter/flutter#120800) * a664f08a5 In test of --(no-)fatal-infos analyzer flags, pin missing_return to info (flutter/flutter#120797) * ef49f5661 Add Android unit tests to plugin template (flutter/flutter#120720) * a12e242c0 Improve CupertinoContextMenu to match native more (flutter/flutter#117698) * a9f43665c Fix the `flutter run -d linux` tests (flutter/flutter#120721) * dff09558d 09da59a5a Roll Dart SDK from c022d475e9d8 to 5d17a336bdfe (1 revision) (flutter/engine#39649) (flutter/flutter#120816) * f35de0c80 Adds wide gamut saveLayer integration test (flutter/flutter#120131) * 99dcaa2d9 Roll Flutter Engine from 09da59a5adcf to a8b3d1af55b6 (3 revisions) (flutter/flutter#120821) * 8d150833b Use the impellerc GLES output flag when compiling shaders for Android (flutter/flutter#120647) * c6b636fa5 [flutter_tools] Replace Future.catchError() with Future.then(onError: ...) (flutter/flutter#120637) * 2b7d709fd Add `@widgetFactory` annotation (flutter/flutter#117455) * e65dfba8e Add Linux unit tests to plugin template (flutter/flutter#120814) * dccec41d5 5de007b90 Remove "bringup: true" from "Linux Fuchsia FEMU" (flutter/engine#39651) (flutter/flutter#120826) * d6de6bc68 9f3b061b7 Roll buildroot to 64b0c3deecaff8e66c2deb74e2171e8297b2bfcd (flutter/engine#39653) (flutter/flutter#120830) * da2508c9f bb1ff84b6 Add a white background to app anatomy diagram (flutter/engine#39638) (flutter/flutter#120832) * 1f85497ef [flutter_tools] Add the NoProfile parameter to the PowerShell execution statement (flutter/flutter#120786) * 4ad47fb47 Fix `StretchingOverscrollIndicator` not handling directional changes correctly (flutter/flutter#116548) * 9a721c456 Update AndroidManifest.xml.tmpl (flutter/flutter#120527) * c0b7d2ddd Roll Flutter Engine from bb1ff84b6c4f to 02a379db1d38 (4 revisions) (flutter/flutter#120845) * a10e295a0 Added identical(a,b) short circuit to Material Library lerp methods (flutter/flutter#120829) * efde35081 Roll Flutter Engine from 02a379db1d38 to a966cf878ffd (2 revisions) (flutter/flutter#120846) * cc473e4f1 Roll Flutter Engine from a966cf878ffd to 3fc40ca5beb9 (3 revisions) (flutter/flutter#120850) * d1252428c Roll Flutter Engine from 3fc40ca5beb9 to 9fa2a5c3cfbd (2 revisions) (flutter/flutter#120856) * 22e17bb71 ea1d087c4 Roll Skia from b8b36146c7a0 to 7b3fb04bc3d4 (3 revisions) (flutter/engine#39673) (flutter/flutter#120860)

…l changes correctly (flutter/flutter#116548)

flutter-dashboard bot added f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. labels Dec 5, 2022

TahaTesser requested a review from Piinks December 6, 2022 08:47

jankuss commented Dec 7, 2022

View reviewed changes

Piinks reviewed Dec 21, 2022

View reviewed changes

jankuss force-pushed the fix-overscroll-always-scrollable-physics branch 2 times, most recently from c06ac80 to 0b40e39 Compare December 22, 2022 20:40

Piinks reviewed Dec 22, 2022

View reviewed changes

AUTHORS Outdated Show resolved Hide resolved

Piinks reviewed Dec 22, 2022

View reviewed changes

packages/flutter/test/widgets/overscroll_stretch_indicator_test.dart Show resolved Hide resolved

jankuss requested a review from Piinks December 23, 2022 00:37

Piinks approved these changes Jan 5, 2023

View reviewed changes

MerdanDev mentioned this pull request Jan 7, 2023

Re-implement stretch overscroll behavior as a custom shader #82906

Open

Piinks requested changes Jan 10, 2023

View reviewed changes

Piinks reviewed Feb 1, 2023

View reviewed changes

jankuss force-pushed the fix-overscroll-always-scrollable-physics branch 2 times, most recently from 6a51488 to d248574 Compare February 2, 2023 11:04

jankuss requested a review from Piinks February 2, 2023 11:41

Piinks added f: material design flutter/packages/flutter/material repository. a: fidelity Matching the OEM platforms better a: quality A truly polished experience labels Feb 2, 2023

Piinks approved these changes Feb 2, 2023

View reviewed changes

jankuss requested a review from Piinks February 15, 2023 15:20

Piinks approved these changes Feb 15, 2023

View reviewed changes

jankuss added 9 commits February 15, 2023 21:07

Fix overscroll behaviour of StretchingOverscrollIndicator on direct…

f5392b7

…ional change while scrolling

Remove trailing spaces

a5b5c10

Change naming of _StretchDirection values to trailing and leading

2466e42

Remove trailing space

00f5919

Apply suggestions from code review

0fb1247

Fix stretching overscroll indicator direction on fling

117a318

Fix issue when changing direction while recede animation is playing

0150902

Remove trailing space

b4f0af5

Add test for changing direction during recede animation

bc89445

jankuss force-pushed the fix-overscroll-always-scrollable-physics branch from 89fc3b4 to bc89445 Compare February 15, 2023 20:08

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 15, 2023

auto-submit bot merged commit 4ad47fb into flutter:master Feb 15, 2023

This was referenced Feb 16, 2023

Roll Flutter from df41e58f6f4e to 22e17bb71050 (28 revisions) flutter/packages#3211

Closed

Roll Flutter from df41e58f6f4e to 22e17bb71050 (28 revisions) flutter/plugins#7186

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2023

4ad47fb47 Fix StretchingOverscrollIndicator not handling directiona…

a383dd7

…l changes correctly (flutter/flutter#116548)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 16, 2023

4ad47fb47 Fix StretchingOverscrollIndicator not handling directiona…

5f9bd4d

…l changes correctly (flutter/flutter#116548)

engine-flutter-autoroll mentioned this pull request Feb 16, 2023

Roll Flutter from df41e58f6f4e to 174a562a665d (30 revisions) flutter/packages#3212

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2023

4ad47fb47 Fix StretchingOverscrollIndicator not handling directiona…

c1c965b

…l changes correctly (flutter/flutter#116548)

engine-flutter-autoroll mentioned this pull request Feb 16, 2023

Roll Flutter from df41e58f6f4e to 170539f83dc8 (31 revisions) flutter/packages#3214

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2023

4ad47fb47 Fix StretchingOverscrollIndicator not handling directiona…

38483d2

…l changes correctly (flutter/flutter#116548)

Piinks mentioned this pull request Feb 21, 2023

Unexpected behavior CustomScrollView with a short list, list jumps up and down with material 3 #118145

Closed

Piinks mentioned this pull request Mar 7, 2023

List hits something while scrolling #122008

Open

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

4ad47fb47 Fix StretchingOverscrollIndicator not handling directiona…

d2bcbd7

…l changes correctly (flutter/flutter#116548)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

4ad47fb47 Fix StretchingOverscrollIndicator not handling directiona…

98228c2

…l changes correctly (flutter/flutter#116548)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `StretchingOverscrollIndicator` not handling directional changes correctly #116548

Fix `StretchingOverscrollIndicator` not handling directional changes correctly #116548

jankuss commented Dec 5, 2022 •

edited by guidezpl

jankuss Dec 7, 2022

Piinks Dec 21, 2022

Piinks left a comment

Piinks Dec 21, 2022

Piinks commented Dec 22, 2022

jankuss commented Dec 23, 2022

Piinks left a comment

Piinks left a comment

jankuss commented Jan 18, 2023

JoanSchi commented Jan 22, 2023

elliotkhd commented Jan 28, 2023

jankuss commented Jan 28, 2023

jankuss commented Jan 30, 2023 •

edited

elliotkhd commented Jan 31, 2023

Piinks left a comment

jankuss commented Feb 2, 2023

Piinks left a comment

jankuss commented Feb 15, 2023 •

edited

Piinks left a comment

Piinks commented Feb 15, 2023

Fix StretchingOverscrollIndicator not handling directional changes correctly #116548

Fix StretchingOverscrollIndicator not handling directional changes correctly #116548

Conversation

jankuss commented Dec 5, 2022 • edited by guidezpl

Pre-launch Checklist

jankuss Dec 7, 2022

Choose a reason for hiding this comment

Piinks Dec 21, 2022

Choose a reason for hiding this comment

Piinks left a comment

Choose a reason for hiding this comment

Piinks Dec 21, 2022

Choose a reason for hiding this comment

Piinks commented Dec 22, 2022

jankuss commented Dec 23, 2022

Piinks left a comment

Choose a reason for hiding this comment

Piinks left a comment

Choose a reason for hiding this comment

jankuss commented Jan 18, 2023

JoanSchi commented Jan 22, 2023

elliotkhd commented Jan 28, 2023

jankuss commented Jan 28, 2023

jankuss commented Jan 30, 2023 • edited

elliotkhd commented Jan 31, 2023

Piinks left a comment

Choose a reason for hiding this comment

jankuss commented Feb 2, 2023

Piinks left a comment

Choose a reason for hiding this comment

jankuss commented Feb 15, 2023 • edited

Piinks left a comment

Choose a reason for hiding this comment

Piinks commented Feb 15, 2023

Fix `StretchingOverscrollIndicator` not handling directional changes correctly #116548

Fix `StretchingOverscrollIndicator` not handling directional changes correctly #116548

jankuss commented Dec 5, 2022 •

edited by guidezpl

jankuss commented Jan 30, 2023 •

edited

jankuss commented Feb 15, 2023 •

edited