Add margin-bottom lint rules for TextControl #64212
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
49 tasks
|
Size Change: +73 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
mirka
commented
Aug 2, 2024
| @@ -63,6 +63,7 @@ function InputFieldBlock( { attributes, setAttributes, className } ) { | |||
| ) } | |||
| <InspectorControls group="advanced"> | |||
| <TextControl | |||
| __nextHasNoMarginBottom | |||
There was a problem hiding this comment.
Testing instructions
In Gutenberg experiments (wp-admin/admin.php?page=gutenberg-experiments), enable the "Form and input blocks" experiment. In the editor, add a Form block. In the block inspector, expand the "Advanced" section at the bottom.
Comment on lines
+98
to
+101
| <VStack spacing={ 4 }> | ||
| <TextControl | ||
| __next40pxDefaultSize | ||
| __nextHasNoMarginBottom |
There was a problem hiding this comment.
Testing instructions
Go to the Pages view in Site Editor (wp-admin/site-editor.php?postType=page), and click the Add New Page button.
| Before | After |
|---|---|
 |
 |
@WordPress/gutenberg-design I'm seeing some inconsistency in the spacing before the CTA button in contexts like this. The Figma specs it as 24px — should I follow that everywhere? Also, would the spacing be different in a non-Modal context like Data Views Quick Edit, or a small block toolbar popover like the Language formatter?
There was a problem hiding this comment.
@jameskoster @mattrwalker this would be a great opportunity to fix up the modal component, bring it in sync. What's the right value?
There was a problem hiding this comment.
I think there could be a case for a dedicated ModalFooter (or similar) component that handles this. It should:
- Align the buttons (right)
- Set spacing
- Set the button order (primary always right-most)
- Limit to two buttons
- Handle small-screen behavior (make buttons full width and/or stack)
But that's probably part of a much broader update to Modal. Until we have a clear design for that I don't suppose the spacing matters too much – I'd probably just aim for what exists on trunk.
There was a problem hiding this comment.
Ok, if we don't have spec'ed values yet, I'm going to go with the path of least resistance as long as it's in the general vicinity of what's correct. In a lot of cases, that's going to be a space equidistant between all the control gaps (i.e. throw them all in a single VStack). This is going to be easier to deal with later once we decide on an official value.
| @@ -13,6 +13,7 @@ import { | |||
| Button, | |||
| Popover, | |||
| __experimentalHStack as HStack, | |||
| __experimentalVStack as VStack, | |||
There was a problem hiding this comment.
Testing instructions
In the editor, add a Paragraph block and select some of the text. Select "Language" from the block toolbar.
| Before | After |
|---|---|
 |
 |
Comment on lines
+41
to
+53
| return ( | ||
| <VStack spacing={ 4 }> | ||
| { visibleFields.map( ( field ) => { | ||
| return ( | ||
| <field.Edit | ||
| key={ field.id } | ||
| data={ data } | ||
| field={ field } | ||
| onChange={ onChange } | ||
| /> | ||
| ); | ||
| } ) } | ||
| </VStack> |
There was a problem hiding this comment.
Testing instructions
In Gutenberg experiments (wp-admin/admin.php?page=gutenberg-experiments), enable the "Quick Edit in DataViews" experiment. In the Site Editor, go to Pages (wp-admin/site-editor.php?postType=page) and switch to Table view.
Select a page row. Click the "Show quick edit sidebar" button.
| Before | After |
|---|---|
 |
 |
These changes can also be seen in the Storybook for DataForm.
Comment on lines
+79
to
+80
| __next40pxDefaultSize | ||
| __nextHasNoMarginBottom |
There was a problem hiding this comment.
This is the SelectControl in the Data Views Quick Edit.
| @@ -54,6 +54,7 @@ function Edit< Item >( { | |||
| value={ value ?? '' } | |||
| onChange={ onChangeControl } | |||
| __next40pxDefaultSize | |||
| __nextHasNoMarginBottom | |||
There was a problem hiding this comment.
This is the TextControl in the Data Views Quick Edit.
Comment on lines
+78
to
+96
| <VStack as="form" onSubmit={ onSubmit } spacing={ 4 }> | ||
| <DataForm | ||
| data={ itemWithEdits } | ||
| fields={ fields } | ||
| form={ form } | ||
| onChange={ setEdits } | ||
| /> | ||
| <Button | ||
| variant="primary" | ||
| type="submit" | ||
| accessibleWhenDisabled | ||
| disabled={ isUpdateDisabled } | ||
| > | ||
| { __( 'Update' ) } | ||
| </Button> | ||
| </form> | ||
| <FlexItem> | ||
| <Button | ||
| variant="primary" | ||
| type="submit" | ||
| accessibleWhenDisabled | ||
| disabled={ isUpdateDisabled } | ||
| __next40pxDefaultSize | ||
| > | ||
| { __( 'Update' ) } | ||
| </Button> | ||
| </FlexItem> | ||
| </VStack> |
There was a problem hiding this comment.
This is the form wrapping the Data Views Quick Edit.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #38730
What?
Adds eslint rules to prevent new instances of
TextControlto be introduced in the Gutenberg codebase without the__nextHasNoMarginBottomprop being added.Why?
These lint rules should prevent new violating usages from being added, until we are ready to officially deprecate the margins on the BaseControl-based components all at once.
Testing Instructions
See code comments.