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

TextControl: Restrict type prop in TypeScript #45433

Merged
merged 3 commits into from
Nov 28, 2022
Merged

Conversation

walbo
Copy link
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

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
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

packages/components/src/text-control/stories/index.tsx Outdated Show resolved Hide resolved
@@ -54,6 +54,7 @@
- `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
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
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
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