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

Remaining accessibility fixes #552

Merged
merged 1 commit into from
Dec 18, 2023
Merged

Remaining accessibility fixes #552

merged 1 commit into from
Dec 18, 2023

Conversation

eric-gade
Copy link
Collaborator

What does this PR do? 🛠️

This PR deals with two remaining accessibility concerns highlighted in #516 :

  • Footer` links are grouped in multiple elements with aria labels?
  • Daily forecast: Repetition temperature reads “day will be high of “54” degrees 54º night will be low of 48ºF”

What does the reviewer need to know? 🤔

You will want to use VoiceOver to test the following:

  • That the links in the footer are labelled only as "footer primary links"
  • That the daily forecast temperature text items are read as a complete sentence, and that the individual number and degrees texts are not read (separately) at all

@eric-gade
Copy link
Collaborator Author

@greg-does-weather One question I'm not sure about: because of our templates, we currently have the site links in the footer wrapped in two nested <nav> elements that appear to be dealing with the same thing. The first (inner one) is here. The second (outer one) is here.

I'm thinking we might want to ditch the inner <nav> and simply have the template it appears in display the unordered list.

@greg-does-weather
Copy link
Collaborator

I love the way the daily forecast reads. 🤩

To your question, I'm happy to follow your lead on HTML layout. If we didn't lift our footer straight from a USWDS template, we might want to do that but honestly, I still get lost sometimes with the right ways to nest semantic elements.

Copy link
Collaborator

@greg-does-weather greg-does-weather left a comment

Choose a reason for hiding this comment

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

Code Review Checklist

A checked item is either an assertion that I have tested this item or an indication that I have verified it does not apply to this pull request.

The Basics

  • Checks are passing
  • I read the code
  • I ran the code
  • (if applicable) Post deploy steps are run
  • (if applicable) I validated the change on the deployed version in

Accessibility

  • New pages have been added to cypress-axe file so that they will be tested with our automated accessibility testing
  • Meets WCAG 2.0 AA or 2.1 AA for Section 508 compliance
    • Site is keyboard accessible. All interactions can be accessed with a keyboard
    • Site is free of keyboard traps. The keyboard focus is never trapped in a loop
    • All form inputs have explicit labels
    • Form instructions are associated with inputs
    • All relevant images use an img tag
    • All images have appropriate alt attributes
    • Multimedia is tagged. All multimedia has appropriate captioning and audio description
    • Text has sufficient color contrast. All text has a contrast ratio of 4.5:1 with the background
    • Site never loses focus. Focus is always visible when moving through the page with the keyboard
    • Tab order is logical
    • Tables are coded properly. Tables have proper headers and column attributes
    • Headings are nested properly. Heading elements are nested in a logical way
    • Language is set. The language for the page is set
    • CSS is not required to use the page. The page makes sense with or without CSS
    • Links are unique and contextual. All links can be understood taken alone, e.g., ‘Read more - about 508’
    • Page titles are descriptive

Device Matrix

  • firefox/gecko (renders correctly and user interactions work)
  • chrome/chromium/edge (renders correctly and user interactions work)
  • safari/webkit (renders correctly and user interactions work)
  • web page is readable and usable
    • at 480px (mobile)
    • at 640px (tablet)
    • at 1024px (desktop)

-- What
Hiding the actual ℉ and temp text in the daily forecast from
ARIA. This ensures that only the `usa-sr-only` text that describes the
container is read.

We are also removing a redundant ARIA label in the footer.
@eric-gade eric-gade added this pull request to the merge queue Dec 18, 2023
Merged via the queue into main with commit 89c15a8 Dec 18, 2023
8 checks passed
@eric-gade eric-gade deleted the eg-516-misc branch December 18, 2023 22:39
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants