-
Notifications
You must be signed in to change notification settings - Fork 202
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
Use contentlist role for search results for a11y #1979
Conversation
Size Change: -4.97 kB (-1%) Total Size: 857 kB
ℹ️ View Unchanged
|
Full-stack documentation: https://docs.openverse.org/_preview/1979 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
734dea8
to
b9c66a4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for the thorough testing instructions!! I haven't done screenreader testing, so this was quite informative. I ended up trying to get orca
working on Linux, and while I was getting screenreader results, I wasn't hearing the text you mentioned about the number of search results. I was also seeing the following in my console running the frontend:
WARN [vue-i18n] Value of key 'browse-page.aria.results' is not a string or function ! 13:00:14
WARN [vue-i18n] Cannot translate the value of keypath 'browse-page.aria.results'. Use the value of keypath as default.
Could that be related? Is there an extra step I'm missing in terms of setup?
Thank you so much for testing, @AetherUnbound ! I'm sure the set up was quite time and effort consuming. Having a Linux machine for testing, too, is great for breadth of different setups. Yes, the two points you mention are related. We save all of the texts as |
Got it! That made those errors go away. Now what I'm hearing when I click on the "Cat" header in the search results is:
I'm still not hearing the specific result count 😮 is there a better way for me to navigate to the search results? If I use TAB I have to hit it a lot to iterate through all the filters before getting to the search results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code changes generally make sense to me! Took me a sec to grok v-for
😄
I suspect the not-counting-results may be a difference between screen reader implementations, since your changes only include reading out the "search results for" label. So I think this is working as expected!
@AetherUnbound we have some docs on general screen reader testing here that might prove useful! https://docs.openverse.org/frontend/reference/testing_guidelines.html#accessibility |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@obulat I am also not hearing result counts, just being told that I am in a group and then read the aria label "Search results for {query}."
I think this is how VoiceOver implements it. When you navigate to the results using Tab, you don't hear the number of items, but if you navigate to the page, you will hear: I also tried other lists on other sites, and they do not announce the number of items in the contentlist. I also tested this change with NVDA on windows, and it says "Search results for "Cat". List with 32 items. Cat cafe in Seol link" when I navigate to the first result using keyboard. The audio results don't say anything about the contentlist in NVDA because they have to have |
b9c66a4
to
217f349
Compare
217f349
to
655f7f1
Compare
Based on the medium urgency of this PR, the following reviewers are being gently reminded to review this PR: @zackkrida Excluding weekend1 days, this PR was ready for review 8 day(s) ago. PRs labelled with medium urgency are expected to be reviewed within 4 weekday(s)2. @obulat, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings. Footnotes
|
Fixes
Fixes #794 by @zackkrida
Fixes #541 by @obulat
Description
This PR wraps the search results into an ordered list to make the screen reader announce it as a contentlist, and to make it possible for the users to know how many items there are.
For the list
aria-label
, I used "Search results for query", without the media type. I think this might be clear from previous navigation, and it's easier to implement this way. I am open to suggestions about the text.There are also a couple of other fixes:
Testing Instructions
Run the app using
just frontend/run dev
Open a search page
localhost:8443/search/image?q=cat
and turn on a screenreader (CMD + F5 for VoiceOver on Mac, for Windows I used the UI to open NVDA)On Mac, with Voiceover:
When you navigate to the search results, you should hear "Content list Search results for cat, 20 items" when screenreader reads out the page, and "Entering Search results for cat content list" when you navigate to the results.
On Windows, with NVDA:
You should hear "Search results for cat List with 27 items Cat link" when you navigate to the first result on All content list.
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin