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

fix: controlled input forms accessibility violations #7059

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

abdulbasithqb
Copy link
Contributor

What does this PR do?

controlled input forms storybook

Where should the reviewer start?

storybook

What testing has been done on this PR?

yarn test

How should this be manually tested?

Check the accessibility tab in the Forms/ControlledInput storybook

Do Jest tests follow these best practices?

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

What are the relevant issues?

#6124

Screenshots (if appropriate)

Screenshot 2023-12-12 at 6 01 22 PM

Do the grommet docs need to be updated?

No

Should this PR be mentioned in the release notes?

No

Is this change backwards compatible or is it a breaking change?

No

Copy link
Collaborator

@jcfilben jcfilben left a comment

Choose a reason for hiding this comment

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

I am still seeing an accessibility issue on the Form/ControlledInput story. I believe the 'interactive controls must not be nested' is part of a larger issue originating in the Select component.

@abdulbasithqb
Copy link
Contributor Author

I am still seeing an accessibility issue on the Form/ControlledInput story. I believe the 'interactive controls must not be nested' is part of a larger issue originating in the Select component.

Thanks for the review,
Encountering the same problem after restarting Metro. Not sure why it didn't show earlier. It seems the Select component might be the culprit, as it's nested inside the input. Probably need to update existing Select components since they have a button nested inside.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants