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: heading elements usage and ordering #1220

Merged
merged 35 commits into from
Mar 7, 2024

Conversation

shkeating
Copy link
Contributor

@shkeating shkeating commented Feb 28, 2024

SC-2310

Proposed changes

In accordance with the Trusted Tester 10: Content Test IDs

  • changes page templates and components that hold the page title to use a h1 for the heading element containing the name of the page
  • removes h2 designation from the Welcome, $name Personal Data component, as well as the preview for it to not use a header element. This h2 ussage was not correct and broke the header order
  • adjusts the headings on components to use the next sequential header (widget headers are now h2s, and the heading below them are h3, h4, etc.
  • ensures header styling is correct and in alignment with how they were designed. Widget headers are bold, and some size corrections were made in the weather widget

TO DO: fix all the unit tests this affects :')

Reviewer notes

there are design system implications to this change. both in figma and storybook we need to make adjustments to our design tokens to reflect different font sizes being assigned to the h2/h3 elements than initally anticipates

this PR changes the semantic output of these elements, and if visual changes were made they were done in accordance with Figma designs, details I think may have been missed when they are first styles (for example Widget headers being bold)

e2e tests: https://github.com/USSF-ORBIT/ussf-portal/pull/371

Setup

Login to the portal http://localhost:3000

Start storybook

yarn storybook

Login to storybook http://localhost:6006, though the command above should open it for you

Screenshots

image image image

@shkeating shkeating requested a review from AnnaGingle February 28, 2024 01:19
@shkeating shkeating self-assigned this Feb 28, 2024
@shkeating shkeating requested a review from a team as a code owner February 28, 2024 01:19
Copy link

This pull request has been linked to Shortcut Story #2310: investigate header order implications of hello, $name.

AnnaGingle

This comment was marked as resolved.

@AnnaGingle AnnaGingle self-requested a review February 28, 2024 16:38
@AnnaGingle
Copy link

AnnaGingle commented Feb 28, 2024

I have a couple of heading levels I want to double-check on.

USSF documentation

The accordion header is H2 and it just looks off to me. Also, the footer H2s are smaller and visually proportional. Can we update the styling of the accordion's H2s?

My Space

The collection header is not visually an h2. I think this fails 1.3.1-heading-level.

See the failing example from the Windows course docs

A web page is displayed when individuals finish a course that is being piloted.
image3
This would FAIL 1.3.1-heading-level. Let's take a look and see why:

If ALL of the following are TRUE, then the content PASSES:
Every programmatically identified heading level logically matches the visual heading structure on the page.
The structure outline in ANDI is:

<h4>Conclusion and Contact Information</h4>
<h3>Contact Information</h3>
<h3>OAST</h3>
<h1>Evaluation</h1>
This DOES NOT match the visual structure of the page. Conclusion and Contact Information” <h4> also needs to be at a higher level than “Contact Information” `<h3>` which is both visually smaller and contextually a subheading under it. Thus, the programmatic heading for “Conclusion and Contact Information”<h4> does not match the visual structure.
Both “Contact Information” and “OAST” are a <h3> heading level. However, contextually “Contact Information” should be a higher level than its subheading, “OAST.” We expect to see “OAST” listed as an <h4> or lower, yet it is listed at the same level as its parent heading. The programmatic heading does not logically match the visual structure.

Copy link

@AnnaGingle AnnaGingle left a comment

Choose a reason for hiding this comment

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

Everything is good to go within the scope of this pr!

@shkeating
Copy link
Contributor Author

shkeating commented Mar 4, 2024

I have a couple of heading levels I want to double-check on.

USSF documentation

The accordion header is H2 and it just looks off to me. Also, the footer H2s are smaller and visually proportional. Can we update the styling of the accordion's H2s?

My Space

The collection header is not visually an h2. I think this fails 1.3.1-heading-level.

re: myspace page 1.3.1 means that h2s are less visually prominent than h1s and more visaully promient than h3s. based on that, this would pass. I do think we need to update the design system docs in storybook to reflect the changes in what an h1-h5 is though.

@shkeating shkeating requested a review from a team as a code owner March 5, 2024 18:08
@gidjin gidjin merged commit 254b456 into main Mar 7, 2024
12 checks passed
@gidjin gidjin deleted the sc-2310-header-welcome-name-semantics branch March 7, 2024 00:31
gidjin pushed a commit that referenced this pull request Mar 7, 2024
## [4.31.2](4.31.1...4.31.2) (2024-03-07)


### Bug Fixes

* a11y: heading elements usage and ordering  ([#1220](#1220)) ([254b456](254b456))
* Settings menu on widgets can be accessed by keyboard ([#1222](#1222)) ([5cf1162](5cf1162))
* Typo on News page ([#1226](#1226)) ([3880ea8](3880ea8))
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