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

UnitControl: Add lint rule for 40px size prop usage #64520

Merged
merged 16 commits into from
Aug 22, 2024
17 changes: 10 additions & 7 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ module.exports = {
'FontSizePicker',
'FormTokenField',
'InputControl',
'LetterSpacingControl',
'LineHeightControl',
'NumberControl',
'RangeControl',
Expand All @@ -341,13 +342,15 @@ module.exports = {
'FormFileUpload should have the `__next40pxDefaultSize` prop to opt-in to the new default size.',
},
// Temporary rules until all existing components have the `__next40pxDefaultSize` prop.
...[ 'SelectControl' ].map( ( componentName ) => ( {
// Not strict. Allows pre-existing __next40pxDefaultSize={ false } usage until they are all manually updated.
selector: `JSXOpeningElement[name.name="${ componentName }"]:not(:has(JSXAttribute[name.name="__next40pxDefaultSize"])):not(:has(JSXAttribute[name.name="size"]))`,
message:
componentName +
' should have the `__next40pxDefaultSize` prop to opt-in to the new default size.',
} ) ),
...[ 'SelectControl', 'UnitControl' ].map(
( componentName ) => ( {
// Not strict. Allows pre-existing __next40pxDefaultSize={ false } usage until they are all manually updated.
selector: `JSXOpeningElement[name.name="${ componentName }"]:not(:has(JSXAttribute[name.name="__next40pxDefaultSize"])):not(:has(JSXAttribute[name.name="size"]))`,
message:
componentName +
' should have the `__next40pxDefaultSize` prop to opt-in to the new default size.',
} )
),
],
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,8 @@ export default function DimensionsPanel( {
>
<HStack alignment="flex-end" justify="flex-start">
<UnitControl
// TODO: Switch to `true` (40px size) if possible (https://github.com/WordPress/gutenberg/pull/64520#discussion_r1717314262)
__next40pxDefaultSize={ false }
label={ __( 'Content' ) }
labelPosition="top"
__unstableInputWidth="80px"
Expand Down Expand Up @@ -504,6 +506,8 @@ export default function DimensionsPanel( {
>
<HStack alignment="flex-end" justify="flex-start">
<UnitControl
// TODO: Switch to `true` (40px size) if possible
__next40pxDefaultSize={ false }
label={ __( 'Wide' ) }
labelPosition="top"
__unstableInputWidth="80px"
Expand Down Expand Up @@ -611,6 +615,9 @@ export default function DimensionsPanel( {
}
className={ clsx( {
'tools-panel-item-spacing': showSpacingPresetsControl,
'single-column':
// If UnitControl is used, should be single-column.
! showSpacingPresetsControl && ! isAxialGap,
} ) }
panelId={ panelId }
>
Expand All @@ -628,8 +635,8 @@ export default function DimensionsPanel( {
/>
) : (
<UnitControl
__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.

Layout Dimensions

label={ __( 'Block spacing' ) }
__unstableInputWidth="80px"
min={ 0 }
onChange={ setGapValue }
units={ units }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,20 @@ The current value of the letter spacing setting.

A callback function invoked when the value is changed.

### `_unstableInputWidth`
### `__unstableInputWidth`
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed typo


- **Type:** `string|number|undefined`
- **Default:** `undefined`

Input width to pass through to inner UnitControl. Should be a valid CSS value.

#### `__next40pxDefaultSize`

- **Type:** `boolean`
- **Default:** `false`

Start opting into the larger default height that will become the default size in a future version.

## Related components

Block Editor components are components that can be used to compose the UI of your block editor. Thus, they can only be used under a [`BlockEditorProvider`](https://github.com/WordPress/gutenberg/blob/HEAD/packages/block-editor/src/components/provider/README.md) in the components tree.
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,16 @@ import { useSettings } from '../../components/use-settings';
/**
* Control for letter-spacing.
*
* @param {Object} props Component props.
* @param {string} props.value Currently selected letter-spacing.
* @param {Function} props.onChange Handles change in letter-spacing selection.
* @param {string|number|undefined} props.__unstableInputWidth Input width to pass through to inner UnitControl. Should be a valid CSS value.
* @param {Object} props Component props.
* @param {boolean} props.__next40pxDefaultSize Start opting into the larger default height that will become the default size in a future version.
* @param {string} props.value Currently selected letter-spacing.
* @param {Function} props.onChange Handles change in letter-spacing selection.
* @param {string|number|undefined} props.__unstableInputWidth Input width to pass through to inner UnitControl. Should be a valid CSS value.
*
* @return {Element} Letter-spacing control.
*/
export default function LetterSpacingControl( {
__next40pxDefaultSize = false,
value,
onChange,
__unstableInputWidth = '60px',
Expand All @@ -35,6 +37,7 @@ export default function LetterSpacingControl( {
} );
return (
<UnitControl
__next40pxDefaultSize={ __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 was already passed through via otherProps, but making it explicit here.

{ ...otherProps }
label={ __( 'Letter spacing' ) }
value={ value }
Expand Down
4 changes: 4 additions & 0 deletions packages/block-editor/src/layouts/constrained.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ export default {
<div className="block-editor-hooks__layout-controls">
<div className="block-editor-hooks__layout-controls-unit">
<UnitControl
// TODO: Switch to `true` (40px size) if possible (https://github.com/WordPress/gutenberg/pull/64520#discussion_r1717314262)
__next40pxDefaultSize={ false }
className="block-editor-hooks__layout-controls-unit-input"
label={ __( 'Content' ) }
labelPosition="top"
Expand All @@ -96,6 +98,8 @@ export default {
</div>
<div className="block-editor-hooks__layout-controls-unit">
<UnitControl
// TODO: Switch to `true` (40px size) if possible
__next40pxDefaultSize={ false }
className="block-editor-hooks__layout-controls-unit-input"
label={ __( 'Wide' ) }
labelPosition="top"
Expand Down
5 changes: 3 additions & 2 deletions packages/block-library/src/cover/edit/inspector-controls.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ function CoverHeightInput( {

return (
<UnitControl
label={ __( 'Minimum height of cover' ) }
__next40pxDefaultSize
Copy link
Member Author

@mirka mirka Aug 15, 2024

Choose a reason for hiding this comment

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

Before After
Cover block inspector, before Cover block inspector, after

Tweaked as discussed in #64520 (comment)

label={ __( 'Minimum height' ) }
id={ inputId }
isResetValueOnUnitChange
min={ min }
onChange={ handleOnChange }
onUnitChange={ onUnitChange }
__unstableInputWidth="80px"
units={ units }
value={ computedValue }
/>
Expand Down Expand Up @@ -299,6 +299,7 @@ export default function CoverInspectorControls( {
) }
<InspectorControls group="dimensions">
<ToolsPanelItem
className="single-column"
hasValue={ () => !! minHeight }
label={ __( 'Minimum height' ) }
onDeselect={ () =>
Expand Down
4 changes: 2 additions & 2 deletions packages/block-library/src/cover/test/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,10 @@ describe( 'Cover block', () => {
} )
);
await userEvent.clear(
screen.getByLabelText( 'Minimum height of cover' )
screen.getByLabelText( 'Minimum height' )
);
await userEvent.type(
screen.getByLabelText( 'Minimum height of cover' ),
screen.getByLabelText( 'Minimum height' ),
'300'
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,7 @@ const DimensionControls = ( {
panelId={ clientId }
>
<SelectControl
// TODO: Switch to `true` (40px size) if possible
__next40pxDefaultSize={ false }
__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.

Before After
Featured Image block inspector, before Featured Image block inspector, after

Incorrect column gap will be addressed separately at #64488

__nextHasNoMarginBottom
label={ __( 'Aspect ratio' ) }
value={ aspectRatio }
Expand All @@ -157,6 +156,7 @@ const DimensionControls = ( {
panelId={ clientId }
>
<UnitControl
__next40pxDefaultSize
label={ __( 'Height' ) }
labelPosition="top"
value={ height || '' }
Expand All @@ -179,6 +179,7 @@ const DimensionControls = ( {
panelId={ clientId }
>
<UnitControl
__next40pxDefaultSize
label={ __( 'Width' ) }
labelPosition="top"
value={ width || '' }
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/specs/editor/blocks/cover.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ test.describe( 'Cover', () => {

// Ensure there the default value for the minimum height of cover is undefined.
const defaultHeightValue = await coverBlockEditorSettings
.getByLabel( 'Minimum height of cover' )
.getByLabel( 'Minimum height' )
.inputValue();
expect( defaultHeightValue ).toBeFalsy();

Expand Down
Loading