Fix drift in scroll_overlay Android item alignment; small updates #13
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These changes are made by a plain `flutter pub get` with the current stable Flutter, 3.7.1.
This helps make it readable in an editor.
The heights on the Android side were systematically up to one physical pixel less than on the Flutter side, due to rounding. This made the items conspicuously out of alignment by the lower part of the first screen, even before any scrolling, making it impossible to tell whether a given scroll physics matched exactly or had a small discrepancy. Fix by using a tiny bit more math in the calculation, so that the rounding error doesn't accumulate.
|
@Piinks, could I interest you in reviewing this PR? After this I have some further enhancements to this test app queued up, including a handy graphical display of the two velocities which I've found quite helpful: I figure I'll send those as a second PR once this one is in (because this one is already doing several different small things, and combining everything together would make the resulting commit a bit messy to read.) |
8 tasks
Piinks
approved these changes
Feb 11, 2023
|
Great, thanks! After the weekend I'll turn those further changes into a second PR. |
This was referenced Feb 13, 2023
OK, and done. I actually broke them up into three PRs:
I'm hoping that reduces, rather than increases, the work it takes to review them. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of my work on flutter/flutter#119875.
The heights on the Android side were systematically up to one physical pixel less than on the Flutter side, due to rounding. This made the items conspicuously out of alignment by the lower part of the first screen, even before any scrolling, making it impossible to tell whether a given scroll physics matched exactly or had a small discrepancy.
Fix by using a tiny bit more math in the calculation, so that the rounding error doesn't accumulate.
Screenshots:
Also make a few small updates and other cleanups, in separate commits for ease of review. In particular, update pubspec.lock so that running the app with a current Flutter doesn't dirty the tree.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.