[Slider] Increase interaction area #18429
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oliviertassinari
added
accessibility
oliviertassinari
changed the title
oliviertassinari
force-pushed
the
slider-increase-interaction-area
branch
2 times, most recently
from
6a82086 to
cee5e04
Compare
oliviertassinari
force-pushed
the
slider-increase-interaction-area
branch
from
cee5e04 to
b8e0814
Compare
Details of bundle changes.Comparing: fa764b9...68d5c5e
|
eps1lon
reviewed
Nov 18, 2019
eps1lon
reviewed
Nov 18, 2019
eps1lon
reviewed
Nov 18, 2019
| right: -18, | ||
| bottom: -18, | ||
| borderRadius: '50%', | ||
| // reach 42px hit target (2 * 15 + thumb diameter) |
There was a problem hiding this comment.
It's a tradeoff to be closer to 44px without leaking too much from the dimension of the parent. 28px is a tradeoff to increase the interaction zone without changing too much the layout for existing users and still enabling dense layout.
I don't think that we should always follow the specification to the letter.
There was a problem hiding this comment.
But if you don't you have to explain how you arrive at these values. And not just some ephemeral "it's a tradeoff". Given the 8px grid this is now 3/4 of a unit instead of the 1/2 which is probably why the recommendation uses 44. Just go with the default to stay in the design system. Not following the default system is actually more likely to cause layout breakage.
There was a problem hiding this comment.
I think that we can ignore the 4px grid for the thumb extension because it's not visible. By tradeoff, I mean, increase the slider height by the minimal grid unit: 4px, make the thumb "leak" by the minimum amount: 7px visually look almost too much.
There was a problem hiding this comment.
What is this "minimum amount" you talk about? How did you construct this number?
There was a problem hiding this comment.
4px is the grid unit, so we get 28/4 = 7.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
... and keep the thumb in parent boundaries
Reverted from #18395 and continued in a standalone effort.
Looking more closely at design systems like Elastic UI, Garden, Blueprint, BaseUI, you can notice that the slider height is larget than us. This change gets us closer to what the Material Design specification recommends: https://material.io/design/usability/accessibility.html#layout-typography: 44px when the main modality is a pointer device. We go from 24px to 28px (still /4).
I have reduced the touch area of the thumb so it almost doesn't "leak" outside its container. This is subjective, but I don't think that the slider should prevent the interaction on the element above it. We have 42px vs the recommended 44px minimum.
Before
After
