-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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): hide ticks when slider is disabled #2687
Conversation
devversion
commented
Jan 17, 2017
•
edited
Loading
edited
- No longer shows the ticks on hover when the slider is disabled.
@@ -5,7 +5,6 @@ | |||
@mixin md-slider-theme($theme) { | |||
$accent: map-get($theme, accent); | |||
|
|||
// TODO(iveysaur): Find an implementation to hide the track under a disabled thumb. |
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 TODO
was actually referring to something else. If you look at the spec, you'll see that there's a tiny empty space around the thumb that covers up the track when the slider is disabled; this is talking about that.
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.
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.
Oh, I didn't realize.
LGTM |
It looks like this causes non-disabled sliders to not render the last tick. Make sure to rebase on top of #2641 it adds some more corner cases to watch out for. Please include me as a reviewer on slider PRs in the future. |
@mmalerba Yeah I locally tested this PR with your PR #2641 but at this time it wasn't merged yet (will rebase it today) Also from looking at the diff, there hasn't been any change for tick sequencing. It only hides the And yes can assign you to future slider PR's |
I think the css rule precedence started getting unwieldy so I added a |
* No longer shows the ticks on hover, when the slider is disabled.
831256c
to
1baf989
Compare
@mmalerba Updated everything as discussed on Slack. |
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. |