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

Add margin-bottom lint rules for BaseControl #64355

Merged
merged 12 commits into from
Aug 12, 2024
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ module.exports = {
...restrictedSyntax,
...restrictedSyntaxComponents,
...[
'BaseControl',
'CheckboxControl',
'ComboboxControl',
'DimensionControl',
Expand Down
5 changes: 4 additions & 1 deletion packages/block-library/src/gallery/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,10 @@ export default function GalleryEdit( props ) {
/>
) }
{ Platform.isWeb && ! imageSizeOptions && hasImageIds && (
<BaseControl className="gallery-image-sizes">
<BaseControl
className="gallery-image-sizes"
__nextHasNoMarginBottom
Copy link
Member Author

Choose a reason for hiding this comment

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

Testing instructions

Somewhere in this file, set the imageSizeOptions variable as false. In the editor, add a Gallery block with an image, and see the block inspector.

Resolution loading placeholder

>
<BaseControl.VisualLabel>
{ __( 'Resolution' ) }
</BaseControl.VisualLabel>
Expand Down
18 changes: 9 additions & 9 deletions packages/block-library/src/search/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
ToolbarButton,
ResizableBox,
PanelBody,
BaseControl,
__experimentalVStack as VStack,
__experimentalUseCustomUnits as useCustomUnits,
__experimentalUnitControl as UnitControl,
} from '@wordpress/components';
Expand Down Expand Up @@ -408,12 +408,14 @@ export default function SearchEdit( {

<InspectorControls>
<PanelBody title={ __( 'Settings' ) }>
<BaseControl
label={ __( 'Width' ) }
id={ unitControlInputId }
<VStack
Copy link
Member Author

Choose a reason for hiding this comment

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

Testing instructions

In the editor, add a Search block and see the block inspector.

Before After
Search block inspector, before Search block inspector, after

This UI probably should be updated to DimensionTool, but I'll just do a quick fix here.

Spacing is the same as a similar pattern in the Latest Posts block.

Copy link
Member

Choose a reason for hiding this comment

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

Looks good, but it would have been suitable for another PR, perhaps with a design person taking a look 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

You may be right! This is still bad though, just incrementally better. I'm keeping an eye on #52223 to figure out what this UI should be ideally.

className="wp-block-search__inspector-controls"
spacing={ 4 }
>
<UnitControl
id={ unitControlInputId }
__next40pxDefaultSize
label={ __( 'Width' ) }
id={ unitControlInputId } // unused, kept for backwards compatibility
min={
isPercentageUnit( widthUnit ) ? 0 : MIN_WIDTH
}
Expand All @@ -427,7 +429,6 @@ export default function SearchEdit( {
parseInt( newWidth, 10 ) > 100
? 100
: newWidth;

setAttributes( {
width: parseInt( filteredWidth, 10 ),
} );
Expand All @@ -445,9 +446,8 @@ export default function SearchEdit( {
value={ `${ width }${ widthUnit }` }
units={ units }
/>

<ButtonGroup
className="wp-block-search__components-button-group"
className="wp-block-search__components-button-group" // unused, kept for backwards compatibility
aria-label={ __( 'Percentage Width' ) }
>
{ [ 25, 50, 75, 100 ].map( ( widthValue ) => {
Expand All @@ -473,7 +473,7 @@ export default function SearchEdit( {
);
} ) }
</ButtonGroup>
</BaseControl>
</VStack>
</PanelBody>
</InspectorControls>
</>
Expand Down
7 changes: 5 additions & 2 deletions packages/block-library/src/search/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@
justify-content: center;
text-align: center;
}
}

&__components-button-group {
margin-top: 10px;
.wp-block-search__inspector-controls {
.components-base-control {
// Counteract the margin added by the block inspector.
margin-bottom: 0;
}
}
69 changes: 36 additions & 33 deletions packages/block-library/src/tag-cloud/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import {
__experimentalUnitControl as UnitControl,
__experimentalUseCustomUnits as useCustomUnits,
__experimentalParseQuantityAndUnitFromRawValue as parseQuantityAndUnitFromRawValue,
__experimentalVStack as VStack,
Disabled,
BaseControl,
} from '@wordpress/components';
import { useSelect } from '@wordpress/data';
import { __ } from '@wordpress/i18n';
Expand Down Expand Up @@ -110,17 +110,20 @@ function TagCloudEdit( { attributes, setAttributes } ) {
const inspectorControls = (
<InspectorControls>
<PanelBody title={ __( 'Settings' ) }>
<SelectControl
__nextHasNoMarginBottom
__next40pxDefaultSize
label={ __( 'Taxonomy' ) }
options={ getTaxonomyOptions() }
value={ taxonomy }
onChange={ ( selectedTaxonomy ) =>
setAttributes( { taxonomy: selectedTaxonomy } )
}
/>
<BaseControl>
Copy link
Member Author

Choose a reason for hiding this comment

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

Testing instructions

In the editor, add a Tag Cloud block and see the block inspector.

Before After
Tag Cloud block inspector, before Tag Cloud block inspector, after

I removed the unnecessary BaseControl and normalized the uneven spacing.

<VStack
spacing={ 4 }
className="wp-block-tag-cloud__inspector-settings"
>
<SelectControl
__nextHasNoMarginBottom
__next40pxDefaultSize
label={ __( 'Taxonomy' ) }
options={ getTaxonomyOptions() }
value={ taxonomy }
onChange={ ( selectedTaxonomy ) =>
setAttributes( { taxonomy: selectedTaxonomy } )
}
/>
<Flex gap={ 4 }>
<FlexItem isBlock>
<UnitControl
Expand Down Expand Up @@ -155,27 +158,27 @@ function TagCloudEdit( { attributes, setAttributes } ) {
/>
</FlexItem>
</Flex>
</BaseControl>
<RangeControl
__nextHasNoMarginBottom
__next40pxDefaultSize
label={ __( 'Number of tags' ) }
value={ numberOfTags }
onChange={ ( value ) =>
setAttributes( { numberOfTags: value } )
}
min={ MIN_TAGS }
max={ MAX_TAGS }
required
/>
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Show tag counts' ) }
checked={ showTagCounts }
onChange={ () =>
setAttributes( { showTagCounts: ! showTagCounts } )
}
/>
<RangeControl
__nextHasNoMarginBottom
__next40pxDefaultSize
label={ __( 'Number of tags' ) }
value={ numberOfTags }
onChange={ ( value ) =>
setAttributes( { numberOfTags: value } )
}
min={ MIN_TAGS }
max={ MAX_TAGS }
required
/>
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Show tag counts' ) }
checked={ showTagCounts }
onChange={ () =>
setAttributes( { showTagCounts: ! showTagCounts } )
}
/>
</VStack>
</PanelBody>
</InspectorControls>
);
Expand Down
8 changes: 8 additions & 0 deletions packages/block-library/src/tag-cloud/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,11 @@
margin: 0;
padding: 0;
}

.wp-block-tag-cloud__inspector-settings {
.components-base-control,
.components-base-control:last-child {
// Cancel out extra margins added by block inspector
margin-bottom: 0;
}
}
Comment on lines +11 to +17
Copy link
Member Author

Choose a reason for hiding this comment

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

So painful 😭 I'm going to propose a better way to deal with these once the deprecation work is done.

Copy link
Member

Choose a reason for hiding this comment

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

Indeed. Let's definitely open an issue to keep track of those 👍

1 change: 1 addition & 0 deletions packages/block-library/src/video/edit-common-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ const VideoSettings = ( { setAttributes, attributes } ) => {
) }
/>
<SelectControl
__next40pxDefaultSize
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the "Preload" dropdown in the Video block inspector controls. (It's named like it's a reused component, but currently only used in the Video block.)

__nextHasNoMarginBottom
label={ __( 'Preload' ) }
value={ preload }
Expand Down
4 changes: 2 additions & 2 deletions packages/block-library/src/video/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ function VideoEdit( {
attributes={ attributes }
/>
<MediaUploadCheck>
<BaseControl className="editor-video-poster-control">
<div className="editor-video-poster-control">
Copy link
Member Author

Choose a reason for hiding this comment

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

Testing instructions

In the block editor, add a Video block and see the block inspector.

Before After
Video block inspector, before Video block inspector, after

Switched the unnecessary BaseControl to a div.

<BaseControl.VisualLabel>
{ __( 'Poster image' ) }
</BaseControl.VisualLabel>
Expand Down Expand Up @@ -265,7 +265,7 @@ function VideoEdit( {
{ __( 'Remove' ) }
</Button>
) }
</BaseControl>
</div>
</MediaUploadCheck>
</PanelBody>
</InspectorControls>
Expand Down
7 changes: 5 additions & 2 deletions packages/components/src/base-control/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const MyCustomTextareaControl = ({ children, ...baseProps }) => (
const { baseControlProps, controlProps } = useBaseControlProps( baseProps );

return (
<BaseControl { ...baseControlProps } __nextHasNoMarginBottom={ true }>
<BaseControl { ...baseControlProps } __nextHasNoMarginBottom>
<textarea { ...controlProps }>
{ children }
</textarea>
Expand Down Expand Up @@ -92,7 +92,10 @@ It should only be used in cases where the children being rendered inside BaseCon
import { BaseControl } from '@wordpress/components';

const MyBaseControl = () => (
<BaseControl help="This button is already accessibly labeled.">
<BaseControl
__nextHasNoMarginBottom
help="This button is already accessibly labeled."
>
<BaseControl.VisualLabel>Author</BaseControl.VisualLabel>
<Button>Select an author</Button>
</BaseControl>
Expand Down
89 changes: 48 additions & 41 deletions packages/components/src/base-control/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,29 +26,6 @@ import { contextConnectWithoutRef, useContextSystem } from '../context';

export { useBaseControlProps } from './hooks';

/**
* `BaseControl` is a component used to generate labels and help text for components handling user inputs.
*
* ```jsx
* import { BaseControl, useBaseControlProps } from '@wordpress/components';
*
* // Render a `BaseControl` for a textarea input
* const MyCustomTextareaControl = ({ children, ...baseProps }) => (
* // `useBaseControlProps` is a convenience hook to get the props for the `BaseControl`
* // and the inner control itself. Namely, it takes care of generating a unique `id`,
* // properly associating it with the `label` and `help` elements.
* const { baseControlProps, controlProps } = useBaseControlProps( baseProps );
*
* return (
* <BaseControl { ...baseControlProps } __nextHasNoMarginBottom={ true }>
* <textarea { ...controlProps }>
* { children }
* </textarea>
* </BaseControl>
* );
* );
* ```
*/
const UnconnectedBaseControl = (
props: WordPressComponentProps< BaseControlProps, null >
) => {
Expand Down Expand Up @@ -105,23 +82,6 @@ const UnconnectedBaseControl = (
);
};

/**
* `BaseControl.VisualLabel` is used to render a purely visual label inside a `BaseControl` component.
*
* It should only be used in cases where the children being rendered inside `BaseControl` are already accessibly labeled,
* e.g., a button, but we want an additional visual label for that section equivalent to the labels `BaseControl` would
* otherwise use if the `label` prop was passed.
*
* @example
* import { BaseControl } from '@wordpress/components';
*
* const MyBaseControl = () => (
* <BaseControl help="This button is already accessibly labeled.">
* <BaseControl.VisualLabel>Author</BaseControl.VisualLabel>
* <Button>Select an author</Button>
* </BaseControl>
* );
*/
const UnforwardedVisualLabel = (
props: WordPressComponentProps< BaseControlVisualLabelProps, 'span' >,
ref: ForwardedRef< any >
Expand All @@ -141,9 +101,56 @@ const UnforwardedVisualLabel = (

export const VisualLabel = forwardRef( UnforwardedVisualLabel );

/**
* `BaseControl` is a component used to generate labels and help text for components handling user inputs.
*
Comment on lines +104 to +106
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved the JSDocs so they are correctly associated with the exported components. Also made sure the code snippets both have the no margin prop.

* ```jsx
* import { BaseControl, useBaseControlProps } from '@wordpress/components';
*
* // Render a `BaseControl` for a textarea input
* const MyCustomTextareaControl = ({ children, ...baseProps }) => (
* // `useBaseControlProps` is a convenience hook to get the props for the `BaseControl`
* // and the inner control itself. Namely, it takes care of generating a unique `id`,
* // properly associating it with the `label` and `help` elements.
* const { baseControlProps, controlProps } = useBaseControlProps( baseProps );
*
* return (
* <BaseControl { ...baseControlProps } __nextHasNoMarginBottom>
* <textarea { ...controlProps }>
* { children }
* </textarea>
* </BaseControl>
* );
* );
* ```
*/
export const BaseControl = Object.assign(
contextConnectWithoutRef( UnconnectedBaseControl, 'BaseControl' ),
{ VisualLabel }

{
/**
* `BaseControl.VisualLabel` is used to render a purely visual label inside a `BaseControl` component.
*
* It should only be used in cases where the children being rendered inside `BaseControl` are already accessibly labeled,
* e.g., a button, but we want an additional visual label for that section equivalent to the labels `BaseControl` would
* otherwise use if the `label` prop was passed.
*
* ```jsx
* import { BaseControl } from '@wordpress/components';
*
* const MyBaseControl = () => (
* <BaseControl
* __nextHasNoMarginBottom
* help="This button is already accessibly labeled."
* >
* <BaseControl.VisualLabel>Author</BaseControl.VisualLabel>
* <Button>Select an author</Button>
* </BaseControl>
* );
* ```
*/
VisualLabel,
}
);

export default BaseControl;
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ import {

const DEFAULT_UNITS = [ 'px', 'em', 'rem', 'vw', 'vh' ];

function SizeControl( props ) {
function SizeControl( {
// Do not allow manipulation of margin bottom
__nextHasNoMarginBottom,
Comment on lines +24 to +25
Copy link
Member Author

Choose a reason for hiding this comment

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

No back compat concerns because this component is private.

...props
} ) {
const { baseControlProps } = useBaseControlProps( props );
const { value, onChange, fallbackValue, disabled } = props;

Expand All @@ -45,7 +49,7 @@ function SizeControl( props ) {
};

return (
<BaseControl { ...baseControlProps }>
<BaseControl { ...baseControlProps } __nextHasNoMarginBottom>
Copy link
Member Author

Choose a reason for hiding this comment

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

Testing instructions

In Global Styles, go to Typography ▸ Font size presets ▸ (whatever option). Enable the Fluid typography and Custom fluid values toggles.

Before After
Font size presets, before Font size presets, after

I normalized the spacing.

I extracted out a labeling bug as a separate issue: #64406.

<Flex>
<FlexItem isBlock>
<UnitControl
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ function PushChangesToGlobalStylesControl( {

return (
<BaseControl
__nextHasNoMarginBottom
Copy link
Member Author

Choose a reason for hiding this comment

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

Testing instructions

In the Site Editor, open a page with a Heading block, or add one. In the block inspector, expanded the Advanced section to see the "Apply globally" control at the bottom.

Apply globally control

className="edit-site-push-changes-to-global-styles-control"
help={ sprintf(
// translators: %s: Title of the block e.g. 'Heading'.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ export function CreateTemplatePartModalContents( {
required
/>
<BaseControl
__nextHasNoMarginBottom
Copy link
Member Author

Choose a reason for hiding this comment

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

Testing instructions

In the Site Editor, go to Patterns ▸ Add New Pattern button ▸ Add new template part.

Add new template part modal

label={ __( 'Area' ) }
id={ `editor-create-template-part-modal__area-selection-${ instanceId }` }
className="editor-create-template-part-modal__area-base-control"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ function PatternOverridesControls( {
<>
<InspectorControls group="advanced">
<BaseControl
__nextHasNoMarginBottom
Copy link
Member Author

Choose a reason for hiding this comment

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

Testing instructions

In the editor, add an Image block. From the block toolbar three-dot menu, choose "Create pattern". Click "Edit original". In the block inspector, expanded the Advanced section to see the Enable overrides button at the bottom.

Edit pattern original block inspector

id={ controlId }
label={ __( 'Overrides' ) }
help={ helpText }
Expand Down
Loading