-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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 discrete Slider
and RangeSlider
to enforce thumb height padding when the track shape is non-rounded
#164703
base: master
Are you sure you want to change the base?
Fix discrete Slider
and RangeSlider
to enforce thumb height padding when the track shape is non-rounded
#164703
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.
This LGTM. Thanks for the fix. Just a little confused about the numbers in the test.
material, | ||
paints | ||
// Start thumb. | ||
..circle(x: sliderPadding, y: 300.0, color: theme.colorScheme.primary) |
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.
Just a question, how do we get this 24? It looks like sliderPadding isn't used anywhere.
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 comes from the track padding applied by base track shape in RangeSlider
widget.
Here trackLeft
and trackRight
are 24
pixels.
flutter/packages/flutter/lib/src/material/slider_theme.dart
Lines 2213 to 2216 in 234b50a
final double trackLeft = offset.dx + math.max(overlayWidth / 2, thumbWidth / 2); | |
final double trackTop = offset.dy + (parentBox.size.height - trackHeight) / 2; | |
final double trackRight = trackLeft + parentBox.size.width - math.max(thumbWidth, overlayWidth); | |
final double trackBottom = trackTop + trackHeight; |
I made such track padding customizable in the Slider
widget last year (#156143) but it's not yet implemented in RangeSlider
but the I used the same wording here "slider padding" since it's effectively the same thing.
I just filed an issue #165046 and assigned. I will complete the Slider
padding work for RangeSlider
. Thanks for the reminder.
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.
Ah I see! Could you add a comment above the number? Thanks a lot for the explanation!
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.
Added a comment
3e364c5
to
3b67573
Compare
The failed Google testing seems related to M2 style. |
@QuncCccccc Looks like checks are green. Could you please confirm this fixes the issue you described? |
…ng when the track shape is non-rounded
1bb820b
to
6a39a39
Compare
Fixes Discrete
Slider
andRangeSlider
applies thumb padding when using custom Slider shapesCode Sample
expand to view the code sample
Before
After
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.