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

InputControl: Add flag for larger default size #40622

Merged
merged 2 commits into from
Apr 28, 2022
Merged

Conversation

mirka
Copy link
Member

@mirka mirka commented Apr 26, 2022

Part of #39397

What?

Adds the temporary prop __next36pxDefaultSize to opt into the new 36px height default size.

Dependent components

This also technically adds __next36pxDefaultSize support to any higher-level component that forwards props onto InputControl.

  • For NumberControl, I checked locally that it looks as expected. I don't think it's worth adding the prop to Storybook at the moment — I'd prefer waiting until the TS conversion + Knobs-to-Controls rewrite.
  • For UnitControl, I'll do a separate PR to adjust paddings and add "official" support for it via TypeScript. (Which will also add it to Storybook) UnitControl: Add flag for larger default size #40627

Why?

As part of the effort to move toward consistent component sizes.

How?

Details of the migration plan are written in the original issue #39397.

Testing Instructions

  1. npm run storybook:dev
  2. Go to the story for InputControl. By setting the __next36pxDefaultSize control to true, the height of the default size variant should increase to 36px. All other size variants are unchanged.

Screenshots or screencast

Old default

Old default height

New default (opt-in for now)

New default height

@mirka mirka added the [Package] Components /packages/components label Apr 26, 2022
@mirka mirka requested a review from ciampo April 26, 2022 13:38
@mirka mirka self-assigned this Apr 26, 2022
@mirka mirka requested a review from ajitbohra as a code owner April 26, 2022 13:38
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.

LGTM 🚀

Would we need need to expose this prop in RangeControl too, similarly to what #40627 does for UnitControl?

Also cc'ing @aaronrobertshaw since this should also affect RangeControl (alco cc'ing @aaroncampbell in case there's any changes to make in #40535 once this gets merged)

@mirka
Copy link
Member Author

mirka commented Apr 28, 2022

Would we need need to expose this prop in RangeControl too, similarly to what #40627 does for UnitControl?

Yep! That and a few more higher-level components listed in "Stage 2" here.

@mirka mirka merged commit be92ce8 into trunk Apr 28, 2022
@mirka mirka deleted the input-control-36px branch April 28, 2022 11:38
@github-actions github-actions bot added this to the Gutenberg 13.2 milestone Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants