Skip to content

Fix dragging scrollbar down causes unexpected behavior when scrollable has a negative minScrollExtent #170331

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Fix dragging scrollbar down causes unexpected behavior when scrollable has a negative minScrollExtent #170331

wants to merge 8 commits into from
+50 −4

Conversation

greyovo
Copy link

@greyovo greyovo commented Jun 10, 2025 •
edited
Loading

This PR fixed #170246.

In the original method for calculating the thumb position based on the offset, the minimum scroll distance was assumed to be 0 by default, which caused the scrollbar offset fraction to be constrained between 0 and 1. However, when minScrollExtent is negative, this range is incorrect.

Modifications are listed below:

  • The valid range has been updated to be between minScrollExtent / scrollableExtent and maxScrollExtent / scrollableExtent.
  • Added corresponding test cases for this change.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@google-cla Google CLA
Copy link

google-cla bot commented Jun 10, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Jun 10, 2025
@greyovo greyovo changed the title Fix dragging scrollbar thumb down causes unexpected behavior when scrollable has a negative minScrollExtent Fix dragging scrollbar down causes unexpected behavior when scrollable has a negative minScrollExtent Jun 10, 2025
@greyovo greyovo mentioned this pull request Jun 10, 2025
Open
@dkwingsmt dkwingsmt requested a review from Piinks June 11, 2025 18:12
Piinks
Piinks approved these changes Jun 12, 2025
View reviewed changes
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

Hey @greyovo welcome! Thanks for contributing.
This change LGTM, but there is an analyzer failure I can't see, probably due to the current GCP outage. I will circle back on that while I find another reviewer.

justinmc
justinmc approved these changes Jun 12, 2025
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

LGTM 👍 . Thank you for filing this issue and then fixing it!

chunhtai
chunhtai reviewed Jun 12, 2025
View reviewed changes
chunhtai
chunhtai approved these changes Jun 12, 2025
View reviewed changes
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

LGTM, just one nit

@greyovo
Copy link
Author

greyovo commented Jun 13, 2025

@chunhtai I added an assertion in getThumbScrollOffset, please have a look :)

@chunhtai
Copy link
Contributor

Looks like there is some formatting issue
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8712195095568406145/+/u/run_test.dart_for_analyze_shard_and_subshard_None/stdout

can you run dart formatter on the files?

@chunhtai
Copy link
Contributor

@chunhtai I added an assertion in getThumbScrollOffset, please have a look :)

Thanks, LGTM

@greyovo
Copy link
Author

greyovo commented Jun 13, 2025

I ran dart format on the files I modified, but it also formatted some parts that I didn't change. Is this the expected behavior?

@chunhtai
Copy link
Contributor

chunhtai commented Jun 13, 2025 •
edited
Loading

I ran dart format on the files I modified, but it also formatted some parts that I didn't change. Is this the expected behavior?

no It is not, do you have outdated pub? try runing flutter update-packages

@chunhtai
Copy link
Contributor

looks like you may have longer line limit?

@Piinks
Copy link
Contributor

Piinks commented Jun 13, 2025

Hmm, another thing might be that it is trying to format with Dart 3.8 instead of 3.7. The framework has not bumped yet. Maybe too try running flutter clean?

@greyovo
Copy link
Author

greyovo commented Jun 14, 2025

Thank you all for the kindness help. ❤️ I reverted the wrong commit and updated with a new one.

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

@Piinks Piinks Piinks approved these changes

@justinmc justinmc justinmc approved these changes

@chunhtai chunhtai chunhtai approved these changes

Assignees
No one assigned
Labels
f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Scrollbar unexpectedly jumps to the starting position when dragging down, with CustomScrollView's center was set
4 participants
@greyovo @chunhtai @Piinks @justinmc