-
Notifications
You must be signed in to change notification settings - Fork 441
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
Fix accessibility issues #2683
Fix accessibility issues #2683
Conversation
- Added aria-label to the new submission buttons (for repositories with more than 1 entity type) - Fixed aria-controls pointing to non-existing ids
- Added aria-labels to trash & select bitstream icon - Added aria-labels ui-switch components (had to upgrade ngx-ui-switch to 14.1.0) - Fixed aria-controls pointing to non-existing ids - Fixed bulk-access-browse not having the tab role on it's tabs - Fixed role="tablist" not having direct role="tab" by adding role="presentation" on the li elements - Fixed aria-expanded being set to true when collapsed and backwards for BulkAccessBrowseComponent & BulkAccessSettingsComponent
- Fixed aria-controls on HealthComponentComponent, HealthInfoComponent, HealthInfoComponentComponent, HealthPanelComponent pointing to non-existing id - Fixed the tabs not having the role tab on its tabs - Fixed aria-expanded being set to true when collapsed and backwards for HealthPanelComponent & HealthInfoComponent - Fixed role="tablist" not having direct role="tab" by adding role="presentation" on the li elements - Fixed minor alignment issues
- Added missing aria-label to buttons
- Added missing aria-label to delete button
- Added missing aria-label to delete buttons - Moved hardcoded translation to translation files - Fix color contrast issues on buttons - Fix minor alignment issues - Added missing aria labels to input and select elements
- Made Process Output keyboard accessible
- Added missing aria-labels to input checkboxes - Fixed minor css alignment
- Added missing aria-labels to input checkboxes - Fixed role="tablist" not having direct role="tab" by adding role="presentation" on the li elements
- Added missing aria-labels to input checkboxes - Fixed multiple tab related accessibility issues
…g for OrgUnits without organization.legalName field
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.
👍 Thank you so much @alexandrevryghem ! I went through all 19 commits one by one and verified that each page's accessibility warnings (from the Deque Axe DevTools plugin) are fixed as described. Also checked the code and everything looks good as well.
I've also double checked the referenced tickets, and the issues you referenced all appear to be fixed. In some situations, that essentially "closes" the old ticket. In others, there's still one or two issues on those pages, but I'll make that clearer in the tickets themselves.
Overall, this is a significant amount of accessibility fixes, so it's much appreciated! I'll make sure it is automatically backported to 7.x (via the port to dspace-7_x
label), so that these fixes can all be included in 7.6.2.
@alexandrevryghem : After finishing my review, I was double checking the referenced tickets. I wanted to note that I **don't believe this PR fixes 470941 in #1191:
I believe the bug in that case is that these buttons disappear completely if you turn off CSS. I can still reproduce that using the WAVE plugin for Chrome (if I turn off styles on that Item Edit -> Bitstreams tab. The action buttons on bitstreams still appear empty & it's difficult to understand them. I think the problem is likely that we aren't adding extra accessibility for these FontAwesome icons like is described here: https://fontawesome.com/docs/web/dig-deeper/accessibility#icons-used-as-semantic-elements-2 In other words, I think we'd need to have an invisible |
…f & replaced authentication dropdown menu link with button - Also fixed box-shadows not working the same way for all footer buttons/links
@tdonohue: I fixed the issue. At first, I addressed it using the |
Thanks @alexandrevryghem ! I've verified today that the latest commits do fix 470941 in #1191. I'll get this merged & ported to 7.x today (and update all the tickets where portions were fixed). Thanks again so much for all the work to clean up accessibility. This PR cleans up a ton of small issues to get us to zero (non-contrast issue) errors (in Axe and WAVE) on many pages. So, this combined with #2587 is a wonderful improvement! |
Successfully created backport PR for |
References
Description
This PR fixes many accessibility bugs, such as missing ARIA tags, absent roles, and incorrect header ordering. I have seperated the fixes by page/type of issue to facilitate the review process.
Instructions for Reviewers
I used the Axe DevTools plugin and the Lighthouse plugin on most of the pages to find and address numerous issues. I did not fix any of the color contrast issues, as these are already being resolved by #2587, except for the ones on the processes, which were not covered in that particular PR.
Checklist
yarn lint
yarn check-circ-deps
)package.json
), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.