-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix scrollable TabBar jittering
#150041
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 scrollable TabBar jittering
#150041
Conversation
|
Even though this fix works to some extent, it doesn't fix "TabBar with isScrollable set to true is broken" as with the changes proposed in this PR the TabBar jumps after swiping to Tab 3 and back to Tab 2: Screen.Recording.2024-06-13.at.10.27.41.mov |
|
@PerLycke |
0d1c142 to
1a31012
Compare
|
@PerLycke Could you please try it and confirm it works as expected? |
|
@TahaTesser I can confirm it now works as expected 👍 |
TabBar jitters in the middle positionTabBar jittering
justinmc
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.
Tricky, good catch. LGTM 👍
FYI @nate-thegrate
| < 0 => lerpDouble(middlePosition, leadingPosition, index - value)!, | ||
| _ => lerpDouble(middlePosition, trailingPosition, value - index)!, |
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 catching my mistake here!
(And thanks @justinmc for letting me know!)
| final double offset = switch ((value, index)) { | ||
| (_, _) when value == index - 1.0 => leadingPosition ?? middlePosition, | ||
| (_, _) when value == index + 1.0 => trailingPosition ?? middlePosition, | ||
| (_, _) when value == index => middlePosition, | ||
| (_, _) when value < index => leadingPosition == null ? middlePosition : lerpDouble(middlePosition, leadingPosition, index - value)!, | ||
| (_, _) => trailingPosition == null ? middlePosition : lerpDouble(middlePosition, trailingPosition, value - index)!, | ||
| }; |
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.
It feels a bit weird to pass a record into a switch expression and then not use it.
| final double offset = switch ((value, index)) { | |
| (_, _) when value == index - 1.0 => leadingPosition ?? middlePosition, | |
| (_, _) when value == index + 1.0 => trailingPosition ?? middlePosition, | |
| (_, _) when value == index => middlePosition, | |
| (_, _) when value < index => leadingPosition == null ? middlePosition : lerpDouble(middlePosition, leadingPosition, index - value)!, | |
| (_, _) => trailingPosition == null ? middlePosition : lerpDouble(middlePosition, trailingPosition, value - index)!, | |
| }; | |
| final double offset = switch (value - index) { | |
| -1.0 => leadingPosition ?? middlePosition, | |
| 1.0 => trailingPosition ?? middlePosition, | |
| 0 => middlePosition, | |
| < 0 => leadingPosition == null ? middlePosition : lerpDouble(middlePosition, leadingPosition, index - value)!, | |
| _ => trailingPosition == null ? middlePosition : lerpDouble(middlePosition, trailingPosition, value - index)!, | |
| }; |
I believe the logic is identical, based on a local test run.
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.
This is better, thanks!
nate-thegrate
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.
I really appreciate the effort put into correcting a previous error I made.
If we want to just merge right away, that's good with me 👍
Manual roll requested by [email protected] flutter/flutter@ccf3abe...6c06abb 2024-06-18 [email protected] Add test for engine artifact framework permissions (flutter/flutter#148786) 2024-06-18 [email protected] Add test for icon_button.3.dart (flutter/flutter#149988) 2024-06-18 [email protected] Roll Flutter Engine from 78fdd06af541 to 74f42ca3544c (6 revisions) (flutter/flutter#150421) 2024-06-18 [email protected] Fix transparent `dividerColor` breaks `TabBar.tabAlignment` (flutter/flutter#150350) 2024-06-18 [email protected] Fix scrollable `TabBar` jittering (flutter/flutter#150041) 2024-06-18 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland 3: [CupertinoActionSheet] Match colors to native (#150386)" (flutter/flutter#150413) 2024-06-18 [email protected] Extend the Windows web_tool_tests_1_2 shard timeout to 45 minutes (flutter/flutter#150393) 2024-06-18 [email protected] Roll Flutter Engine from 1c4e5e230ecb to 78fdd06af541 (3 revisions) (flutter/flutter#150403) 2024-06-18 [email protected] Roll Flutter Engine from a4f266f7eb1a to 1c4e5e230ecb (8 revisions) (flutter/flutter#150399) 2024-06-18 [email protected] Rename doc file to use standard hyphens (flutter/flutter#150314) 2024-06-17 [email protected] Fix typo in `SliverLayoutDimensions.hashCode` where not all properties are used in the hash code. (flutter/flutter#150306) 2024-06-17 [email protected] Fix doc comment references to 'this' (flutter/flutter#150379) 2024-06-17 [email protected] Add 'fail-fast' argument to flutter test (flutter/flutter#149587) 2024-06-17 [email protected] Update matchesGoldenFile documentation reference to goldenFileComparator (flutter/flutter#150343) 2024-06-17 [email protected] Reland 3: [CupertinoActionSheet] Match colors to native (flutter/flutter#150386) 2024-06-17 [email protected] [a11y] Add semantics: button to bottom navigation bar items and dropdown menu items (flutter/flutter#149375) 2024-06-17 [email protected] Reland "sliverGridDelegate mainAxisExtent add assert (#148470)" (flutter/flutter#149720) 2024-06-17 [email protected] `ScaffoldBackgroundColor` should default to `ColorScheme.surface` (flutter/flutter#149772) 2024-06-17 [email protected] Reland TreeSliver (flutter/flutter#149839) 2024-06-17 [email protected] Reland: [CupertinoActionSheet] Add sliding tap gesture (flutter/flutter#150219) 2024-06-17 [email protected] Roll Flutter Engine from 5989f0215fed to a4f266f7eb1a (1 revision) (flutter/flutter#150377) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

fixes TabBar with isScrollable set to true is broken
This regressed due to a tiny mistake in #146293
Code sample
expand to view the code sample
Before
before.mov
After
after.mov
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.