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

BorderControl: Fix vertical alignment of inner slider control #39750

Merged
merged 1 commit into from
Mar 28, 2022

Conversation

aaronrobertshaw
Copy link
Contributor

What?

Improve the styling of the BorderControl to ensure the inner slider control is more reliably vertically aligned to the center out of the box.

Why?

This change is being made to the component itself so there is less that any consumer may need to tweak for everything to be neatly aligned in their UI.

How?

Adds back the display: flex rule that the component originally used to solve this alignment (#37769 (comment))

Testing Instructions

  1. Checkout this branch and start up the Storybook
    • npm run storybook:dev
  2. Confirm the slider within the BorderControl is still vertically aligned to the center.
  3. Check the alignment for the BorderBoxControl's linked view.
  4. If you're still not convinced. You can apply the changes from this branch onto #37770 and confirm the alignment now works in the block editor
    • Create a post, add a group block, and select it
    • Take a look at the border control in the inspector controls sidebar

Screenshots or screencast

Screenshots below are from #37770 and the block editor.

Before After
Screen Shot 2022-03-25 at 5 39 30 pm Screen Shot 2022-03-25 at 5 40 54 pm

@aaronrobertshaw aaronrobertshaw added [Type] Enhancement A suggestion for improvement. [Feature] UI Components Impacts or related to the UI component system labels Mar 25, 2022
@aaronrobertshaw aaronrobertshaw self-assigned this Mar 25, 2022
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

✅ Confirmed that in the Storybook version of the component there's no apparent difference
✅ Confirmed that in real-world usage in #37770 this is required to switch the element from being inline-flex to being flex to vertically centre the component in the available space

Sounds like a good place to put it to me, to ensure that the slider in this context is positioned correctly 👍

Before After
image image

@aaronrobertshaw aaronrobertshaw merged commit 98773d8 into trunk Mar 28, 2022
@aaronrobertshaw aaronrobertshaw deleted the fix/border-control-slider-alignment branch March 28, 2022 01:24
@github-actions github-actions bot added this to the Gutenberg 13.0 milestone Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] UI Components Impacts or related to the UI component system [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants