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.

Sign up for GitHub

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

Jump to bottom

QueryControls: Add opt-in prop for 40px default size #56576

Merged
merged 3 commits into from
Nov 30, 2023
Merged

QueryControls: Add opt-in prop for 40px default size #56576

merged 3 commits into from
Nov 30, 2023

Conversation

brookewp
Copy link
Contributor

Part of #46741

What?

Add a new opt-in prop __next40pxDefaultSize to QueryControls, following the plan outlined in above mentioned PR.

Why?

For more consistency in styling.

How?

A new, temporary __next40pxDefaultSize prop. When the prop is set to true, the inputs will have a height of 40px.

Testing Instructions

In Storybook:

  1. npm run storybook:dev
  2. Set __next40pxDefaultSize to true for QueryControls
  3. The input height of the component should now be 40px

In the editor

Smoke test the component in the editor; QueryControls shouldn't have any visible changes for now.
@brookewp brookewp added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Nov 28, 2023
@brookewp brookewp self-assigned this Nov 28, 2023
@brookewp
Copy link
Contributor Author

The RangeControl in QueryControls already has the prop enabled:

gutenberg/packages/components/src/query-controls/index.tsx

Lines 178 to 180 in 05dcf4b

<RangeControl
__nextHasNoMarginBottom
__next40pxDefaultSize

Would we want to set all to true for consistency instead of making it opt-in? Or make RangeControl optional?
@brookewp brookewp requested a review from a team November 28, 2023 00:23
@ciampo
Copy link
Contributor

ciampo commented Nov 29, 2023

I would pass the __next40pxDefaultSize to RangeControl, instead of having it hardcoded to true — which means that we will then need to pass __next40pxDefaultSize to instances of QueryControls around the editor (cc @richtabor )
ciampo
ciampo reviewed Nov 29, 2023
View reviewed changes
Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

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

Things are looking good, probably this is good to be merged after the __next40pxDefaultSize prop gets passed to RangeControl
@brookewp brookewp requested a review from ciampo November 30, 2023 01:50
ciampo
ciampo approved these changes Nov 30, 2023
View reviewed changes
Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

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

🚀
@brookewp brookewp merged commit aa19787 into trunk Nov 30, 2023
53 checks passed
@brookewp brookewp deleted the add/40pxsize-querycontrols branch November 30, 2023 17:32
@github-actions github-actions bot added this to the Gutenberg 17.3 milestone Nov 30, 2023
derekblank pushed a commit that referenced this pull request Dec 7, 2023
@brookewp @derekblank
QueryControls: Add opt-in prop for 40px default size (#56576)
b6bd315 
* `QueryControls`: Add opt-in prop for 40px default size

* Update RangeControl 40px size to be opt-in

* Update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
2 participants
@brookewp @ciampo