Render skip-to-main-content as <a href="#main-content"> instead of a button#5610
Draft
bram-atmire wants to merge 1 commit intoDSpace:mainfrom
Draft
Render skip-to-main-content as <a href="#main-content"> instead of a button#5610bram-atmire wants to merge 1 commit intoDSpace:mainfrom
bram-atmire wants to merge 1 commit intoDSpace:mainfrom
Conversation
…button The button worked in the running SPA via a JS click handler that focused #main-content, but the canonical and most thoroughly assistive-tech-tested skip-link pattern is a plain anchor. Anchors work without JavaScript, survive hydration glitches, and need no custom focus handler. Changes: - root.component.html: replace <button (click)="skipToMainContent()"> with <a href="#main-content">. Add tabindex="-1" to <main id="main-content"> so the fragment navigation reliably moves focus across browsers. - root.component.ts: drop the now-unused skipToMainContent() method. - root.component.spec.ts: add specs verifying the anchor is rendered with the correct href and that the <main> target carries tabindex="-1". References DSpace#5605
lgeggleston
added
improvement
accessibility
1 APPROVAL
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
2 participants
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
Description
Render the skip-to-main-content control as
<a href="#main-content">instead of a<button>. Aligns with the canonical WAI-ARIA Authoring Practices skip-link pattern and removes the JS click-handler dependency.Instructions for Reviewers
List of changes in this PR:
src/app/root/root.component.html- the skip control becomes<a href="#main-content" class="sr-only" id="skip-to-main-content">. Addedtabindex="-1"to<main id="main-content">so the fragment navigation reliably moves focus across browsers (withouttabindex, some browsers do not move focus to non-form elements after fragment navigation).src/app/root/root.component.ts- removed the now-unusedskipToMainContent()method that previously focused#main-contentprogrammatically.src/app/root/root.component.spec.ts- added specs asserting (1) the skip link is an<a>with the correcthref, and (2) the<main>target carriestabindex="-1".How to test:
yarn startorpm2 start config/dspace-ui-test.json).Tab. The "Skip to main content" link should slide into view (already styled byroot.component.scss).Enter(no JS click handler is involved). The URL should gain#main-contentand keyboard focus should land on the<main>element. A subsequentTabshould move focus to the first interactive element inside the page content rather than back to a navbar item.npm run test -- --include='src/app/root/root.component.spec.ts'shows 3 specs pass.This is not fixing a current WCAG failure (the existing button works for any user who reaches the running SPA, since DSpace requires JS to render). It aligns the implementation with the canonical pattern - see #5605 for full rationale.
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)root.skip-to-contentretained)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation. (n/a)