Skip to content

Conversation

ShimiSun
Copy link
Collaborator

What does this PR do?

Fixes accessibility issues in storybook for ArrayOfFormFields

Where should the reviewer start?

storybook file

What testing has been done on this PR?

local

How should this be manually tested?

strorybook

Do Jest tests follow these best practices?

N/A

Any background context you want to provide?

nope

What are the relevant issues?

#4068 #6124

Screenshots (if appropriate)

Before
image (1)

After
image

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?

b-c

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.

Looks good!

@ericsoderberghp
Copy link
Contributor

Should we be automatically setting aria-label to the label property if no aria-label is provided?

@britt6612
Copy link
Collaborator

Should we be automatically setting aria-label to the label property if no aria-label is provided?

good point we could or maybe a console warning for it? Im open to discussing either

@ShimiSun
Copy link
Collaborator Author

ShimiSun commented Nov 7, 2022

Should we be automatically setting aria-label to the label property if no aria-label is provided?

Good comment. Yea, we can optimize FormField to set aria-label value from label, but we would need to ensure backward compatibility in case aria is provided.

That being said, we should also consider #6466 (comment)

@ericsoderberghp ericsoderberghp merged commit 8bb2c8a into master Nov 8, 2022
@ericsoderberghp ericsoderberghp deleted the fix/axe-storybook branch November 8, 2022 19:30
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.

4 participants