[Slider] Fix thumb outline not matching spec #12967
Conversation
Follows material design spec.
Also fixes track outspacing thumb when using arrow keys while slider is focused.
The thumb never resizes on any interaction in the new design spec. Only the outline changes.
|
Looks good. While testing, I noticed what appears to be (another) bug - pressing space resets the slider to zero. Not one for this PR, just thought I'd mention it while you're on a Slider cleanup crusade. 😄 |
|
Argos fails are expected since I had to increase to root padding so that the outline of the thumb does not overflow the slider container. |
oliviertassinari
added
the
component: slider
I'm not sure that's the right trade-off. Looking at the spec, it seems the shadow, and possibly also the thumb should be allowed to overflow the container:
|
|
@mbrookes I created a quick example to compare the crane theme with this implementation. This implementation has a little bit more horizontal space close to theme.spacing but way less vertical. It's hard to tell if it's the slider bbox or outside margins that create the space but I think it's the responsibility of the slider to not overflow into it's surroundings. This has also practical implications since it increases the area for mouse interactions which was already improved in #11889 and also addressed possible overflow issues. |
|
@eps1lon What I was looking at in that screenshot is the alignment of the ends of the slider track with the left edge of the label, and the right edge of the checkbox. If that is the outer margin, then the thumb and shadow would overflow. If it isn't then we need to demonstrate how to reconcile the alignment of the slider with other components. |
|
@mbrookes Ah yes now I get it. I scanned some of the other components and it seems that they never define margins/paddings themself and leave it to the user to align them. I think we should therefore remove the padding completely which makes it straightforward to align components. |
|
@mbrookes Removed padding so that users are in full control of the alignment. |
|
Okay, I'm good with this, it feels like the right compromise. Not sure what's up with CI though. |
* [Slider] Lowest value for vertical should be at the bottom * [Slider] Fix lagging transition on vertical slider * [Slider] Fix thumb position on horizontal sliders * [docs] Document how to improve actionable area Document how to apply #11889 which was removed from default implementation in #12967.
The material design specification defines a
boxShadowfor hover, pressed and focused which was currently only partially applied from the implementation.WIth this PR the implementation should match the specification considering the thumb outline.
Visual comparison: