Better Headings Structure #1473
Conversation
ChristopherChudzicki
force-pushed
the
cc/headings
branch
from
869ae6c to
eccf0e4
Compare
| h1: "span", | ||
| h2: "span", | ||
| h3: "span", | ||
| h4: "span", | ||
| h5: "span", |
There was a problem hiding this comment.
This is to prevent accidental future headings.
In general, our h1...h5 font variant may not correspond to an h1...h5 header, which was the old behavior.
| ` | ||
|
|
||
| const Title = styled.h3<{ lines?: number; size?: Size }>` | ||
| const Title = styled.span<{ lines?: number; size?: Size }>` |
There was a problem hiding this comment.
This, and a similar change to ListCard, broke a lot of tests. (A downside of testing roles). Thats accounts for a lot of the changed files.
There was a problem hiding this comment.
This file was unused. It was our old (pre-professional designs) card template.
| disableEnforceFocus={disableEnforceFocus} | ||
| PaperProps={PaperProps} | ||
| TransitionComponent={Transition} | ||
| aria-labelledby={titleId} |
There was a problem hiding this comment.
This was an issue I noticed while looking at accessibility stuff. Previously MUI did this for us, but when we stopped using their DialogTitle component, we lost the automatic id assignment.
| screen.findByRole("tab", { name: tab }).then(async (featuredTab) => { | ||
| await user.click(within(featuredTab).getByRole("tab", { name: tab })) | ||
| const [featuredPanel] = screen.getAllByRole("tabpanel") | ||
| await within(featuredPanel).findByText(resources.results[0].title) | ||
| }) |
There was a problem hiding this comment.
I decided to just remove this test. It was broken and not adding much value, IMO. Testing the behavior of the resource carousel (clicking tabs, etc) is pretty well covered in ResourceCarousel.test.tsx
How it was broken: screen.findByRole("tab", ... at the end here is a promise and it was not being awaited.
- if you insert an
awaithere you'll see the test fails onmain - Usually un-awaited asynchronous assertions can cause artificial failures on subsequent async tests in the same file, but since this was the last test with any
awaits, we weren't noticing any.
ChristopherChudzicki
force-pushed
the
cc/headings
branch
from
efcf4f4 to
271859b
Compare
ChristopherChudzicki
added
the
Needs Review
gumaerc
left a comment
gumaerc
left a comment
There was a problem hiding this comment.
👍
This looks good. I checked out as many pages as I could using HeadingsMap and the flow on every page looks a lot more logical.
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/5242
Description (What does it do?)
Adds better headings to our pages.
Screenshots (if appropriate):
Home / Search
Unit / Department / Topic Channel
Unit / Department / Topics / Listing
About Us / Privacy
How can this be tested?
Check that:
VO + U(where VO = voiceover key combo) opens the "Rotor". Then arrow keys nav to the headings navigation section.