Improve screen reader representation of OSD buttons (fixes #1170)#1174
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
demiankatz
left a comment
There was a problem hiding this comment.
I agree that this isn't especially pretty, but I can't think of a better way, and it works very nicely. Thanks, @LlGC-jop!
|
@demiankatz The mobile footer has a separate set of buttons outside of the main panel, which I assume use IIIF events to trigger the actions in OSD. At some point when we want to do an overhaul of things re styling and responsiveness we can look at making this a single element shared between mobile + desktop so the OSD controls aren't used at all. |
Yes, that's a good point. We also have the "image adjust" button which is a locally-created button that's appended on to the OSD-created buttons. Just building everything consistently is probably less confusing in the long run! |
demiankatz
left a comment
There was a problem hiding this comment.
I noticed there were some conflicts here, so I went ahead and resolved them. While testing the results, I noticed that the controls do not appear as expected when they are activated by tabbing with the keyboard. I wonder if we need to apply the solution-in-progress from #1168 to the newly generated buttons here, once it is finalized; I suspect that rebuilding the buttons is causing some internal OSD listeners to get lost.
|
I think I see what you mean @demiankatz, the controls don't get focus when tabbing forwards, but when you go backwards they do. Re the listeners, they should be the same ones that the mobile versions of the buttons have, so there shouldn't be anything missing, Either way, I'll fix the tabbing issue and let you know when it can be re-tested. Update: I see the real issue, must be a listener on the controls display container or something. |
…to the next/prev buttons.
|
I've added listeners for focus and blur to the controls to make sure they show up when appropriate. Again, not the most elegant solution, but it works well enough without needing to dig around in OSD. I also found that the prev/next buttons didn't have a blur handler so I've added that as well. All seems to work ok now, both with and without the reduced animation setting. |
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @LlGC-jop, this all looks good to me now. Your work here also solves #1076, which still had some ongoing work in progress in #1158. Since #1158 included multiple fixes, I reverted the parts of it that would conflict with this PR and merged the rest, then merged the dev branch back into this PR. After all that, everything still seems to be working as expected/desired.
I'd appreciate it if somebody else could test for good measure, but once we get thumbs-up from one more person, I'm comfortable merging this one!
LanieOkorodudu
left a comment
There was a problem hiding this comment.
@LlGC-jop @demiankatz, I’ve tested the fixes, and I can confirm that tabbing, zoom, and the other buttons mentioned in issue #1076 are all functioning as expected. I also tested with the NVDA screen reader, and everything appears accessible and smooth. Great work on this update. Thanks @LlGC-jop
It's not ideal, but this will copy the old 'buttons' into proper button elements and give them the right click handler, which should work for space + enter as well due to the element type.
Fixes #1076 and #1170.