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

NumberControl: Add lint rule for 40px size prop usage #64561

Merged
merged 1 commit into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ module.exports = {
'BorderControl',
'DimensionControl',
'FontSizePicker',
'NumberControl',
'ToggleGroupControl',
].map( ( componentName ) => ( {
// Falsy `__next40pxDefaultSize` without a non-default `size` prop.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ The value of the line height.

A callback function that handles the application of the line height 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 @@ -16,6 +16,8 @@ import {
} from './utils';

const LineHeightControl = ( {
/** Start opting into the larger default height that will become the default size in a future version. */
__next40pxDefaultSize = false,
Comment on lines +19 to +20
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 being passed through via otherProps, but making it explicit here. All in-app usage already uses the correct size.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thought: instead of exposing __next40pxDefaultSize, could we instead just set __next40pxDefaultSize to true on NumberControl ? Or do we still care about offering consumers the option to opt-out of the new size, for the time being?

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered it, but checked third-party usage of some of these Typography controls and decided against it. They were used more than I thought, and some of them still use them in the old sizes, so decided against it.

value: lineHeight,
onChange,
__unstableInputWidth = '60px',
Expand Down Expand Up @@ -91,6 +93,7 @@ const LineHeightControl = ( {
<div className="block-editor-line-height-control">
<NumberControl
{ ...otherProps }
__next40pxDefaultSize={ __next40pxDefaultSize }
__unstableInputWidth={ __unstableInputWidth }
__unstableStateReducer={ stateReducer }
onChange={ handleOnChange }
Expand Down
Loading