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

FormTokenField: handle disabled in Token; add storybook stories #23713

Closed

Conversation

adekbadek
Copy link
Contributor

Description

In FormTokenField component, the disabled prop was not passed to the Button, which made the delete button clickable. The click would not be handled, but the DOM element was focusable. This PR fixes that by passing the disabled prop to the Button.

How has this been tested?

Locally. Storybook stories were added for the FormTokenField component for ease of testing.

Screenshots

Focusing on the token when the component is disabled:

image

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

The `disabled` prop was not passed to the `Button`, which made it clickable
(the click would not be handled, but the DOM element was focusable).
Base automatically changed from master to trunk March 1, 2021 15:43
@ramonjd ramonjd added the [Package] Components /packages/components label Aug 26, 2021
@ramonjd ramonjd added the [Type] Bug An existing feature does not function as intended label Aug 26, 2021
@t-hamano
Copy link
Contributor

I checked the latest trunk and it appears that Storybook has already been added in #25439. However, the problem of the focus outline appearing on the button still seems to exist.

1a3bc61bf05b10a49ee91c5d57348a1d.mp4

@mirka
Copy link
Member

mirka commented Dec 14, 2023

@adekbadek Are you still available to resolve the conflict and remove the Storybook bit? If not, let us know and we'll find someone to take over 🙂

@adekbadek
Copy link
Contributor Author

Hi @mirka – I don't have bandwidth to do that currently, if it's possible please take over.

@chad1008
Copy link
Contributor

I've just taken a look and with the existing Storybook setup, it does look like all we need is to pass that disabled prop. Rebasing and taking over this PR from a fork isn't ideal so I'm spinning up a new one real quick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants