-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix Slider renders track when track colors are transparent
#161814
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 Slider renders track when track colors are transparent
#161814
Conversation
|
Wow, awesome fixes! Thanks @TahaTesser 💙 |
c096882 to
f5318f9
Compare
QuncCccccc
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.
LGTM! Thanks a lot for the fix:)
|
|
||
| final Color? effectiveActiveTrackColor = | ||
| widget.activeColor ?? sliderTheme.activeTrackColor ?? defaults.activeTrackColor; | ||
| final Color? effectiveInactiveColor = |
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.
nit: maybe rename to effectiveInactiveTrackColor to keep consistent:)?
f5318f9 to
0ceba5c
Compare
|
@QuncCccccc Looks like this is failing google testing. |
|
Taking a look |
4c65335 to
9dd944d
Compare
9dd944d to
86647fa
Compare
| sliderTheme.inactiveTrackColor == Colors.transparent) { | ||
| trackHeight = 0; | ||
| } | ||
|
|
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 fix seems only related to the first issue in the description. Are we still include the fix for the second issue #161805?
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.
Yes, I commented one of the fixes to figure out which fix is actually failing google testing.
Now I know which code is causing the issue. I will try to find a root cause.
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'm removing non google test failing fix to better understand the cause and file separate PR for it.
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.
Also updated the description to remove second the issue.
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.
Would you please mind approving second time with only single issue fix now so I can land this?
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 thanks a lot for letting me know! LGTM:)
Slider thumb cannot reach extreme endsRangeSlider renders track when track colors are transparent
RangeSlider renders track when track colors are transparent Slider renders track when track colors are transparent
|
So these changes removes the original fix I commented on earlier? Just asking, because if so, I need to reflect that change in the status of original issue. |
It'll be in separate PR |
| sliderTheme.inactiveTrackColor == Colors.transparent) { | ||
| trackHeight = 0; | ||
| } | ||
|
|
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 thanks a lot for letting me know! LGTM:)
Description
Fixes
Sliderwith transparent track colors and customtrackHeightcannot reach the extreme endsCode 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.