Skip to content

Fix remaining ARIA issues from #5488 (complementary to #5514)#5611

Draft
bram-atmire wants to merge 1 commit intoDSpace:mainfrom
bram-atmire:fix/5488-aria-followups
Draft

Fix remaining ARIA issues from #5488 (complementary to #5514)#5611
bram-atmire wants to merge 1 commit intoDSpace:mainfrom
bram-atmire:fix/5488-aria-followups

Conversation

@bram-atmire
Copy link
Copy Markdown
Member

References

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:

  1. src/app/shared/auth-nav-menu/auth-nav-menu.component.html — login dropdown trigger.

    • Was: 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.
    • Now: aria-haspopup="dialog". Also dropped the redundant role="button" / tabindex="0" on a real <button> and added an explicit type="button".
  2. src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html — top-level navbar section toggler.

    • Was: <a href="javascript:void(0);" role="menuitem">.
    • Now: <button type="button" role="menuitem">. The navbar wrapper has role="menubar" (navbar.component.html), so the menuitem nesting is valid; the only issue was the href="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).
  3. src/app/header/context-help-toggle/context-help-toggle.component.html — header help button.

    • Was: <a href="javascript:void(0);" role="menuitem">. This control is not nested in any role="menu" / role="menubar" , so its role="menuitem" was an orphan, flagged by axe (aria-required-parent).
    • Now: a plain <button type="button"> with no role attribute. Added a small SCSS reset for the same reason as above.
  4. src/app/shared/dso-page/dso-edit-menu/dso-edit-menu-section/dso-edit-menu-section.component.html — each rendered section button under role="menubar".

    • Was: the descendant <a class="btn"> / <button class="btn"> had no role. axe's aria-required-children fires on the parent menubar.
    • Now: each rendered control declares role="menuitem".

Specs were updated for each component:

  • auth-nav-menu.component.spec.ts asserts the login toggle has aria-haspopup="dialog".
  • expandable-navbar-section.component.spec.ts asserts the toggler is a real <button type="button" role="menuitem">.
  • context-help-toggle.component.spec.ts asserts the toggle is a <button type="button"> with no orphan role.
  • dso-edit-menu-section.component.spec.ts asserts the rendered control declares role="menuitem".

How to test:

  1. Start the UI (yarn start or pm2 start config/dspace-ui-test.json).
  2. With a screen reader (VoiceOver, NVDA), focus the navbar Login trigger - announcement should say "has popup, dialog" rather than "menu".
  3. Tab to a top-level expandable navbar section ("Browse" by default). The element should be a button (right-click → Inspect to confirm <button type="button">); pressing Enter / Space / ArrowDown still toggles / activates the dropdown.
  4. Visit /info/accessibility, focus the context-help icon in the header. Inspect: it should be a <button>, not an anchor.
  5. As a logged-in admin, visit any item or community page that renders the dso-edit-menu (<div role="menubar">). Inspect each section's icon button - it should declare role="menuitem".
  6. 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.
  7. Re-run axe / Pa11y on a community page, an item page, the login form, and /info/accessibility. The aria-haspopup="menu" on login button, aria-required-parent on the help toggle, and the aria-required-children on the visible dso-edit-menu should all clear.

Checklist

  • My PR is created against the main branch of code (unless it is a backport or is fixing an issue specific to an older branch).
  • My PR is small in size (10 files, 56 insertions, 13 deletions).
  • My PR passes ESLint validation using npm run lint
  • My PR doesn't introduce circular dependencies (verified via npm run check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. (n/a - no method changes)
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • My PR aligns with Accessibility guidelines if it makes changes to the user interface.
  • My PR uses i18n (internationalization) keys instead of hardcoded English text, to allow for translations. (existing keys retained)
  • My PR includes details on how to test it.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation. (n/a)
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself. (n/a)
  • If my PR fixes an issue ticket, I've linked them together.

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
@lgeggleston lgeggleston added bug accessibility 1 APPROVAL pull request only requires a single approval to merge testathon Reported by a tester during Community Testathon labels May 4, 2026
@lgeggleston lgeggleston moved this to 🙋 Needs Reviewers Assigned in DSpace 10.0 Release May 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 APPROVAL pull request only requires a single approval to merge accessibility bug testathon Reported by a tester during Community Testathon

Projects

Status: 🙋 Needs Reviewers Assigned

Development

Successfully merging this pull request may close these issues.

ARIA menu error on community and item pages

2 participants