Skip to content
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

Refine spacing on cells that contain Slider controls #2126

Closed
iamthomasbishop opened this issue Apr 7, 2020 · 4 comments · Fixed by WordPress/gutenberg#24161
Closed

Refine spacing on cells that contain Slider controls #2126

iamthomasbishop opened this issue Apr 7, 2020 · 4 comments · Fixed by WordPress/gutenberg#24161
Assignees
Labels
good first issue Good for newcomers Proposal [Status] Needs Design Review Needs design review or sign-off before shipping

Comments

@iamthomasbishop
Copy link
Contributor

Cells that contain the Slider control feel a bit too spacious in most cases — particularly on block settings bottom sheets. For example:

Current

iOS Android

Note: Android appears to have even more spacing than iOS (about ~10px more), so we could also tighten that spacing up to the same values as iOS unless we want to provide more spacing on Android. Edit: I just realized this 10px disparity might be because of the drag handle size difference between platforms.

Proposal

While it was designed this way in an effort to align with some of the native patterns, over time it has become clear that spacing feels a bit too much in practice, esp on Android. What I'm proposing is to essentially remove about ~14px of spacing between cell title and slider on iOS and ~24px on Android, like this:

iOS Android

Before/After

iOS Android

cc'ing for feedback: @SylvesterWilmott @mattmiklic — do you think this feels too condensed? Should we worry about matching the native component exactly or is it okay to refine these based on how they've felt in context?

@iamthomasbishop iamthomasbishop added Proposal [Status] Needs Design Review Needs design review or sign-off before shipping labels Apr 7, 2020
@mattmiklic
Copy link
Member

Doesn't feel too condensed to me; I couldn't find any specific guidance in the Material Design spec for spacing around the slider, only the size of the thumb and track. 22pt makes sense on iOS as it's 1/2 the size of the tap target (44pt on iOS) for the thumb. On Android I might increase that to 24dp so it aligns to the 8dp grid and is 1/2 the size of the tap target (48dp on Android) for the thumb.

@iamthomasbishop
Copy link
Contributor Author

Doesn't feel too condensed to me

Okay cool, glad to hear 😄

I couldn't find any specific guidance in the Material Design spec for spacing around the slider

Yea, I think I ended up literally designing over a screenshot for Android for the initial version of the component. The track and thumb should be 1:1 w/ the guidelines.

22pt makes sense on iOS as it's 1/2 the size of the tap target (44pt on iOS) for the thumb

To be clear, I'm not 100% sure if these are 100% accurate values, but they are based on screenshots from the app/editor as it exists today. That said, I think most of our row content styling matches sizings between platforms, so in other words, row height is 48 on both platforms.

On Android I might increase that to 24dp so it aligns to the 8dp grid and is 1/2 the size of the tap target (48dp on Android) for the thumb.

I'm not 100% sure if that would make it align to the grid per se, because I'm not positive of what the height of the slider component's "container" itself is (it looks like 28px which is the height of the iOS slider thumb, but I'm not 100% sure where the paddings/margin lay.

@SylvesterWilmott
Copy link

Looks good to me too. I would place the slider itself within an invisible 44pt bar on iOS and 48dp bar on Android and align the label to the top of that bar. But doing that gives you close enough values to 22pt/dp on both platforms so it doesn't really matter so much anyway.

@antonis
Copy link

antonis commented Jul 22, 2020

This is Reopened with WordPress/gutenberg#24109
due to #2501

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers Proposal [Status] Needs Design Review Needs design review or sign-off before shipping
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants