Skip to content
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: a11y: move skip link anchor entry point to unique page content in default layout #1212

Merged
merged 4 commits into from
Feb 22, 2024

Conversation

shkeating
Copy link
Contributor

SC-3212

Proposed changes

this branch edits the DefaultLayout.tsx file which is out base for our core functionality pagesMy Space, Sites & Applications, Guardian Directory that have the left side nave and repeated PageHeader component to move the id anchor link for the Skip to main content link to direct to.

This ensures the user can skip right to the unique page content instead of getting directed to the search, side nav, etc.

This ensures compliance for WCAG 2.0 2.4.1 Bypass success criteria Trusted Tester ID 9A

Reviewer notes

this PR does not make changes to the other page layouts, which appear to be compliant. I am unsure if we should skip the Page Header component on the PageLayout.tsx layout.

Setup

Start the system

yarn services:up
yarn dev
cd ../ussf-portal-cms
yarn dev

Login to the portal http://localhost:3000
Press Tab to expose the Skip to main content link and ensure it skips the repeated content we have on other pages, and that it goes directly to our unique page content in all system page layouts.


Code review steps

As the original developer, I have

  • Met the acceptance criteria
  • Created new stories in Storybook if applicable
  • Created/modified automated unit tests in Jest
  • Created/modified automated E2E tests
  • Followed guidelines for zero-downtime deploys, if applicable
  • Use ANDI to check for basic a11y issues

As a reviewer, I have

Check out our How to review a pull request document.


Screenshots

skip.link.mp4

@shkeating shkeating self-assigned this Feb 15, 2024
@shkeating shkeating requested a review from a team as a code owner February 15, 2024 22:14
Copy link

@shkeating shkeating requested a review from AnnaGingle February 15, 2024 22:14
@shkeating
Copy link
Contributor Author

oop i messed up a test, gotta update it to not look for the id on the main element

gidjin
gidjin previously approved these changes Feb 15, 2024
Copy link
Contributor

@gidjin gidjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like id isn't getting rendered on the Grid component. Looks like it's not being passed down correctly. From what I can tell it's the same as not having the id at all. Is this working for you? We might need to fix the Grid component to properly pass id down.

@gidjin gidjin dismissed their stale review February 15, 2024 23:47

Looks like it's not working as expected

@gidjin gidjin merged commit c2e8e8e into main Feb 22, 2024
11 checks passed
@gidjin gidjin deleted the sc-3212-content-anchor branch February 22, 2024 00:54
gidjin pushed a commit that referenced this pull request Feb 22, 2024
## [4.31.0](4.30.1...4.31.0) (2024-02-22)


### Features

* Add additional content to third-party API ([#1206](#1206)) ([053cd1d](053cd1d))
* Add Site Header ([#1210](#1210)) ([4f34bc4](4f34bc4))
* Create new portal user from JWT request if one doesn't exist ([#1209](#1209)) ([7e0102c](7e0102c))
* Support additional file types for opening docs in browser ([#1214](#1214)) ([0c67389](0c67389))


### Bug Fixes

* a11y:  Guardian Directory field labeling ([#1205](#1205)) ([db3df68](db3df68))
* a11y: edit display name component syntax  ([#1204](#1204)) ([c6408cf](c6408cf))
* a11y: move skip link anchor entry point to unique page content in default layout ([#1212](#1212)) ([c2e8e8e](c2e8e8e))
* adds page title to Guardian Directory page ([#1211](#1211)) ([8f0b0cb](8f0b0cb))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants