Skip to content

Conversation

jashanshah33
Copy link
Contributor

What does this PR do?
Fixed label issues in storybook for AggregateValidation

Where should the reviewer start?
storybook

What testing has been done on this PR?
local

How should this be manually tested?
storybook

Do Jest tests follow these best practices?
none

Any background context you want to provide?
no

What are the relevant issues?
#6124

Screenshots (if appropriate)
Before:
before

After:
After

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?
backwards compatible

Signed-off-by: jashanshah33 <jashanshah33@gmail.com>
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.

Same question as #6464 (comment). It seems like if we provide a label to FormField and don't have an aria-label or a11yTitle that should be used as the aria-label. This would take care of the accessibility issue and we wouldn't need to change anything about this story

@jashanshah33
Copy link
Contributor Author

jashanshah33 commented Nov 3, 2022

Hi @jcfilben ,
What should be the value of aria-label or a11yTitle, if we provide lable to form field ?

@jcfilben
Copy link
Collaborator

jcfilben commented Nov 3, 2022

Hi @jcfilben ,
What should be the value of aria-label or a11yTitle, if we provide lable to form field ?

If the form has a label and neither aria-label or a11yTitle are defined, the aria-label should be the form label

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.

Update on this, I chatted with some other members of the team and we decided to hold off on using the label from FormField as the aria-label. We opened a separate issue to explore this #6483.

@jcfilben jcfilben added the waiting Awaiting response to latest comments label Nov 9, 2022
jashanshah33 and others added 3 commits November 11, 2022 21:07
Co-authored-by: Jessica Filben <54560994+jcfilben@users.noreply.github.com>
Co-authored-by: Jessica Filben <54560994+jcfilben@users.noreply.github.com>
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!

@jcfilben jcfilben removed the waiting Awaiting response to latest comments label Nov 14, 2022
@ericsoderberghp ericsoderberghp merged commit 67795aa into grommet:master Nov 14, 2022
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.

3 participants