Make h1 hold full title while maintaining formatting
#531
Conversation
milofultz
changed the base branch from
Development
to
issue-456/home-page-structure-enhancements
| @@ -35,6 +35,7 @@ | |||
| "@mui/material": "^5.12.2", | |||
| "@mui/styled-engine-sc": "^5.12.0", | |||
| "@mui/system": "^5.12.1", | |||
| "@mui/utils": "^5.14.18", | |||
There was a problem hiding this comment.
What's this additional dependency? I can't find any documentation on it.
The npm repo makes it look like it may be private: https://www.npmjs.com/package/@mui/utils
There was a problem hiding this comment.
This is what MUI recommended for screen-reader-only text: https://mui.com/system/screen-readers/
Turns out the dep is already in there (check out package lock), so this isn't actually much extra for the bundle
src/pages/Home.jsx
Outdated
| <Stack component="span" justifyContent="center" alignItems="center" spacing={2} mb={2}> | ||
| <Typography variant="h1" component="span" fontWeight="500" fontSize="72px"> | ||
| {heading} | ||
| {punctuation} |
There was a problem hiding this comment.
I'm not quite sure I understand what this punctuation is doing. Could you describe it a bit?
There was a problem hiding this comment.
The punctuation is largely just a nice thing for screen reader users, so when they get to the h1, they don't hear "PASS Personal Access..." as all one sentence. The colon will usually provide a pause between "PASS" and "Personal...". Otherwise, assistive technologies may say "colon" literally, which is also descriptive.
There was a problem hiding this comment.
Is there a way to achieve this effect without writing screen reader specific code? Maybe wrap the section in a header tag and wrap the subtitle in a p element? https://mikebifulco.com/posts/semantic-html-heading-subtitle#use-this-code-to-add-subtitles-to-headings
PASS is volunteer-driven, and it doesn't have the resources to test PRs in multiple display environments. It's already a challenge to fully test our app in both desktop and mobile setups, so we can't guarantee that screen reader code will ever be validated, or maintained.
The ideal would be code that is fully agnostic to the display environment, and functions comparably in all environments. That way, a visual validation in the most recent version of Chrome can make us reasonably confident of the app's behavior in JAWS and NVDA. It's also easier to write automated tests for that kind of code.
I'm 100% on board with making PASS more usable for screen readers. We just need to do it in a way that's maintainable for a project this size. It's kind of a fun design challenge :)
There was a problem hiding this comment.
I hear you on maintainability. This code may not be the best solution (likely), but as you said, more maintainable and accessible solutions may require some significant changes elsewhere, when dealing with issues in the future.
As for the h1/p suggestion, the text to be rendered needs to be within the h1 for it to be presented to the screen reader. If someone hears “PASS”, it doesn’t actually say what it is.
Another way would be leaving “PASS” out of the h1 and only containing the expanded acronym in the h1. Might be a bit cleaner overall.
There was a problem hiding this comment.
All this being said, I could leave some comments explaining the purposes of these elements.
There was a problem hiding this comment.
I cleaned it up a little and added a comment there. Happy to try something else, too, but this should be fairly simple to maintain, I think.
There was a problem hiding this comment.
There was a problem hiding this comment.
This should be ok then. I think it would be good to go over accessibility standards with the project team at some point.
@milofultz do you think you would be able to add a small section to the testing wiki about testing the app with screenreaders and other assistive technologies? It's been a few years since I've done accessibility testing and I don't remember what the best practices are.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
milofultz
force-pushed
the
issue-456/home-page-structure-enhancements--h1
branch
from
1b57dd9 to
3be9bea
Compare
|
The test failed but once that's resolve I think this is good to go. |
|
The re-written passing tests are in the branch we are trying to merge this code fix into. |
milofultz
force-pushed
the
issue-456/home-page-structure-enhancements--h1
branch
from
3be9bea to
80eec96
Compare
Rebasing this branch onto yours to get those tests |
|
Assuming tests pass, I assume we could merge this and do a fast follow later for the concerns @timbot1789 has to not impede your branch merging @xscottxbrownx . Will leave it up to one of you three, as you have more PASS domain knowledge of what's best in a situation like this than I do. |
xscottxbrownx
merged commit 323a65c
into
issue-456/home-page-structure-enhancements
andycwilliams
deleted the
issue-456/home-page-structure-enhancements--h1
branch
Part of PR 516.
This puts the whole heading and subheading within the
h1, maintaining formatting and visuals.