samvera / hyrax

Hyrax is a Ruby on Rails Engine built by the Samvera community. Hyrax provides a foundation for creating many different digital repository applications.
http://hyrax.samvera.org/
Apache License 2.0
183 stars 123 forks source link

Search results slideshow modal - fix aria attribute for improved accessibility #3084

Closed adamjarling closed 7 months ago

adamjarling commented 6 years ago

Descriptive summary

From the Accessibility360 Audit Report:

Slideshow modal on search results page: https://demo.curationexperts.com/catalog?f%25255Bhuman_readable_type_sim%25255D%25255B%25255D=Collection&f%255Bcontributor_sim%255D%255B%255D=Charlotte+Nunes&locale=en&view=slideshow

The modal container is not labeled as such.

Give the <div> containing the modal an aria-label="modal dialog" and remove aria-hidden="true".

slideshow-modal-aria-hidden

Rationale

Improved accessibility. The modal container is not labeled as such.

Expected behavior

aria-label="modal dialog" is added and aria-hidden="true" is removed from the <div id="slideshow-modal"> element.

Actual behavior

See screenshot html.

Steps to reproduce the behavior

  1. Enter a search term in Hyrax homepage.
  2. Click on the 'Slideshow' display icon to get a slideshow view.
adamjarling commented 5 years ago

I can't seem to find where this modal element is being brought into the app. Is it from Blacklight? I've searched both the Hyrax app and Blacklight 6.15.0 app for "slideshow-modal", "slideshow-presenter", and other attribute/id/class text which gets added to the DOM, but coming up empty in my hunt.

If anyone has any ideas...

kdid commented 5 years ago

@adamjarling - The element is here: https://github.com/projectblacklight/blacklight-gallery/blob/master/app/views/catalog/_slideshow_modal.html.erb#L2

Reading the documentation for this - https://www.w3.org/TR/wai-aria-practices/#dialog_modal

I'm wondering, do we need to do more that just add aria-label and remove aria-hidden? Like, do we need to also remove aria-labelledby? And also add aria-modal. I'm a little confused by the part where it says it might cause problems to remove the aria-hidden... Can you shed any light?

adamjarling commented 5 years ago

@kdid I'm not really too sure. Most of these tickets were just transferring recommendations from the Audit360 report. Maybe Google best practices for modal aria attributes as a starting point? That's what I've kind of been doing for every ticket, and eventually seems to lead back to the WC3 specs. I'll take a look after stand-ups though and see if we're on the same page.

adamjarling commented 5 years ago

@kdid For this, if it's using Bootstrap modal (I assume it is), then maybe we just follow Bootstrap's setup for aria labeling: https://getbootstrap.com/docs/3.3/javascript/#live-demo Sometimes Bootstrap will handle automated aria labeling depending on the element state.

If there's no label or title for the modal, then could probably lose the aria-labelledby. Doesn't look like Bootstrap uses aria-modal.

Bootstrap docs also reference this if it helps:

Make modals accessible Be sure to add role="dialog" and aria-labelledby="...", referencing the modal title, to .modal, and role="document" to the .modal-dialog itself.

Additionally, you may give a description of your modal dialog with aria-describedby on .modal.

DiemBTran commented 3 years ago

The original issue still exists and requires community discussion. The following image demonstrates that the exact same attributes (excluding some styling) exist for the original post and present-day code. The <div> can be identified with id="slideshow-modal" nurax+3084

The original post refers to an Accessibility360 Audit Report that suggests to Give the <div> containing the modal an aria-label="modal dialog" and remove aria-hidden="true". It is suggested the aria-label="modal dialog" is added and aria-hidden="true" is removed from the <div id="slideshow-modal"> element.

However, other comments suggest that there may be other styles of implementation that must be followed with Bootstrap.

scherztc commented 7 months ago

When examining this modal on dev.nurax.samvera.org on January 2024, I no longer see the aria-hidden="true", the role= and I do see aria-labelledby=. I assumed this got fixed with the upgrade of Bootstrap.

Screenshot 2024-01-25 at 2 13 19 PM