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.

Sign up for GitHub

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

Jump to bottom

Add aria-hidden and role to SVGs. #1032

Merged
merged 1 commit into from
Jun 6, 2017
Merged

Add aria-hidden and role to SVGs. #1032

merged 1 commit into from
Jun 6, 2017

Conversation

jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Jun 6, 2017 •
edited
Loading

This PR adds aria-hidden="true" and role="img" to the SVG files inside the dashicons component, addressing feedback here: #1012 (comment)

Upstream PR: WordPress/dashicons#194

Test instructions:

  • Build the plugin, then using the web inspector verify that the above mentioned attributes are present on the SVG icons.

CC: @afercia

@jasmussen
Add aria-hidden and role to SVGs.
dc26f4b 
This PR adds aria-hidden="true" and role="img" to the SVG files inside the dashicons component, addressing feedback here: #1012 (comment)

Upstream PR: WordPress/dashicons#194

CC: @afercia
@jasmussen jasmussen self-assigned this Jun 6, 2017
@jasmussen jasmussen requested a review from aduth June 6, 2017 07:40
youknowriad
youknowriad approved these changes Jun 6, 2017
View reviewed changes
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Did you try removing the ="true" from the aria-hidden="true"? I think React is smart enough to add it.

Anyway, LGTM 👍

@jasmussen
Copy link
Contributor Author

Nice, thank you! I did not try and remove the true, but I did try ariaHidden presuming no dashes in react components. Anyway merging for now, thanks :)

@jasmussen jasmussen merged commit adf7d8d into master Jun 6, 2017
@jasmussen jasmussen deleted the update/dashicons-aria branch June 6, 2017 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youknowriad youknowriad youknowriad approved these changes

@aduth aduth Awaiting requested review from aduth

Assignees

@jasmussen jasmussen

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jasmussen @youknowriad