✨ Site accessibility improvements #4131
Conversation
|
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 2057 (a01871) ❌ Edited: 2024-12-03 16:14:42 UTC |
| /** | ||
| * Convert 0 to "0th", 1 to "1st", etc. | ||
| * */ | ||
| export function getOrdinalNumberString(n: number): string { |
There was a problem hiding this comment.
Not a big deal, but consider using Intl.PluralRules, which would make this more readable (IMHO) and standardized.
There was a problem hiding this comment.
@rakyi Just a drive by comment to say thanks for mentioning this, I didn't know about it and this seems pretty neat. Maybe post about it on slack in developers or tools-and-workflows?
| // inert isn't supported in react@17 | ||
| // prevents focus on elements that are not visible | ||
| // ideally would only apply to elements below the fold but that's hard | ||
| {...{ inert: isClosed ? "true" : undefined }} |
There was a problem hiding this comment.
Is this really what we want? What problem do we solve? It also prevents users from selecting text within the element, and the text will be excluded from browser search, which seems too harsh.
There was a problem hiding this comment.
The issue I was trying to address here is that you can tab through links that are hidden, which renders poorly (see the video.)
I tried setting up a prop that would set tabIndex="-1" on any nested span-link elements but couldn't get it to work in the small amount of time I was budgeting for this PR so I went with this simpler solution instead.
I'm open to changing this though, if you think this change is an overall downgrade 🙂
tabbing.mov
There was a problem hiding this comment.
Hmm, I'm not sure what's better. Let's go with what you did and revert/rework later, if necessary.
| ...restProps | ||
| }: ComponentPropsWithRef<"button">) { | ||
| }: ComponentPropsWithRef<"button"> & { index: number }) { |
There was a problem hiding this comment.
Actually, I was meaning to change this to a div instead of a button and remove the interactivity. The buttons are too small for touch navigation — it was flagged as such by some other web test tool I used. Should we do that instead?
|
This PR has had no activity within the last two weeks. It is considered stale and will be closed in 3 days if no further activity is detected. |
Some QOL things I noticed + Lighthouse recommendations
A few +/-5% lightness adjustments here and there because the background colour isn't white, because that's easier than adding $blue-61
I showed the
*:focus-withinrule to Marwa and she's happy with it 👍