RangeControl: Add lint rule for 40px size prop usage #64558
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
|
Size Change: -100 B (-0.01%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
mirka
commented
Aug 15, 2024
| @@ -51,6 +51,7 @@ export default function QueryPaginationNumbersEdit( { | |||
| <InspectorControls> | |||
| <PanelBody title={ __( 'Settings' ) }> | |||
| <RangeControl | |||
| __next40pxDefaultSize | |||
There was a problem hiding this comment.
Insert a Query Loop block, choose something with pagination, and click the pagination block to show the block inspector.
| Before | After |
|---|---|
 |
 |
| @@ -231,6 +231,7 @@ export default function PostExcerptEditor( { | |||
| } | |||
| /> | |||
| <RangeControl | |||
| __next40pxDefaultSize | |||
| @@ -264,6 +264,7 @@ function GridLayoutMinimumWidthControl( { layout, onChange } ) { | |||
| </FlexItem> | |||
| <FlexItem isBlock> | |||
| <RangeControl | |||
| __next40pxDefaultSize | |||
There was a problem hiding this comment.
Can be seen in Grid block. No visual change because this is a standalone slider-only RangeControl instance.
| @@ -368,6 +369,7 @@ function GridLayoutColumnsAndRowsControl( { | |||
| /> | |||
| ) : ( | |||
| <RangeControl | |||
| __next40pxDefaultSize | |||
| @@ -249,6 +249,7 @@ export default function SpacingInputControl( { | |||
| } } | |||
| /> | |||
| <RangeControl | |||
| __next40pxDefaultSize | |||
There was a problem hiding this comment.
No visual change because these are standalone slider-only RangeControl instances.
Comment on lines
+35
to
+45
| <DropdownContentWrapper paddingSize="medium"> | ||
| <RangeControl | ||
| __next40pxDefaultSize | ||
| __nextHasNoMarginBottom | ||
| label={ __( 'Zoom' ) } | ||
| min={ MIN_ZOOM } | ||
| max={ MAX_ZOOM } | ||
| value={ Math.round( zoom ) } | ||
| onChange={ setZoom } | ||
| /> | ||
| </DropdownContentWrapper> |
There was a problem hiding this comment.
Insert an Image block, choose the Crop tool in the block toolbar, and open the Zoom dropdown.
| Before | After |
|---|---|
 |
 |
I added some padding to the dropdown content because the slider thumb was almost touching the edge.
| @@ -164,6 +164,7 @@ export default function HeightControl( { | |||
| <FlexItem isBlock> | |||
| <Spacer marginX={ 2 } marginBottom={ 0 }> | |||
| <RangeControl | |||
| __next40pxDefaultSize | |||
There was a problem hiding this comment.
Can be seen in the Group block ▸ Styles ▸ Dimensions ▸ Minimum Height. No visual change because this is a standalone slider-only RangeControl instance.
| @@ -104,6 +104,7 @@ export default function BorderRadiusControl( { onChange, values } ) { | |||
| units={ units } | |||
| /> | |||
| <RangeControl | |||
| __next40pxDefaultSize | |||
There was a problem hiding this comment.
Can be seen in the block inspector ▸ Styles ▸ Border & Shadow of an Image block. No visual change because this is a standalone slider-only RangeControl instance.
mirka
requested review from
tellthemachines,
ellatrix and
ajitbohra
as code owners
|
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. |
28 tasks
|
Flaky tests detected in d46937c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10409746115
|
tyxla
approved these changes
Aug 16, 2024
There was a problem hiding this comment.
LGTM 👍 Thanks!
A few places we could potentially add the new prop to:
- This
RangeControlusage which seems to use an underlyingRangeControlcomponent. RangeControl's storybook examplesRangeControl's tests
Comment on lines
-21
to
-25
| height: 40px; | ||
| /* Vertically center the RangeControl until it has true 40px height. */ | ||
| display: flex; | ||
| align-items: center; | ||
| margin-bottom: 0; |
There was a problem hiding this comment.
Love it that we can clean up all those styles 🚀
Good catch, done in 7f28701 
|
Is it expected that the lint rule didn't pick that up? |
I thought it was because of the suppressed TS error in the instance, but upon further testing it was a configuration error 😓 So complicated... I'll fix it after all the open PRs are merged. |
Thanks for keeping things tidy 🙏 |
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 #63871
What?
Add a lint rule to prevent people from introducing new usages of RangeControl that do not adhere to the new default size.
Testing Instructions
The lint error should trigger for violations as expected.