Legend accessibility

[sam-silv]

In their current form, in our product we can't use the builtin react-vis legends as they provide no keyboard accessibility, specifically tabindex and aria-* attributes.

Probably a bit wide for one issue, but it would be nice to be able to make series that have event handlers focusable too.

As always, happy to pitch in with a PR along these lines!

[mcnuttandrew]

You raise some great points! I guess we don't usually think about accessibility in visualization frameworks, but we obviously should be doing that.

I'm not incredibly familiar with the process of making things accessible, though we could certainly be better with using semantic tag names rather than just divs everywhere. Would you mind filing a pr to update one of the legends as an example of what we should be doing?

[sam-silv]

Sure thing, I'll have a go at putting together a PR for DiscreteColorLegend, and I'll ponder approaches for a broader strategy

[sam-silv]

@mcnuttandrew would you have objections to adding something like eslint-plugin-jsx-a11y to the config of react-vis?

Using the recommended config, this is the resulting output

/Users/samh/Projects/react-vis/src/plot/voronoi.js
  16:9  error  onMouseOver must be accompanied by onFocus for accessibility  jsx-a11y/mouse-events-have-key-events
  16:9  error  onMouseOut must be accompanied by onBlur for accessibility    jsx-a11y/mouse-events-have-key-events

/Users/samh/Projects/react-vis/src/sankey/index.js
  107:15  error  onMouseOver must be accompanied by onFocus for accessibility  jsx-a11y/mouse-events-have-key-events
  107:15  error  onMouseOut must be accompanied by onBlur for accessibility    jsx-a11y/mouse-events-have-key-events

/Users/samh/Projects/react-vis/src/treemap/treemap-leaf.js
  82:7  error    Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
  82:7  warning  Visible, non-interactive elements should not have mouse or keyboard event listeners             jsx-a11y/no-static-element-interactions
  82:7  error    Visible, non-interactive elements with click handlers must have role attribute                  jsx-a11y/onclick-has-role

/Users/samh/Projects/react-vis/showcase/index.js
  78:9  error  Links must not point to "#". Use a more descriptive href or use a button instead  jsx-a11y/href-no-hash

/Users/samh/Projects/react-vis/showcase/plot/voronoi-line-chart.js
  78:9  error  Form controls using a label to identify them must be programmatically associated with the control using htmlFor  jsx-a11y/label-has-for

✖ 9 problems (8 errors, 1 warning)

Interestingly, it doesn't catch the issue with DiscreteColorLegendItem, probably due to the object spread notation used to add the onClick prop to the div.

Would like to hear your take, I don't think it's too much effort to put together something that is compliant.

[mcnuttandrew]

@samhogg, I think adding eslint-plugin-jsx-ally is a splendid idea! Maybe you know more about the react-ally stuff than I do: do you know if the react-ally package will produce different results than the linter? If so should we use both?

[sam-silv]

@mcnuttandrew

I'm going to do a bit more digging, but from what I can tell, react-a11y actually inspects rendered elements, so might catch more than the linter, which is just static. It does mutate the React object though, so we should only use it when hacking on the library (and potentially testing), not in the packaged build - it's also pretty verbose in your console

[sam-silv]

@mcnuttandrew (and anyone else who might be playing along at home) - in the midst of putting together a PR here - should have it in by the end of the week.