-
Notifications
You must be signed in to change notification settings - Fork 2
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
Improve accessibility for collections pages #3161
Conversation
efa5078
to
47b1ee0
Compare
47b1ee0
to
1aaf163
Compare
@@ -1,9 +1,10 @@ | |||
<table class="table table-sm mb-5 caption-header"> | |||
<caption>Details | |||
<%= helpers.turbo_frame_tag dom_id(collection_version, :edit), | |||
<%= helpers.turbo_frame_tag dom_id(collection_version, :edit_details), |
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.
Non-unique IDs bad.
@@ -11,7 +11,7 @@ | |||
<%= render CitationModalComponent.new %> | |||
</div> | |||
</div> | |||
<span class="header-text"><%= name %></span><%= render Collections::DraftComponent.new(collection_version: collection_version) %> | |||
<h1 class="header-text d-inline-block"><%= name %></h1><%= render Collections::DraftComponent.new(collection_version: collection_version) %> |
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.
Missing headings bad.
@@ -1,5 +1,5 @@ | |||
<section class="collection-nav"> | |||
<nav class="nav"> | |||
<nav class="nav" role="tablist"> |
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.
Make it clear what the aria role is.
// This removes the pagination list and if it is empty, and adds an aria | ||
// label for its parent nav, which is done to improve accessibility. | ||
dt.on('datatable.init', () => { | ||
const paginationList = document.querySelector("ul.dataTable-pagination-list") | ||
if (paginationList === null) return | ||
|
||
paginationList.parentNode.setAttribute("aria-label", "Pagination Controls for Deposits") | ||
|
||
if (!paginationList.hasChildNodes()) paginationList.remove() | ||
}) |
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 changes to this file add accessibility improvements to simple-datatables.
NOTE: This is currently based on the commits in #3159 for easier testing, so the diff looks larger than it really is.Why was this change made? 🤔
Fixes #3151
Fixes #3152
This deals with a11y violations of the A/AA/aria variety as identified by the Siteimprove Accessibility Checker extension and the WAVE extension.
How was this change tested? 🤨
Does your change introduce accessibility violations? 🩺
No