Accessibility - 3313 - Wrap abbreviations in <abbr> tag sitewide #5599
Conversation
|
@mnigh Does adding the abbreviations scramble the MacOS Voiceover even more? |
| title?: string, | ||
| ): JSX.Element => { | ||
| const stringifyText = text && text.toString(); // grabs text from React.ReactNode (is there a better way to get text from React.ReactNode type?) | ||
| if (typeof stringifyText !== "string") { |
There was a problem hiding this comment.
I'd recommend logging a warning message if this happens.
There was a problem hiding this comment.
Could you use the logger in frontend/common/src/hooks/useLogger.ts instead of console? This will allow us to log other places, like Azure AppInsights, for example.
There was a problem hiding this comment.
Unfortunately, I can't use the useLogger hook since wrapAbbr() isn't a component or hook function. It looks like for most of these "message" functions we have a fallback string, so I used that. I removed the console warning's and made sure all edge cases display that title. I made the changes here 78adc9d
There was a problem hiding this comment.
Yeah, that works OK. If you do need to use the logger outside a component you can import the logger directly instead of using the hook, like in frontend/common/src/components/Auth/AuthenticationContainer.tsx .
| </abbr> | ||
| ); | ||
| default: | ||
| return ( |
There was a problem hiding this comment.
Maybe worth logging a warning here, too.
There was a problem hiding this comment.
Could you use the logger in frontend/common/src/hooks/useLogger.ts instead of console? This will allow us to log other places, like Azure AppInsights, for example.
@petertgiles not sure about scrambling abbreviations up (do we mean this?), but it does some weird stuff. for example, have a look at how it reads the Group and Level chip. @esizer does this seem correct to you? Screen Recordingpools.mov |
There was a problem hiding this comment.
Nice start @yonikid15! This is definitely a difficult issue.
Screenshots
This group value is missing abbreviations, though maybe this is not possible since its value comes from an open text field?
The entire IT-01 is part of the abbr instead of just IT.
Missing abbreviations (GC and CIO in the following screenshot but more throughout the project)
The chip seems to be a little verbose but other than that it seems okay. Maybe we could drop the "Information Technology" off of the chip? I was talking with @yonikid15 about this and think we need a page that displays definitions for all of the acronyms. They are not very intuitive for non-government people and could be valuable. |
|
Reasons not to use abbreviations in page titles or headings:
|
Oh sorry, no for this one specifically Rosalie said it's not necessary since the abbreviation title is next to it.
Since we want the screen reader to ready I T 1 or A S 4 the entire group and level were added to the abbreviation. Otherwise it would read out "I T minus 0 1" or "A S minus 0 4". Unless you know of a better solution. Ideally we get rid of the "-0" throughout the site, but I'm not sure if that's possible.
Oh great thanks! I'll add these in. |
apps/web/src/pages/Home/HomePage/components/Opportunities/Opportunities.tsx
Show resolved
Hide resolved
🤖 Resolves #3313
👋 Introduction
The goal of this PR is to wrap all classification abbreviations in
<abbr>tag, and add helper functions to make it easier from now on. After doing research it's important to "...use the element consistently throughout your web page. If you use an abbreviation in one part of the page, make sure to use the<abbr>element to define it, and provide the full expansion using the title attribute. This will help to ensure that the user has access to the full meaning of the acronym or abbreviation wherever it is used on the page". (https://webreference.com/html/tags/abbr/)Talent Search Pages:
Admin:
IAP:
🕵️ Details
There were a lot of changes to copy so I'm expecting some typos. Please be thorough in your review.
🧪 Testing
<abbr>tag. They should have a dotted underline and show the full title when hovered over.