Fix remaining ARIA issues from #5488 (complementary to #5514)#5611
Draft
bram-atmire wants to merge 1 commit intoDSpace:mainfrom
Draft
Fix remaining ARIA issues from #5488 (complementary to #5514)#5611bram-atmire wants to merge 1 commit intoDSpace:mainfrom
bram-atmire wants to merge 1 commit intoDSpace:mainfrom
Conversation
PR DSpace#5514 (also referencing DSpace#5488) makes the dso-edit-menu menubar role vanish when the menu is empty/hidden. This commit addresses the four remaining ARIA problems flagged in the issue and the supporting WAVE/Pa11y/HTMLCS findings: 1. auth-nav-menu (login dropdown trigger): The button declared aria-haspopup="menu" but actually opened a <div role="dialog" aria-modal="true">. Screen readers therefore announced "has popup, menu" and then revealed a dialog. Updated to aria-haspopup="dialog" so the announcement matches what opens. Also removed the redundant role="button" / tabindex="0" on a real <button> and added an explicit type="button". 2. expandable-navbar-section (top-level navbar section toggler): Replaced <a href="javascript:void(0);" role="menuitem"> with a real <button type="button" role="menuitem">. The navbar wrapper has role="menubar", so the menuitem nesting is valid; the issue was the href="javascript:void(0)" antipattern. Added a small SCSS reset so the button keeps the link-like appearance the previous anchor had. 3. context-help-toggle (header help button): This control is not nested in any role="menu" / role="menubar", so its role="menuitem" was an orphan flagged by axe (aria-required-parent). Replaced <a href="javascript:void(0);" role="menuitem"> with a plain <button type="button"> and dropped the orphan role. 4. dso-edit-menu-section (each section button under role="menubar"): When the parent menubar role is rendered (the case once DSpace#5514's visibility logic enables it), axe's aria-required-children rule still fires because the descendant <a>/<button> elements lack role="menuitem". Added role="menuitem" to each rendered control so the menubar pattern is structurally valid. Spec updates accompany every template change. References DSpace#5488
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
References
role="menubar"case indso-edit-menu. This PR is complementary; both can merge to fully close ARIA menu error on community and item pages #5488.Description
Fix the four remaining ARIA bugs from the WAVE / Pa11y / axe findings on #5488 that #5514 does not cover.
Instructions for Reviewers
List of changes in this PR:
src/app/shared/auth-nav-menu/auth-nav-menu.component.html— login dropdown trigger.aria-haspopup="menu"on the button, but the popup it controls is a<div role="dialog" aria-modal="true">. Screen readers announced "has popup, menu" then revealed a dialog.aria-haspopup="dialog". Also dropped the redundantrole="button"/tabindex="0"on a real<button>and added an explicittype="button".src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html— top-level navbar section toggler.<a href="javascript:void(0);" role="menuitem">.<button type="button" role="menuitem">. The navbar wrapper hasrole="menubar"(navbar.component.html), so themenuitemnesting is valid; the only issue was thehref="javascript:void(0)"antipattern. A small SCSS reset (expandable-navbar-section.component.scss) keeps the button looking like the previous anchor (no native button chrome).src/app/header/context-help-toggle/context-help-toggle.component.html— header help button.<a href="javascript:void(0);" role="menuitem">. This control is not nested in anyrole="menu"/role="menubar", so itsrole="menuitem"was an orphan, flagged by axe (aria-required-parent).<button type="button">with noroleattribute. Added a small SCSS reset for the same reason as above.src/app/shared/dso-page/dso-edit-menu/dso-edit-menu-section/dso-edit-menu-section.component.html— each rendered section button underrole="menubar".<a class="btn">/<button class="btn">had norole. axe'saria-required-childrenfires on the parent menubar.role="menuitem".Specs were updated for each component:
auth-nav-menu.component.spec.tsasserts the login toggle hasaria-haspopup="dialog".expandable-navbar-section.component.spec.tsasserts the toggler is a real<button type="button" role="menuitem">.context-help-toggle.component.spec.tsasserts the toggle is a<button type="button">with no orphanrole.dso-edit-menu-section.component.spec.tsasserts the rendered control declaresrole="menuitem".How to test:
yarn startorpm2 start config/dspace-ui-test.json).<button type="button">); pressingEnter/Space/ArrowDownstill toggles / activates the dropdown./info/accessibility, focus the context-help icon in the header. Inspect: it should be a<button>, not an anchor.<div role="menubar">). Inspect each section's icon button - it should declarerole="menuitem".npm run test -- --include='src/app/shared/auth-nav-menu/auth-nav-menu.component.spec.ts' --include='src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts' --include='src/app/header/context-help-toggle/context-help-toggle.component.spec.ts' --include='src/app/shared/dso-page/dso-edit-menu/dso-edit-menu-section/dso-edit-menu-section.component.spec.ts'shows 37 specs pass./info/accessibility. Thearia-haspopup="menu"on login button,aria-required-parenton the help toggle, and thearia-required-childrenon the visible dso-edit-menu should all clear.Checklist
mainbranch of code (unless it is a backport or is fixing an issue specific to an older branch).npm run lintnpm run check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation. (n/a)