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

Add Various Accessibility Properties #290

Merged
merged 1 commit into from
May 11, 2023

Conversation

hmh84
Copy link
Contributor

@hmh84 hmh84 commented Mar 3, 2023

My company was testing our mobile app for accessibility with VoiceOver, three issues we noticed that are seemingly unsolvable in the package's current state/available properties were

  1. When readOnlyHeadings={true} the headings do say "dimmed" which is fine however they should really be identified as a "header" as that's what it's trying to achieve instead of being read aloud like they are still selectable items.
  2. Selectable items aren't read as "selected" by the screen reader and the user can't know what selections they have made. By adding accessibilityState={{ selected: itemSelected }} the screen reader now says "Selected, [Item name]" when an item is selected.
  3. The select component that's rendered is trying to act as a combobox, and therefore the role should be combobox.

Please consider my changes, thank you for taking the time to review this pull request.

@github-actions
Copy link

github-actions bot commented Apr 3, 2023

This PR is stale because it has been open 45 days with no activity.

@github-actions github-actions bot added the Stale label Apr 3, 2023
@renrizzolo
Copy link
Owner

Thanks for the PR!
I added the menuitem role to the sub item and the expanded state to the dropdown toggle.
@hmh84 I'm not able to test this so if you can confirm all OK, I'll merge

@github-actions github-actions bot removed the Stale label Apr 25, 2023
@hmh84
Copy link
Contributor Author

hmh84 commented Apr 27, 2023

Thanks for the PR! I added the menuitem role to the sub item and the expanded state to the dropdown toggle. @hmh84 I'm not able to test this so if you can confirm all OK, I'll merge

This works! Thanks

@renrizzolo renrizzolo merged commit b1e5491 into renrizzolo:master May 11, 2023
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.

2 participants