-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
UnitControl: Normalize wrapper classnames by removing outer wrapper #45139
UnitControl: Normalize wrapper classnames by removing outer wrapper #45139
Conversation
This is achieved by removing the unnecessary outer wrapper for the UnitControl.
Open in CodeSandbox Web Editor | VS Code | VS Code Insiders |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before removing the wrapper, I went through a bunch of searches in Gutenberg looking for anything targeting or relying on the outer wrapper.
The only direct reference found was in the border radius control. So I made this PR based off #41860 where we remove that and introduce the __unstableWrapperClassname
to the UnitControl. I figure we could merge this back into #41860 to avoid that addition even momentarily.
The BorderControl
also leveraged the styled div Root
component from the UnitControl. Switching that to the styled input worked smoothly.
There were a few places passing inline styles to the UnitControl which were previously placed on the outer wrapper. They are still on the outer most element and I didn't spot any issues for these in testing. One example of this was the spacing presets control.
Size Change: -41 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
@glendaviesnz if you have time would you mind giving this one a double-check for the spacing sizes control? I'd like to make sure it doesn't mess anything up there for you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this so quickly, @aaronrobertshaw !
I figure we could merge this back into #41860 to avoid that addition even momentarily.
Sounds like a good idea, especially since this PR doesn't look that large anyway.
The BorderControl also leveraged the styled div Root component from the UnitControl. Switching that to the styled input worked smoothly.
Nice!
There were a few places passing inline styles to the UnitControl which were previously placed on the outer wrapper. They are still on the outer most element and I didn't spot any issues for these in testing.
Good point. That would be my expectation too, but we should definitely do some extensive testing
Doesn't appear to affect the spacing size controls at all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes LGTM!
I checked Storybook and the editor and wasn't able to notice any regressions.
No need to add a CHANGELOG entry for now if this gets merged back into #41860
* Update border control components to allow 40px height * Make 40px height work again after rebase * Turn on 40px tall border controls in editors * Update radius control to handle extra padding These tweaks update the BorderRadiusControl such that the entered values are actually visible despite the increased internal padding applied to the UnitControl when the `__unstable-large` variant is used. * First pass at improving border controls when at 40px Initial tweaks to improve the UX for the border controls. Aims to address the increased padding for large 40px high UnitControl variant that cuts of entered values from display. * Make compact inputs 118px wide * Increase padding to 16px between input and slider * Make split controls gap a uniform 16px * Update split layout for floating linked button * Make color dropdown 40px square with 16px indicator * Tweak input widths again to fit editor sidebar * Make radius control match border and grid * Remove hardcoded width from radius input wrapper * Try using Mix as placeholder for mixed values * Revert placeholder back to 'Mixed' * Remove use of internal component classnames * Remove unnecessary flex styles from linked button * Use space util for color swatch dimensions * Add explanatory comment for compact widths * Better align the left border for 30px height variant * Consolidate border radius controls to single 40px height This allows us to remove most of the changes that would have been introduced in this PR as well as simplify the CSS. * Fix RTL rendering and style naming * Remove unneeded prop setting 40px height * Switch __next40pxDefaultSize to size __unstable-large * Fix up defaults in types and update READMEs * Add components changelog * Rename new prop `__unstableWrapperClassName` * UnitControl: Normalize wrapper classnames by removing outer wrapper (#45139) * BorderBoxControl: Refactor stories to TypeScript and Controls (#45002) * Remove max-width wrappers from border control stories Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>
Looks like there were a couple of blocks that force a max-width on bespoke controls via inline styles. These include Cover, Search, and Spacer blocks. A fix has been put up in #45329. |
Fixes: #45094
Related:
What?
Normalizes the application of class names for the UnitControl, so it is applied to the outermost wrapper.
Why?
className
props being applied to the outermost element.How?
This PR removes the outer wrapper for the UnitControl. It appears unnecessary, and with only a few other minor tweaks, we maintain the status quo in terms of layout for other components e.g. BorderRadiusControl, while also bringing UnitControl in line with our other components.
Testing Instructions
components-unit-control-wrapper
class that was formerly on the outer wrapper.✍️ Dev Note
In order to improve consistency across the
@wordpress/components
package, any class names to theUnitControl
component through itsclassName
prop will be applied to its outer wrapper element, instead of an inner input wrapper element.As a consequence of this change, the
components-unit-control-wrapper
class name and the__unstableWrapperClassName
prop have been removed from the component.