-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix StretchingOverscrollIndicator not handling directional changes correctly
#116548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix StretchingOverscrollIndicator not handling directional changes correctly
#116548
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically we use terms like leading and trailing so it can describe both vertical and horizontal cases.
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, and for your patience! Can you update the up/down to use leading and trailing terms?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically we use terms like leading and trailing so it can describe both vertical and horizontal cases.
c06ac80 to
0b40e39
Compare
|
It looks like this also fixes #88741 :) |
|
Thanks for the review! I added a test for horizontal scrolling, feel free to check again. |
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you! This will need a second review, I will see about finding someone. :)
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
|
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 |
@elliotkhd In the meantime, I'll try writing tests to cover this. |
It works for me now! |
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
6a51488 to
d248574
Compare
|
@Piinks I rebased the branch and added two more tests covering the bug @elliotkhd found. Feel free to review again |
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Piinks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, thank you! LGTM - again. :)
|
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
89fc3b4 to
bc89445
Compare
…changes correctly (flutter/flutter#116548)
…l changes correctly (flutter/flutter#116548)
…changes correctly (flutter/flutter#116548)
…changes correctly (flutter/flutter#116548)
* 3ad7ea3 Roll Plugins from 9c312d4d2f5f to 2ce625f1a87e (5 revisions) (flutter/flutter#120793) * 7865713 Roll Flutter Engine from 1328c4bc6299 to 4db9673d48d6 (2 revisions) (flutter/flutter#120796) * 541a8bf Fix switching from scrollable and non-scrollable tab bars throws (flutter/flutter#120771) * ab1390e Use black30 for CupertinoTabBar's border (flutter/flutter#119509) * a513d4e Fix `flutter_localizations` README references (flutter/flutter#120800) * a664f08 In test of --(no-)fatal-infos analyzer flags, pin missing_return to info (flutter/flutter#120797) * ef49f56 Add Android unit tests to plugin template (flutter/flutter#120720) * a12e242 Improve CupertinoContextMenu to match native more (flutter/flutter#117698) * a9f4366 Fix the `flutter run -d linux` tests (flutter/flutter#120721) * dff0955 09da59a5a Roll Dart SDK from c022d475e9d8 to 5d17a336bdfe (1 revision) (flutter/engine#39649) (flutter/flutter#120816) * f35de0c Adds wide gamut saveLayer integration test (flutter/flutter#120131) * 99dcaa2 Roll Flutter Engine from 09da59a5adcf to a8b3d1af55b6 (3 revisions) (flutter/flutter#120821) * 8d15083 Use the impellerc GLES output flag when compiling shaders for Android (flutter/flutter#120647) * c6b636f [flutter_tools] Replace Future.catchError() with Future.then(onError: ...) (flutter/flutter#120637) * 2b7d709 Add `@widgetFactory` annotation (flutter/flutter#117455) * e65dfba Add Linux unit tests to plugin template (flutter/flutter#120814) * dccec41 5de007b90 Remove "bringup: true" from "Linux Fuchsia FEMU" (flutter/engine#39651) (flutter/flutter#120826) * d6de6bc 9f3b061b7 Roll buildroot to 64b0c3deecaff8e66c2deb74e2171e8297b2bfcd (flutter/engine#39653) (flutter/flutter#120830) * da2508c bb1ff84b6 Add a white background to app anatomy diagram (flutter/engine#39638) (flutter/flutter#120832) * 1f85497 [flutter_tools] Add the NoProfile parameter to the PowerShell execution statement (flutter/flutter#120786) * 4ad47fb Fix `StretchingOverscrollIndicator` not handling directional changes correctly (flutter/flutter#116548) * 9a721c4 Update AndroidManifest.xml.tmpl (flutter/flutter#120527) * c0b7d2d Roll Flutter Engine from bb1ff84b6c4f to 02a379db1d38 (4 revisions) (flutter/flutter#120845) * a10e295 Added identical(a,b) short circuit to Material Library lerp methods (flutter/flutter#120829) * efde350 Roll Flutter Engine from 02a379db1d38 to a966cf878ffd (2 revisions) (flutter/flutter#120846) * cc473e4 Roll Flutter Engine from a966cf878ffd to 3fc40ca5beb9 (3 revisions) (flutter/flutter#120850) * d125242 Roll Flutter Engine from 3fc40ca5beb9 to 9fa2a5c3cfbd (2 revisions) (flutter/flutter#120856) * 22e17bb ea1d087c4 Roll Skia from b8b36146c7a0 to 7b3fb04bc3d4 (3 revisions) (flutter/engine#39673) (flutter/flutter#120860) * f85438b c8b1d2ffa Roll Fuchsia Mac SDK from YpQKlqmyn8r_snx06... to xl9Y8o-9FDyvPogki... (flutter/engine#39675) (flutter/flutter#120887) * 174a562 d699b4a91 Roll Flutter from e3471f0 to df41e58 (83 revisions) (flutter/plugins#7184) (flutter/flutter#120888) * 170539f 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)
…changes correctly (flutter/flutter#116548)
…changes correctly (flutter/flutter#116548)
|
Still issue on When the exception was thrown, this was the stack: The _StretchController sending notification was: _StretchController() ` |
…r#7186) * 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)

This PR fixes #116522
Fixes #88741
Fixes #118145
When using a
StrechingOverscrollIndicatorin combination withAlwaysScrollableScrollPhysics, 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
distanceForPullis calculated, as well as how the alignment for the transform is determined.After this change is made, the
StrechingOverscrollIndicatorwill 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
///).