UniversalViewer / universalviewer

A community-developed open source project on a mission to help you share your 📚📜📰📽️📻🗿 with the 🌎
http://universalviewer.io
Other
496 stars 191 forks source link

Accessibility issue: inactive buttons in tab order #1086

Closed scoutb-cogapp closed 2 weeks ago

scoutb-cogapp commented 2 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

back and forward buttons in header panel as well as image controls

Issue description

The back buttons for the first image and the forward buttons for the last image are inactive.

The problem is that the inactivity is insufficiently conveyed: The items remain in the tabbing order so are still accessed when a user navigates by keyboard. This indicates they are acvite. The colour change for the buttons in the header panel is also relatively subtle and will not be visible to all users.

Steps to reproduce

  1. open this manifest
  2. navigate to any of the backward buttons and observe that they are accessible even though this is the first image and there is no previous image to move to
  3. activate a screenreader and navigate to the same buttons, observe that they are announced no different from when they are active

Expected behaviour

I would expect tab order to skip inactive links/buttons/elements. Sometimes it can be useful to screenreader users to have them there so they know about certain functionality. In this case, I see no reason to do so since a user will know that where there is a next button, there will be a previous button once there is a previous item.

Possible fix

Remove the buttons from the tab order when they become inactive.

Alternatively, if it is desired that they remain in the tab order, ensure that their inactive state is in some form known to and announced by assistive technologies.

WCAG criterion

4.1.2 Name, Role, Value (Level A)

Related code example

<button class="btn imageBtn first disabled" title="First Image"><i class="uv-icon-first" aria-hidden="true"></i><span class="sr-only">First Image</span></button>
hannahb-cogapp commented 2 months ago

At the moment the buttons receive styling to indicate that they're disabled with the "disabled" class, this doesn't indicate to screen readers that the button is disabled. In addition to the disabled class the buttons should receive the HTML disabled attribute.

demiankatz commented 2 months ago

@Saira-A very recently (yesterday, in fact) fixed some of these in #1093. @Saira-A, did you try systematically searching the code for other instances of the 'disabled' class? I haven't tried that, but it might be easy to extend your fix further if there are more references that we missed. Or maybe we need to create utility methods for disabling and enabling things so we can more easily change this from a central place if we need a different strategy in the future (and to reduce code lines).

Saira-A commented 2 months ago

@Saira-A very recently (yesterday, in fact) fixed some of these in #1093. @Saira-A, did you try systematically searching the code for other instances of the 'disabled' class? I haven't tried that, but it might be easy to extend your fix further if there are more references that we missed. Or maybe we need to create utility methods for disabling and enabling things so we can more easily change this from a central place if we need a different strategy in the future (and to reduce code lines).

Yes, I think the search bar in the footer panel works the same way

Geoffsc commented 2 weeks ago

@demiankatz and/or @jamesmisson This is looking ok to me but potentially you could try other manifests and see if the issue still exists in a different module?

Saira-A commented 2 weeks ago

Looks fine for me with OSD files, but for PDF manifests it still lets you select the back button by backwards tabbing if you're on the first page, though from there you can't do anything

Screenshot 2024-10-21 at 16 00 15
Geoffsc commented 2 weeks ago

Thanks @Saira-A I will look into this more on my end :)

demiankatz commented 2 weeks ago

Thanks, @Saira-A and @Geoffsc. Let me know if you need any help tracking down the offending part of the code. Hopefully if we fix PDF extension, that covers it -- I can't think of another extension that has equivalent pagination controls.