Support for adding accessibility features at chart generation

[gherka]

Currently, to add accessibility features to dc.js charts you need to do it as part of post-processing. It would be easier to simply specify what features you'd like to include together with other chart attributes, like width and height and labels. This idea has been brought up before (#1185) and after raising it again on the mailing list recently, @gordonwoodhull encouraged me to try to implement something.

I'm thinking of a series of commits / PRs to cover the basics:

• This one makes all SVG charts accessible by keyboard using tabindex. It also adds a description that screen readers can access. The configuration of a <desc> element and aria-labelledby attribute seems to have the best support according to this. I tried adding <title>, but it fails a few tests looking at tooltips and besides, some screen readers will read it twice. It defaults to an empty string and ideally, I'd like it to reflect the default chart type - so a dc.PieChart will have "pie chart" in the description and so on. This will likely require modifying each chart file with some default attribute, however. Users can add a description for their charts by simply passing a string to .svgDescription().

• Make internal chart elements, like <circle> in a bubble chart or rect in a bar chart keyboard-selectable. Also, make them "clickable" from keyboard.

• When I tested tooltips created using <title> in Narrator / NVDA, I got mixed results for a variety of browsers. In some, the tooltip text was read out, on others it wasn't. There is probably a way to make it a little bit more consistent.

• Optionally designating the NumberDisplay widget as aria-live region, meaning changes to it will be read out by screen readers.

Here's the summary of this PR as it currently stands:

• root SVG can get focus and users can add custom description using svgDescription method

• internal chart elements can get focus and filtering interactions will trigger on Enter or Space as if clicked

• CSS will behave on focus in the same way as it behaves on hover

• Individual chart types are accessible

  • Bar chart
  • Box plot
  • Bubble chart
    • DOM order of bubbles can be changed based on x value
  • Choropleth
  • Heatmap
  • Line chart
  • Pie chart
  • Row chart
  • Scatter plot
    • DOM order of bubbles can be changed based on x value
  • Sunburst chart

• NumberDisplay can optionally be set as aria-live region

• Legends can get focus and interactivity

• Add example

[gordonwoodhull]

Haven’t tested yet, but it looks good!

I think you can default svgDescription to something reasonable with this.constructor.name

https://stackoverflow.com/a/10314492

[gherka]

Good idea! I've added it and set a basic test up on a bar chart. I also checked other chart examples (pie, line, row, composite) and it works as expected.

[gherka]

To make significant internal chart elements like bars in a bar chart accessible, I've added a dc prefixed class in the chart's module, but since the effects of this feature are the same across all chart types (tabindex=0, triggers onClick on keypress), the function is part of base-mixin.js. This way, if a new chart type comes along, its creator would only need to think about which elements are "significant" and simply add the dc-tabbable class to them.

Also, elements created at the initial render can disappear and new ones appear after filtering - so the new function would need to run on both render() and redraw().

@gordonwoodhull, if you approve of this approach, I'll go through all existing chart types and amend them in the same way as I did with the bar chart, and write some tests.

[gordonwoodhull]

I hope to take a look at this today, but I’m out sick, so please bear with me if it takes a little longer.

[gherka]

Of course, no worries at all. Hope you get better soon!

[gordonwoodhull]

I like the general-purpose approach and clean implementation. Also it preserves backward compatibility for people who are doing something else with the keyboard.

A few nits below, otherwise this looks great!

[gordonwoodhull]

Please document the default here (in prose)

[gordonwoodhull]

Looks great, thank you!

[gherka]

Had to modify the approach a little bit because the way .on('click', fn) function is specified varies between concrete chart implementations - could be a straightforward onClick(d), or onClick(d, layerIndex) or _onClick(d) or onClick(d, i) or just an anonymous function. Now in addition to adding a dc-tabbable class to the relevant elements, you also need to tell _makeKeyboardAccessible what to do on click.

A couple of other observations:

• the DOM order of bubble chart circles is based on the key - what do you think of making it based on the x-value instead so that tab order goes predictably from left to right? We'd need to preserve the option of putting smaller-radius circles on top with sortBubbleSize
• since BubbleOverlay requires "external" SVG to function, generateSVG from BaseMixin isn't called so svgDescription isn't added, even if called. What's the best way to signpost this behaviour?

[gordonwoodhull]

Hi @gherka, sorry for the slow reply.

Yes, I think that ordering the bubble chart circles by X value would be okay, as long as sortBubbleSize is still respected. I don't think it makes any difference except for Z and tab order.

That's an interesting question about BubbleOverlay. I don't think this chart is used very much, so it might not matter too much, but I suppose you could do the annotation on render (or _initOverlayG) instead? I know it's ugly to have a special case for this one chart, but that's just what is, a single chart that doesn't create its own svg.

[gherka]

I think this now covers everything in the initial scope of features. There's just a couple of things I'm not too sure about -

• What tests do I need to add? Some, like ensuring that NumberDisplay doesn't have any transitions when keyboardAccessible is true can get quite tricky and haven't been able to figure out how to do it yet.
• What examples do I need to add? While developing, I've tested each feature on the web-src examples so they can all potentially work.
• What about resolving conflicts? Do I need to do it in my PR?

[gordonwoodhull]

One small suggestion, and a bigger change to update to dc@4.1.

[gordonwoodhull]

Maybe test if the key accessor returns truthy value? Just to be careful.

[gherka]

I tried testing for it (this.keyAccessor()(d)), but it failed for bar and pie charts and testing this.keyAccessor() is always truthy because it returns a function. Is there another way?

[gordonwoodhull]

I agree, there probably is no better way. I am not sure that checking typeof d === 'object' provides all that much protection, since objects are pretty common in JS.

Were you seeing errors if you didn't check this? Or just being careful?

[gherka]

I thought you were on holiday! :) Yes, on charts like boxplot where you have a group with bound data and a clickable circle element. If you press enter on the circle, the event triggers, then propagates up to the parent group. If we don't check for d type, the onClick function will try to run twice and the first call (from circle) will error out with TypeError: filters[0] is undefined.

[gordonwoodhull]

Thanks. Okay, let’s stick with the object check for now. Maybe we will figure something out (like another CSS class?) later.

[gordonwoodhull]

Hi @gherka!

Sorry for the slow reply - I am on vacation this week and next.

• We don't generally test animated transitions, so I think it is okay to skip that. The specific tests that you made for edge cases are great! It would also be good to test the general case for each chart, i.e. that each element has been modified. This will help to prevent regressions in the future when other people work on the code.
• I think it would be sufficient to add one example with a few charts - for example, maybe adding accessibility to the filtering example - so that people can try the feature out.
• Yes, please rebase your branch onto develop and then force-push the result to resolve conflicts. The PR will update automatically.

Unfortunately, there was a big change in dc@4.1 which affects your PR. As I mentioned above, the signature of event handlers changed in d3@6, and we are using an adapter to continue support for dc@5 until we decide to drop it. Please use this adapter and test your code with d3@5 and d3@6 to make sure it functions correctly. We are no longer testing d3@5, so I'll take your word on this.

Thanks for your careful work on this feature - people have asked for this for a long time, and it was something that worried me personally. It's good to see that accessibility can be added without much disruption to other functionality.

[gherka]

Hi @gordonwoodhull, thank you for your feedback! In this commit I've added automatic tests for each chart type that I had modified and also tested each chart type on the standard examples using v5 and v6. As you suggested, I've included keyboardAccessible() and svgDescription() in the filtering.html example.

There are a few minor things that can be tweaked further, but the base functionality is now in place, I think.

[gordonwoodhull]

Fantastic work. Love to see thorough tests, and I finally tried it out via the new example and everything works flawlessly, as far as I can tell. The only thing I see you have not tested is newly added elements on other charts, but I think it's okay to skip that, given how tedious that is to test.

This is almost ready to merge, but I'd like to see the tests tightened up a little, a little cleanup to the number display, and also the example clarified, as mentioned below.

Thanks for your hard work!

[gordonwoodhull]

Excellent work, thanks @gherka!

I squashed the commits and released this as 4.2.0.