Accessibility issue: items incorrectly announced as "clickable"

scoutb-cogapp commented 3 months ago

UV version: universalviewer@4.0.25

I'm submitting a: bug report => please fork one of these codesandbox examples with a repro of your issue and include a link to it below

Page area

image viewer when using NVDA screenreader

Issue description

When navigating by keyboard using nvda screenreader, several items get announced as clickable when they are not, which can be very disorienting for non-sighted users:

an item right after the settings button. It is not visually highlighted which item it is. When i try to activate it, nothing happens.
when the user opens the more information sidebar, the first thing they tab to after opening is the div for the whole sidebar. It is announced as "about the item clickable". The label is descriptive and focus is visible but there is nothing "clickable" about the item itself.
every div inside the info-sidebar is preceded by the term "clickable" before the content is read out. These are just paragraphs and not interactive.

Furthermore: the image control buttons (eg zoom or next buttons) are all announced as clickable (eg "clickable next"). As they are buttons, this is unneccessary - users will understand what "zoom" or "next" mean.

This was tested using NVDA. Other screenreaders may act differently. But it highlights an issue with the accessibility of the code.

Steps to reproduce

activate NVDA screenreader (requires Windows)
open this manifest
use the keyboard to navigate to the items mentioned and obsere how they are announced by the screenreader

Expected behaviour

Only interactive items should be announced as clickable.

Possible fix

These are all divs. I think the problem stems from the fact that NVDA struggles to interpret the divs and is looking for all sorts of (inherited) CSS and event listeners to make sense of them and things get confused. If appropriate semantic HTML was used (ie button elements), I expect the issues to go away.

WCAG criterion

4.1.2 Name, Role, Value (Level A)

hannahb-cogapp commented 3 months ago

The item right after the settings button is: <div class="openseadragon-canvas" tabindex="0" dir="ltr" style="…"></div>. The issue is caused by this having tabindex="0"

scoutb-cogapp commented 1 month ago

The suggested fix from @hannahb-cogapp will work for all the items that are not meant to be interactive. But we cannot remove the tabindex from items like buttons because they then become inaccessible to keyboard users.

I experimented with the zoom-in button and was able to fix this:

i turned the zoom-in div into a button element and then removed the tabindex from that element, the "clickable" announcement went away and the item was correctly announced as "zoom in button". And because buttons are inherently clickable, they are still accessible for keyboard navigation.

jamesmisson commented 1 month ago

Hi @scoutb-cogapp , I'm unable to reproduce the 'clickable' more information panel bugs. When I tab to the open panel, pressing tab again jumps to the next 'more' link, or embedded link, it doesn't announce each item as clickable as described in the issue. It's possible I'm using NVDA wrong, would you be able to show me what you're hearing?

demiankatz commented 1 month ago

@jamesmisson, I'm not using a screen reader, but I think I see where at least one of the problem areas lies... If you open the viewer and start tabbing, you'll notice that the focus moves across the top bar... but once the focus is on the settings gear icon at the top right, if you hit tab one more time, focus disappears. You have to hit tab again to focus the "Zoom in" button in the OSD panel, which I think should be the next item in the tab order after the options gear. I think what is happening in between these two controls is that focus is being applied to the OSD top-level div because it has a tabindex, but since that's not really a focusable element, the focus has no desirable effect. It just causes the focus to get lost, and possibly announces nonsense in some screen readers.

jamesmisson commented 1 month ago

Thanks @demiankatz , that one I can reproduce, it's the clickable metadata items that aren't happening for me (the 3rd bullet in the issue description), are they related?

demiankatz commented 1 month ago

@jamesmisson, when I inspect the metadata panel, I see there are click events attached to all of the divs for some reason, but I'm not sure where they are coming from or what they are for:

Probably worth digging into the metadata panel component to figure out what's going on here!

jamesmisson commented 1 month ago

I've split the 'clickable' zoom/rotate buttons into a separate issue as it requires some refactoring of the OSD center panel: https://github.com/UniversalViewer/universalviewer/issues/1170

jamesmisson commented 1 month ago

I've submitted a fix in iiif-metadata-component and split out an issue for the OSD control buttons.

The only thing that remains is the focusable element between the settings menu and the zoom button. As Hannah noted, this is the OSD canvas, which has a tabIndex. It's possible to remove the tabIndex, but this would mean that the arrow key controls that move the image around won't be accessible. The 'clickable' announcement here isn't misleading - the canvas can be clicked, to move the image around. So I think the solution may be to make it clear what is actually being focused here by adding an aria label to the element. Would 'clickable canvas' work?

demiankatz commented 1 month ago

As always, I defer to others with more accessibility expertise, but it seems reasonable to me to add a label to the canvas. I assume the label wouldn't need to include the word "clickable" since that comes from the context, so we could just label it "canvas" (or, if "image" would be an appropriate label, we could use that since the term is already localized and we wouldn't have to come up with new translations).

UniversalViewer / universalviewer