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

Reduce spacing between label and slider control #2451

Closed

Conversation

antonis
Copy link

@antonis antonis commented Jun 30, 2020

Fixes #2126

Related PRs:

gutenberg WordPress/gutenberg#23580

Description

Reduces spacing between label and slider control by:

  • 14px on iOS
  • 24px on Android

How has this been tested?

  • Add a block that has as slider control in the settings (e.g. spacer)
  • Press on the settings button
  • Verify that the space between the label and the slider control is appropriate

Screenshots

Android iOS
Screenshot_1593521798 Simulator Screen Shot - iPhone 11 - 2020-06-30 at 15 52 30

Types of changes

Enhancement

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to [RELEASE-NOTES.txt](RELEASE-NOTES.txt) if necessary.

@peril-wordpress-mobile
Copy link

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

@iamthomasbishop
Copy link
Contributor

@antonis Thank you so much for tackling this one — it's been bothering me for a while 😄 2 quick questions:

  • Is there a way you can overlay keylines/metric lines so I can see the spacing/margins in more detail? I've seen this done on Android PR's but I'm not sure how it's done (or if it's even possible on iOS). If not I can also pull these screenshots into Figma to compare against my component library.
  • Would it be possible to align the left side of the slider track on Android with the 16px keyline/margin (example)? I'm guessing there is a small inset that comes with the native component, but I'd like to align this if possible.

@antonis
Copy link
Author

antonis commented Jun 30, 2020

Hello @iamthomasbishop 👋

Thank you for the quick feedback. I tried to align the content but it brakes when the material design animation enlarges the toggle and drops the shadow.
android

I'll check If I can add the keylines on both platforms and get back to you on your other question.

@iamthomasbishop
Copy link
Contributor

I tried to align the content but it brakes when the material design animation enlarges the toggle and drops the shadow.

@antonis ahh okay so the overflow is hidden, that makes sense. Let's ignore that for now, we can address it later.

@antonis
Copy link
Author

antonis commented Jul 1, 2020

Hello @iamthomasbishop,
I'm attaching an Android screenshot with the keylines/metric lines.
2020-07-01 10 51 28
I didn't find something similar on iOS.

@iamthomasbishop
Copy link
Contributor

@antonis These measurements should work pretty well. I have 2 tiny requests, both of which we can attack separately at a future time if we need to keep moving. Other than these things, feel free to ship this as-is!

  1. I still have the concern about Android's track not aligning to the left 16 key line, but it's not a blocker specific to this.
  2. I would like to at some point update the style of the text inputs on the Slider cells slightly -- it would look something like this, and align more closely to the native platforms.

Side note: For consistency, I pulled these into Figma and aligned with my library 👍

@antonis
Copy link
Author

antonis commented Jul 13, 2020

Gutenberg PR merged WordPress/gutenberg#23580

@antonis antonis closed this Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants