Skip to content

Conversation

@Piinks
Copy link
Contributor

@Piinks Piinks commented Apr 17, 2020

Description

This PR updates the NestedScrollCoordinator to prioritize scrolling 'away' any inner overscroll before any of the outer/inner positions. Previously, if you did not lift your finger from the screen and allow the overscroll to bounce back, the overscroll would stick and scroll in between the inner and outer scrollables.

Bug behavior:

ezgif com-video-to-gif (5)

Fixed behavior:

Fixed

Related Issues

#29264
#17518

Tests

I updated the nested overscroll related tests. This is not being considered a breaking change. Although existing tests needed to be changed, there was a TODO stating these expectations when this was fixed. The comments around these tests were acknowledging that the expected behavior was bad. Ran through internal tests and everything passed, customer tests are passing as well. :)

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

  • No, this is not a breaking change.

@Piinks Piinks added platform-ios iOS applications specifically framework flutter/packages/flutter repository. See also f: labels. c: API break Backwards-incompatible API changes a: fidelity Matching the OEM platforms better f: scrolling Viewports, list views, slivers, etc. a: quality A truly polished experience work in progress; do not review customer: crowd Affects or could affect many people, though not necessarily a specific customer. labels Apr 17, 2020
@Piinks Piinks changed the title Prioritize scrolling away nested overscroll [WIP] Prioritize scrolling away nested overscroll Apr 17, 2020
@Piinks Piinks removed the c: API break Backwards-incompatible API changes label Apr 20, 2020
@Piinks Piinks marked this pull request as ready for review April 20, 2020 18:07
@Piinks Piinks changed the title [WIP] Prioritize scrolling away nested overscroll Prioritize scrolling away nested overscroll Apr 20, 2020
@Piinks Piinks requested a review from goderbauer April 24, 2020 19:30
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite web_tests-7_last-linux has failed. Please fix the issues identified (or deflake) before re-applying this label.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a: fidelity Matching the OEM platforms better a: quality A truly polished experience customer: crowd Affects or could affect many people, though not necessarily a specific customer. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. platform-ios iOS applications specifically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants