-
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
SelectControl: remove margin overrides and add new opt-in prop #46448
Conversation
Size Change: +63 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
36aaa60
to
936c689
Compare
width: 100%; | ||
} | ||
.components-select-control__input { |
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.
This CSS was removed because it wasn't being used (overwritten by other styles).
@@ -164,11 +164,12 @@ | |||
} | |||
|
|||
.wp-block-legacy-widget { | |||
.components-base-control { | |||
.wp-block-legacy-widget__selector-container { | |||
width: 100%; |
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.
This override was added because of flex on the Placeholder
fieldset, with no options on Placeholder
to change that other than with CSS. So I added a div with a class to specify the style of this block and remove it from BaseControl
. I could have added the class to WidgetTypeSelector
, but it would be more code changes in two different files, so it seemed better to go this way.
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.
Great job 👏 I especially like how you resolved the style hack in legacy-widget
!
I actually learn a lot about Gutenberg functionality by going through your thoughtful testing instructions 😂 Thanks for that, too.
...s/edit-site/src/components/sidebar-edit-mode/navigation-menu-sidebar/navigation-inspector.js
Outdated
Show resolved
Hide resolved
...s/edit-site/src/components/sidebar-edit-mode/navigation-menu-sidebar/navigation-inspector.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/template-details/template-part-area-selector.js
Outdated
Show resolved
Hide resolved
|
||
${ StyledField } { | ||
margin-bottom: 0; | ||
} |
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.
🎉
@@ -164,11 +164,12 @@ | |||
} | |||
|
|||
.wp-block-legacy-widget { | |||
.components-base-control { |
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.
Good call to remove the hack 👍
packages/widgets/src/blocks/legacy-widget/edit/widget-type-selector.js
Outdated
Show resolved
Hide resolved
} | ||
} } | ||
/> | ||
<div className="wp-block-legacy-widget__selector-container"> |
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.
Btw there is also a FlexBlock
component you can use for this purpose. (Sorry for all these undiscoverable components 😅)
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.
Don't be sorry! It's super valuable for me to know, and demonstrates why this project is such a great learning experience! 😄
packages/components/CHANGELOG.md
Outdated
@@ -31,6 +31,7 @@ | |||
- Lighten the border color on control components ([#46252](https://github.com/WordPress/gutenberg/pull/46252)). | |||
- `Popover`: Prevent unnecessary paint when scrolling by using transform instead of top/left positionning ([#46187](https://github.com/WordPress/gutenberg/pull/46187)). | |||
- `CircularOptionPicker`: Prevent unecessary paint on hover ([#46197](https://github.com/WordPress/gutenberg/pull/46197)). | |||
- `ColorPicker` & `QueryControls`: Replace bottom margin overrides with `__nextHasNoMarginBottom` ([#46448](https://github.com/WordPress/gutenberg/pull/46448)). |
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.
We can move this to the "Internal" section since it doesn't really affect consumers.
936c689
to
7a1f9d7
Compare
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.
Looking good! 🚀
What?
Added new opt-in prop
__nextHasNoMarginBottom
for usages ofSelectControl
in the Gutenberg codebase and removed margin overrides.Why?
Part of this project: #38730
The tl;dr is
BaseControl
has amargin-bottom
which makes it difficult to reuse and results in inconsistent use.How?
By removing margin overrides in the CSS and adding the prop
__nextHasNoMarginBottom
.Additional Notes
ManageLocations
wasn't updated, as I found this wasn't being used when looking atCheckboxControl
: Navigation Editor Tracking Issue #29102I've updated
PostTemplate
although it isn't being used anymore, as it was replaced byPostTemplateForm
: Turn Template settings panel into a Template popover #41925 (comment)Testing Instructions
1. The Site Editor:
NavigationInspector
TemplatePartAreaSelector
TemplatePartAdvancedControls
Color Picker
2. The Block Editor:
DefaultStylePicker
Comment out the following lines:
gutenberg/packages/block-editor/src/components/default-style-picker/index.js
Lines 53 to 55 in b333d1d
Add a Quote block to the editor
Look for 'Default Style' label in the block inspector
Ensure the space below the select element is the same as before
FontFamilyControl
For ImageSizeControl
ArchivesEdit
AudioEdit
CommentsInspectorControls
FileBlockInspector
GalleryEdit
GalleryEdit V1
I have tested this myself, and it's a bit involved to test, but here are the steps if using
wp-env
and Docker:gutenberg/.wp-env.json
, change the first line to:"core": "WordPress/WordPress#5.9"
npm run wp-env start
/.wp-env/{DOCKER-CONTAINER}/WordPress/wp-content/plugins
plugins": [ ],
npx wp-env run cli wp option set use_balanceTags 1
to enableuseBalanceTags
GroupEditControls
PostAuthorEdit
QueryControls
DimensionControls
QueryContent & QueryInspectorControls
There are a few components to test in the Query loop block inspector so this can be done in one shot:
Add Query Loop block and select 'Start Blank'
Click any variation
Look for the following labels in the block inspector and ensure the below select elements' bottom spacing is the same as before:
Next, go to the bottom of the block inspector and open the 'Advanced' section
Look for the select element under the 'HTML ELement' label
Ensure the space below the select element is the same as before
TagCloudEdit
VideoSettings && SingleTrackEditor
Add video block
Upload/add video
Look for the 'Preload' label in the block inspector
Ensure the space below the select element is the same as before
Click on the block and select 'Text tracks' from the toolbar
Add text track (I used a sample I found here)
PostTemplateForm
3. An older theme (e.g. Twenty Seventeen):
PostFormat && PostAuthorSelect
WidgetTypeSelector
Storybook
Disabled
Checklist template (if it helps for testing)
Site Editor:
Block Editor:
With an older theme:
Storybook