-
Notifications
You must be signed in to change notification settings - Fork 2
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
tkakar/cat-1029-fix-links-for-accessibility #3620
base: main
Are you sure you want to change the base?
Conversation
return <a href={href}>{tile}</a>; | ||
return ( | ||
<a href={href} aria-label={href}> | ||
{tile} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we want to parse the hrefs in this case by extracting keywords from them. But it would need some kind of mapping, easy for organs, difficult for datasets page with long uuids that are not meaningful
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto here - I think constructing a descriptive text would be more straightforward if we were to define that logic from the parent element that uses these tiles.
} | ||
|
||
const decodedHref = JSON.parse(LZString.decompressFromEncodedURIComponent(encodedURI)) as searchObj; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a first pass on this, this extracts the filter (assaytype, etc) and organ, we can also include the count (shown on hover) but not sure if that's important as hover does provide that info?
Also, would be happy to move it to some utils file, or adding more meaningful content to the aria-label
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we could pass the aria-label text generation logic to the stacked bar chart, then directly pass the label as a prop here? That seems simpler than reverse engineering the aria label from the href, and will work for non-search page links as well.
Summary
This PR addresses the accessibility issue of
Ensuring links having discernible text
by adding aria-labels to any links used in the code base, particularly, the tiles and stacked barchart components.Design Documentation/Original Tickets
CAT-1029
Testing
Manually tested with Axe DevTools
Screenshots/Video
Include screenshots or video demonstrating any significant visual or behavioral changes.
Checklist
CHANGELOG-your-feature-name-here.md
is present in the root directory, describing the change(s) in full sentences.Additional Notes
Please specify any additional information or context relevant to this PR.