Skip to content

TextControl: Restrict type prop in TypeScript#45433

Merged
ciampo merged 3 commits intotrunkfrom
try/text-control-limit-type
Nov 28, 2022
Merged

TextControl: Restrict type prop in TypeScript#45433
ciampo merged 3 commits intotrunkfrom
try/text-control-limit-type

Conversation

@walbo
Copy link
Copy Markdown
Member

@walbo walbo commented Oct 31, 2022

What?

Update type props to only allow email, number, password, tel, text, search or url.

Why?

A lot of the default type options doesn't make sense to use with the TextControl component.

How?

  • No runtime changes.
  • Set a list of allowed type. Types remove are:
    • button
    • checkbox
    • color
    • date
    • datetime-local
    • file
    • hidden
    • image
    • month
    • radio
    • range
    • reset
    • submit
    • time
    • week

Testing Instructions

  • Everything should work as before.
  • Test the component in Storybook. Should now only allow you to set type from a select control.

@walbo walbo added the [Package] Components /packages/components label Oct 31, 2022
@walbo walbo requested a review from ajitbohra as a code owner October 31, 2022 23:37
@codesandbox
Copy link
Copy Markdown

codesandbox bot commented Oct 31, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@walbo walbo self-assigned this Oct 31, 2022
@walbo walbo requested review from ciampo and mirka November 1, 2022 10:14
@ciampo ciampo added the [Type] Code Quality Issues or PRs that relate to code quality label Nov 1, 2022
@ciampo ciampo requested a review from chad1008 November 1, 2022 17:31
Copy link
Copy Markdown
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.

The change seems reasonable to me 🚀 but happy to hear other folks' opinions here

- `CustomGradientBar`: Refactor away from Lodash ([#45367](https://github.com/WordPress/gutenberg/pull/45367/)).
- `TextControl`: Set Storybook control types on `help`, `label` and `type` ([#45405](https://github.com/WordPress/gutenberg/pull/45405)).
- `Autocomplete`: use Popover's new `placement` prop instead of legacy `position` prop ([#44396](https://github.com/WordPress/gutenberg/pull/44396/)).
- `TextControl`: Restrict `type` prop to `email`, `number`, `password`, `tel`, `text`, `search` or `url` ([#45433](https://github.com/WordPress/gutenberg/pull/45433/)).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We'll need to move the CHANGELOG entry up to the new Unreleased section

@ciampo
Copy link
Copy Markdown
Contributor

ciampo commented Nov 26, 2022

@mirka quick check that you don't see any issues with restricting the values of the type prop?

@mirka
Copy link
Copy Markdown
Member

mirka commented Nov 28, 2022

@mirka quick check that you don't see any issues with restricting the values of the type prop?

Nope! Plus they're just type annotations so it would be trivial to update if we need any tweaks.

@ciampo ciampo force-pushed the try/text-control-limit-type branch from ae0f052 to 79189e3 Compare November 28, 2022 21:29
@ciampo ciampo enabled auto-merge (squash) November 28, 2022 21:29
@ciampo ciampo merged commit c34f716 into trunk Nov 28, 2022
@ciampo ciampo deleted the try/text-control-limit-type branch November 28, 2022 22:05
@github-actions github-actions bot added this to the Gutenberg 14.7 milestone Nov 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 [Type] Code Quality Issues or PRs that relate to code quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants