Skip to content
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.

Sign up for GitHub

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
+173 −37
Open

tkakar/cat-1029-fix-links-for-accessibility #3620

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3e5fb67
Linting and type fixing
tkakar Nov 25, 2024
87b9502
Fixed import for LZString
tkakar Nov 25, 2024
85dd9ae
Moved ariaLabelText addition logic to parent components
tkakar Dec 2, 2024
9bac495
Merge branch 'main' into tkakar/cat-1029-fix-links
tkakar Dec 2, 2024
c0c6403
Updated changelog message
tkakar Dec 2, 2024
dcd164e
Addressed html having lang attribute issue
tkakar Dec 2, 2024
a1968fb
Fixed barchart aria-label logic
tkakar Dec 2, 2024
744062d
Fixed types issues
tkakar Dec 3, 2024
db32c64
Merge branch 'main' into tkakar/cat-1029-fix-links
tkakar Dec 3, 2024
ff2d4bb
Sorted datasets in the provTable component
tkakar Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG-links-with-discernible-text.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Added discernible text to the links on stacked barcharts.
27 changes: 26 additions & 1 deletion context/app/static/js/shared-styles/charts/StackedBar/StackedBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import React, { SVGProps } from 'react';

import LZString from 'lz-string';
import { FiltersType } from 'js/components/search/store';
import { StyledRect } from './style';

type Direction = 'vertical' | 'horizontal';
Expand Down Expand Up @@ -55,8 +57,31 @@ function StackedBar({ direction = 'vertical', bar, hoverProps, href }: StackedBa
<StyledRect fill={bar.color} {...mappedProps} $showHover={Boolean(hoverProps) || Boolean(href)} {...hoverProps} />
);
if (href) {
const page = href.split('?')[0].replace('/search/', '');
const encodedURI = href.split('?')[1];
interface searchObj {
filters: FiltersType;
}

const decodedHref = JSON.parse(LZString.decompressFromEncodedURIComponent(encodedURI)) as searchObj;

Copy link
Collaborator Author

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

Copy link
Collaborator

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.

// Extracting the filters from the searchURI
const extractedValues = (() => {
const results: string[] = [];
Object.values(decodedHref.filters).forEach(({ values }) => {
if (Array.isArray(values)) {
results.push(...values);
} else if (typeof values === 'object' && values !== null) {
results.push(...Object.keys(values));
}
});
return results;
})();

const labelString = `${page} page for the selected bar representing ${extractedValues.join(', ')}`;

return (
<a href={href} target="_parent">
<a href={href} target="_parent" aria-label={labelString}>
{rect}
</a>
);
Expand Down
6 changes: 5 additions & 1 deletion context/app/static/js/shared-styles/tiles/Tile/Tile.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ function Tile({
</StyledPaper>
);
if (href) {
return <a href={href}>{tile}</a>;
return (
<a href={href} aria-label={href}>
{tile}
Copy link
Collaborator Author

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

Copy link
Collaborator

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.

</a>
);
}
return tile;
}
Expand Down
Loading