-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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): thumb disappearing on disabled element with thumb label #6641
fix(slider): thumb disappearing on disabled element with thumb label #6641
Conversation
src/lib/slider/slider.scss
Outdated
@@ -214,7 +214,7 @@ $mat-slider-focus-ring-size: 30px !default; | |||
|
|||
|
|||
// Active slider. | |||
.cdk-focused { | |||
.mat-slider.cdk-focused:not(.mat-slider-disabled) { |
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 adds a lot off specificity, are you sure it doesn't break any of the other states?
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 didn't notice anything from poking around the demo. I can reduce the specificity slightly by changing the selector to .cdk-focused:not(.mat-slider-disabled)
.
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 think its good to leave the .mat-slider
so it doesn't wind up triggering when inside of another focused element, just curious if you had checked for other cases because I know I've had CSS regressions in the past with slider when I try to do these kind of changes
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.
One place I could see it potentially breaking is the mouse, keyboard etc. focused styles a bit below. I can scope those to .mat-slider:not(.mat-slider-disabled)
as well.
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.
Reworked as discussed @mmalerba.
Fixes the slider thumb becoming hidden when a disabled slider with a thumb label is focused. Fixes angular#6631.
dbced21
to
cd30d8d
Compare
LGTM styles still seem to work |
Fixes the slider not updating if its direction is changed dynamically. This is something I came across while testing angular#6641.
Fixes the slider not updating if its direction is changed dynamically. This is something I came across while testing angular#6641.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes the slider thumb becoming hidden when a disabled slider with a thumb label is focused.
Fixes #6631.