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

fix: set completion popup role to 'menu' for safari #5403

Merged
merged 4 commits into from
Nov 28, 2023

Conversation

akoreman
Copy link
Contributor

@akoreman akoreman commented Nov 28, 2023

Issue #, if available: #5377

Description of changes: On Safari, aria-activedescendant is not supported for the listbox role but is supported for the menu role. This results in an unusable autocomplete for VoiceOver/Safari users. This changes the role to menu for Safari to give some (but not perfect) support for this browser/screenreader combo:

Screen.Recording.2023-11-28.at.11.17.37.mov

Sets aria-roledescription to keep the behaviour somewhat consistent between the two flows. Sets aria-selected for the selected item, without it Safari/VoiceOver doesn't read the item.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

codecov bot commented Nov 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1c207c9) 87.54% compared to head (fbcb850) 87.54%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5403   +/-   ##
=======================================
  Coverage   87.54%   87.54%           
=======================================
  Files         583      583           
  Lines       46216    46222    +6     
  Branches     6998     7001    +3     
=======================================
+ Hits        40459    40465    +6     
  Misses       5757     5757           
Flag Coverage Δ
unittests 87.54% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@akoreman akoreman marked this pull request as ready for review November 28, 2023 10:27
@akoreman akoreman linked an issue Nov 28, 2023 that may be closed by this pull request
@InspiredGuy
Copy link
Contributor

The experience in other browsers is not affected by setting the aria-selected property, right?

@akoreman
Copy link
Contributor Author

akoreman commented Nov 28, 2023

@InspiredGuy not as far as I can tell, setting aria-selected to true makes the screenreader read selected on the selected item but Chrome and FF were already doing that.

@akoreman akoreman merged commit ee917cf into ajaxorg:master Nov 28, 2023
3 checks passed
@akoreman akoreman deleted the completion_a11y branch November 28, 2023 10:47
akoreman added a commit that referenced this pull request Dec 11, 2023
follow-up to #5403, we set the role to menuitem for items in the completer popup on webkit browsers but aria-selected is not allowed for menuitem roles triggering customers who run automated a11y tests. This changes it to aria-current which is allowed for this role.
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.

Autocomplete: Screen reader doesn't read suggestions on Safari
3 participants