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

a11y: add ariaLabel props to EditableField and TextFields #2259

Merged
merged 8 commits into from
Mar 12, 2020

Conversation

beyackle
Copy link
Contributor

This adds a new prop to EditableField so we can wire through the accessibility information wherever it's needed.

Description

EditableField uses a TextField under the hood, which already supports "ariaLabel" as a prop, so we just need to wire that through directly as a new prop and then set sensible labels on all the EditableFields.

Task Item

Closes #2140

This adds a new prop to EditableField so we can wire through the accessibility information wherever it's needed.
@github-actions
Copy link

Coverage Status

Coverage decreased (-0.003%) to 40.66% when pulling d9a6b43 on beyackle/ariaLabelFields into d91eaa4 on master.

@beyackle beyackle changed the title add ariaLabel prop to EditableField a11y: add ariaLabel prop to EditableField Mar 12, 2020
@beyackle beyackle changed the title a11y: add ariaLabel prop to EditableField a11y: add ariaLabel props to EditableField and TextFields Mar 12, 2020
Copy link
Contributor

@a-b-r-o-w-n a-b-r-o-w-n left a comment

Choose a reason for hiding this comment

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

I think we should use the placeholder string on a lot of these. They add some additional context to the field.

@corinagum
Copy link
Contributor

Only thing with making the placeholder same as the label is that sometimes the screenreader will read both. Should check for this before merging :)

@cwhitten cwhitten merged commit eb0188b into master Mar 12, 2020
@cwhitten cwhitten deleted the beyackle/ariaLabelFields branch March 12, 2020 22:22
@a-b-r-o-w-n a-b-r-o-w-n mentioned this pull request Mar 18, 2020
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