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

Settings: Update FormToggle to align to the top of the label #11237

Merged
merged 1 commit into from
Feb 23, 2017

Conversation

ryelle
Copy link
Member

@ryelle ryelle commented Feb 7, 2017

Fixes #10899

Toggles should now be aligned to the top line of the label when the label wraps:

screen shot 2017-02-10 at 11 25 44 am

To test:

  1. Visit the settings screens where toggles are used, for example, Writing
  2. Shrink your screen to force the labels to wrap
  3. Toggles should stay top-aligned
  4. Check other places where toggles are used (Page & post settings in the editor, followed sites management) to make sure these aren't negatively affected.

@rickybanister @MichaelArestad

@ryelle ryelle added [Feature] Site Settings All other general site settings. [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 7, 2017
@ryelle ryelle self-assigned this Feb 7, 2017
@matticbot
Copy link
Contributor

@rickybanister
Copy link

Vertical alignment of toggle looks great.

Quick question, it appears that the margin between the main toggles and their labels vs the sub-toggles and their labels is different by about 4px, can you confirm and or fix that?

@ryelle
Copy link
Member Author

ryelle commented Feb 7, 2017

You're right, it looks like we're setting the margin for JetpackModule toggles to 8px, while the others are 12px. Should it always be 12, or always be 8?

@rickybanister
Copy link

I'd guess it should always be 12 if that style pre-dates the module toggles.

@MichaelArestad does that make sense?

@MichaelArestad
Copy link
Contributor

Yeah. It should be 12, but we'll need to update the indent a bit to match so the child toggles line up with the module toggle's label.

@ryelle
Copy link
Member Author

ryelle commented Feb 7, 2017

@MichaelArestad I'm not sure what you mean, can you tweak the CSS here (either as a commit to this PR or just drop in a comment)?

@ryelle
Copy link
Member Author

ryelle commented Feb 10, 2017

@rickybanister @MichaelArestad #11240's been merged, taking care of the child toggle alignment - is this OK to merge now?

@MichaelArestad
Copy link
Contributor

Tested. Beautiful. Ready to merge. Thanks for tackling this!

@MichaelArestad MichaelArestad added [Status] Ready to Merge and removed [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 11, 2017
@oskosk oskosk merged commit d661448 into master Feb 23, 2017
@oskosk oskosk deleted the fix/align-form-toggles branch February 23, 2017 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Settings All other general site settings.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants