Improve accessibility of React components #1612

ivababukova · 2023-07-17T11:16:41Z

Fixes #1467 and Fixes #787

Background

Change List

Checked each Material UI component and made sure its usage conforms with its accessibility guidelines
Added aria labels
Fixed this issue with colours to improve accessibility Accessibility: Numbers not readable on dark colored sliders #787
Made the dropdowns in PopperMenu clickable on touch screen

Checklist

Ensure PR works with all demos on the dev.vitessce.io homepage
Open (draft) PR's into vitessce-python and vitessce-r if this is a release PR
Documentation added or updated

github-actions · 2023-07-17T11:32:10Z

Size Change: +7.91 kB (0%)

Total Size: 11.4 MB

Filename	Size	Change
`./packages/main/prod/dist/index-********.js`	6.94 MB	+7.91 kB (0%)

ℹ️ View Unchanged

Filename	Size
`./packages/main/prod/dist/deflate-********.js`	243 B
`./packages/main/prod/dist/hglib-********.js`	4.34 MB
`./packages/main/prod/dist/index.min.js`	687 B
`./packages/main/prod/dist/jpeg-********.js`	15.3 kB
`./packages/main/prod/dist/lerc-********.js`	47.2 kB
`./packages/main/prod/dist/lzw-********.js`	2.1 kB
`./packages/main/prod/dist/packbits-********.js`	576 B
`./packages/main/prod/dist/pako.esm-********.js`	68.6 kB
`./packages/main/prod/dist/raw-********.js`	168 B
`./packages/main/prod/dist/webimage-********.js`	872 B

_{compressed-size-action}

github-actions · 2023-07-18T09:50:57Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	78.47%	9265 / 11807
🔵	Statements	78.47%	9265 / 11807
🔵	Functions	65.6%	267 / 407
🔵	Branches	79.29%	881 / 1111

File Coverage

File	Stmts	% Branch	% Funcs	% Lines	Uncovered Lines
Changed Files
packages/view-types/scatterplot/src/shared-spatial-scatterplot/ToolMenu.js	72.22%	100%	66.66%	72.22%	105-144

Generated in workflow #3114

packages/vit-s/src/shared-plot-options/CellColorEncodingOption.js

packages/view-types/feature-list/src/FeatureListOptions.js

keller-mark · 2023-07-18T14:26:55Z

packages/view-types/layer-controller/src/styles.js

+export const channelSliderStyles = makeStyles(theme => ({
+  valueLabel: {
+    marginTop: '7px',
+    '& *': {


Can this selector be more specific?

Yes, done in this commit: ee2c738

packages/view-types/scatterplot/src/ScatterplotOptions.js

keller-mark · 2023-07-18T14:31:25Z

packages/view-types/spatial/src/SpatialOptions.js

-  return (
-    <TableRow>
-      <TableCell className={classes.cameraLabel}>
+  return use3d && (


Is this change to using rendering vs. the disabled prop related to accessibility?

It is done for the same reasons as this one: https://github.com/vitessce/vitessce/pull/1612/files#r1266860907 Let me know if I should revert it.

I read the comments further and I see that both of you are on the opinion that using disabled is better, so I reverted this change in commit e5371cb

packages/view-types/spatial/src/SpatialOptions.js

keller-mark · 2023-07-19T21:47:57Z

We should also start using https://react.dev/reference/react/useId

NickAkhmetov

Thanks for taking on this monumental task!

I have some suggestions after reviewing tooltip and view-types; I will continue my review picking up from the vit-s directory tomorrow.

To sum up my thoughts so far:

I agree with @keller-mark re: the use of useId, especially for components which may appear more than once on a page; does the current solution work if there's a second vitessce instance further down the page?
I also second preferring to disable controls rather than conditionally render them. Controls popping in and out of existence is detrimental to user experience, especially for users of screen-readers.
I think a lot of the places where key is currently in use may be better served with an id instead.
Slider accessibility may be further enhanced in cases where there are multiple thumbs on one slider by having dynamic aria labels for each thumb.

packages/tooltip/src/Tooltip.js

packages/view-types/feature-list/src/FeatureListOptions.js

packages/view-types/heatmap/src/HeatmapOptions.js

packages/view-types/layer-controller/src/VectorLayerController.js

packages/view-types/scatterplot-gating/src/GatingScatterplotOptions.js

packages/view-types/scatterplot/src/ScatterplotOptions.js

packages/view-types/scatterplot/src/shared-spatial-scatterplot/ToolMenu.js

packages/view-types/scatterplot/src/ScatterplotOptions.js

NickAkhmetov

After reviewing the rest of the changes, the feedback so far should cover just about everything I saw, i.e.:

Opt for disabled over conditional rendering
Use id instead of key in most places where a key has been added
Consider useId for anything that can appear more than once on a page

…ds in HeatmapOptions

NickAkhmetov

Great job, thank you for making those corrections!

I have a few remaining concerns:

I'm not sure that using ['string-1', 'string-2'].join('-') is the best for clarity in defining ID's; could we use template strings instead?
ARIA labels should be brief (same as any input label, ideally); if there's a full sentence/if a period is needed, using aria-description is a good way to provide any extra context while keeping the label itself short and to the point.

packages/view-types/feature-list/src/FeatureListOptions.js

packages/view-types/layer-controller/src/ChannelOptions.js

packages/view-types/layer-controller/src/ImageAddButton.js

packages/view-types/layer-controller/src/LayerController.js

…mmary

ivababukova · 2023-08-17T07:13:22Z

Thanks for the review @NickAkhmetov. I have now addressed all your comments. In terms of the concerns you outlined:

I'm not sure that using ['string-1', 'string-2'].join('-') is the best for clarity in defining ID's; could we use template strings instead?

Yes, you are totally right, I went with ['string-1', 'string-2'].join('-') purely out of habit. The code is now corrected to use template strings instead.

ARIA labels should be brief (same as any input label, ideally); if there's a full sentence/if a period is needed, using aria-description is a good way to provide any extra context while keeping the label itself short and to the point.

Good point. I have changed the aria-labels, removing the . and making the longer ones briefer, where sensible.

Could you take a look at this PR again and let me know if there is anything else that should be fixed?

NickAkhmetov

Great work, thank you for making those revisions! I have no further concerns; if @keller-mark has no additional comments, this should be good to merge in my opinion! 👏

packages/view-types/feature-list/src/FeatureListOptions.js

packages/view-types/layer-controller/src/RasterChannelController.js

packages/view-types/scatterplot-gating/src/GatingScatterplotOptions.js

packages/view-types/scatterplot/src/ScatterplotOptions.js

sites/docs/docs/tutorial-plugin-coordination-type.mdx

Co-authored-by: Mark Keller <7525285+keller-mark@users.noreply.github.com>

packages/view-types/feature-list/src/FeatureListOptions.js

ivababukova added 7 commits June 30, 2023 10:19

Improve accessibility of existing Material-UI components

1426193

Fix sliders label text not visible

8ea79e7

Improve accessibility of TitleInfo

82eea6b

Merge main branch in

4b3a9c5

make buttons on poppermenu clickable

b4b5ba8

same pnpm lock as main

f49e35e

update pnpm lock

9e93184

ivababukova changed the title ~~Ivababukova/accessiility~~ Improve accessibility of React components Jul 17, 2023

ivababukova added 2 commits July 17, 2023 15:15

add changeset

7ed7d04

forgot to import css class during merge of main branch

56168a8

keller-mark reviewed Jul 18, 2023

View reviewed changes

Merge branch 'main' into ivababukova/accessiility

c6d3f11

NickAkhmetov reviewed Aug 11, 2023

View reviewed changes

NickAkhmetov reviewed Aug 13, 2023

View reviewed changes

ivababukova added 13 commits August 14, 2023 08:50

Merge branch 'main' into ivababukova/accessiility

2fdc513

Remove key from TableRow because it doesn't improve accessibility

34a756f

Generate tooltip id with useId hook

8c848e3

Fix bug with react hook useId not always rendering and fix typos in i…

80821e2

…ds in HeatmapOptions

Use getAriaLabel function for Slider in HeatmapOptions

a4e69d5

Fix linting and actually use tooltipId value

b0c404d

Change color name to US convention

0dc2d0e

Better label for Slider in RasterChannelController

152a3bb

Better label for Slider in VectorLayerController

802c0c2

Make the styles definition more specific

ee2c738

fix typos

c6ee12f

one more typo

ec1f243

Better label for IconButton

7919472

ivababukova added 3 commits August 15, 2023 14:30

Address PR comments

337f8cc

Add labels to TableCell and fix eslint settings

7473eb6

Some small fixes that slipped previously

4c172a2

NickAkhmetov reviewed Aug 16, 2023

View reviewed changes

ivababukova added 4 commits August 16, 2023 17:15

Use string template

0627aff

Some small things

1054f81

Match the id of AccordionDetails with aria-controls id of AccordionSu…

a6bfe7c

…mmary

Make naming of aria labels consistent

c644063

NickAkhmetov approved these changes Aug 17, 2023

View reviewed changes

keller-mark reviewed Aug 17, 2023

View reviewed changes

ivababukova and others added 3 commits August 21, 2023 16:26

Update packages/view-types/feature-list/src/FeatureListOptions.js

f1a2f75

Co-authored-by: Mark Keller <7525285+keller-mark@users.noreply.github.com>

forgotten fixes of labels

d78a38f

fix comment

08336a8

keller-mark reviewed Aug 21, 2023

View reviewed changes

packages/view-types/feature-list/src/FeatureListOptions.js Outdated Show resolved Hide resolved

ivababukova and others added 9 commits August 22, 2023 10:12

fix

db5f915

more small fixes

2aa2009

small fixes

e645eb6

remove unneccesary keys

2f32d9c

another small fix

d5f61de

Accessibility fixes (#1652)

bf7a34f

Fix ariaLabel

bfe3ec2

Revert extra space

4986854

Revert extra space

555caa8

keller-mark approved these changes Aug 23, 2023

View reviewed changes

ivababukova merged commit e5469ee into main Aug 24, 2023

ivababukova deleted the ivababukova/accessiility branch August 24, 2023 11:03

ivababukova mentioned this pull request Aug 24, 2023

Accessibility: Numbers not readable on dark colored sliders #787

Closed

github-actions bot mentioned this pull request Aug 24, 2023

Create release #1639

Merged

keller-mark mentioned this pull request Sep 12, 2023

Title dropdowns not working on touch screen #877

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve accessibility of React components #1612

Improve accessibility of React components #1612

ivababukova commented Jul 17, 2023 •

edited

Loading

github-actions bot commented Jul 17, 2023 •

edited

Loading

github-actions bot commented Jul 18, 2023 •

edited

Loading

keller-mark Jul 18, 2023

ivababukova Aug 14, 2023

keller-mark Jul 18, 2023

ivababukova Aug 14, 2023

ivababukova Aug 14, 2023

keller-mark commented Jul 19, 2023

NickAkhmetov left a comment

NickAkhmetov left a comment

NickAkhmetov left a comment

ivababukova commented Aug 17, 2023 •

edited

Loading

NickAkhmetov left a comment

Improve accessibility of React components #1612

Improve accessibility of React components #1612

Conversation

ivababukova commented Jul 17, 2023 • edited Loading

Background

Change List

Checklist

github-actions bot commented Jul 17, 2023 • edited Loading

github-actions bot commented Jul 18, 2023 • edited Loading

Coverage Report

keller-mark Jul 18, 2023

Choose a reason for hiding this comment

ivababukova Aug 14, 2023

Choose a reason for hiding this comment

keller-mark Jul 18, 2023

Choose a reason for hiding this comment

ivababukova Aug 14, 2023

Choose a reason for hiding this comment

ivababukova Aug 14, 2023

Choose a reason for hiding this comment

keller-mark commented Jul 19, 2023

NickAkhmetov left a comment

Choose a reason for hiding this comment

NickAkhmetov left a comment

Choose a reason for hiding this comment

NickAkhmetov left a comment

Choose a reason for hiding this comment

ivababukova commented Aug 17, 2023 • edited Loading

NickAkhmetov left a comment

Choose a reason for hiding this comment

ivababukova commented Jul 17, 2023 •

edited

Loading

github-actions bot commented Jul 17, 2023 •

edited

Loading

github-actions bot commented Jul 18, 2023 •

edited

Loading

ivababukova commented Aug 17, 2023 •

edited

Loading