feat(a11y): add data table for screen readers (sunburst, treemap, icicle, flame) #1155

rshen91 · 2021-05-10T22:25:27Z

Summary

This PR creates a dynamic data table for partition type charts to present the data for screen reader users. There is an added row for percentages to provide more context to users of assisted technologies. In addition, the data table will provide more information on multilayer pie charts or small multiple partition charts. The partition data table for screen users can be seen when setting debug to true.

Details:

Fixes #1154

Need to fix <td> for icicle/flame charts or properly disable it in this PR
Need to add tests for this functionality
Limit the number of results for a data table OR render the top N results vs limiting the data 🤔

Checklist

Delete any items that are not applicable to this PR.

Unit tests were updated or added to match the most common scenarios

monfera

Very important and nice work Rachel, thanks 🎉

I added a bunch of comments which are mostly just minor improvements on preexisting code you retained; I don't think those changes impact logic. Each is a small suggestion around one function, but together add up to much shorter logic.

As a next step, eg. next week is good too, it'd be great to hop on a short call for jointly test driving it with VO. Also, could we include Michail Yasonik in the review?

src/chart_types/partition_chart/layout/utils/group_by_rollup.ts

src/chart_types/partition_chart/layout/utils/legend_labels.ts

monfera · 2021-05-19T09:07:24Z

src/chart_types/partition_chart/state/selectors/get_screen_reader_data.tsx

+  (specs, { legendMaxDepth }, trees) => {
+    const lengthOfResults = getScreenReaderDataForPartitions(specs, legendMaxDepth, trees).length;
+    // how to control how much is calculated
+    return lengthOfResults > 20


Wondering if 20 is good hardcoded, @markov00 might want to chime in. Also, even if it's too many, maybe the top N values could be kept, to not disadvantage the user with the loss of all items. Also, what's the harm in just doing it unconditionally for all sectors? Hopefully the VO user can "back out", ie. doesn't need to resort to hearing all the 0.1% slices before continuing with the rest of the page contents

Should we really limit that, or is the user that can directly skip the content after the first top N values?

Yeah I think the data table definitely impacts rendering performance when loading flame/icicle charts (particularly in terms of #1155 (comment)), so I wondering about implementing some sort of escape hatch if results are in the 1000s. This 20 is just arbitrary for now but I am definitely open to suggestions. 🙇🏻‍♀️

I think only rendering the top N in a data table sounds interesting! I wouldn't mind trying to implement that.

@myasonik do you have suggestions for larger datasets? I see the top N being kind fo cool and just having somewhere (maybe the table caption?) say the amount of results there were?

@myasonik in line with Rachel's question, it would be interesting to know if there's a good practice for handling the current tradeoffs between accessibility annotations (DOM with ARIA labels etc.) and other goals, as I'm not currently aware of an API that would detect if a screen reader is used. The other goals include, bounded rendering time, responding page, economy of power and CPU utilization, economy with browser memory. Unfortunately the resource usage of DOM rendering is roughly 100x larger than that of DOM rendering.

Rachel I'm also thinking that maybe, reaching a cap, a "slice row" for "Other" could be created, with a single, longer text description for screen reading. While that can still be a long string, it'd establish a bounded DOM element count. DOM and general webpage performance worsens with every DOM element, even if those DOM elements do nothing.

Rachel, another important thing to do is to ensure that the table and row creation do not cause undue layout work for the browser. Depending on a few factors, table rendering speed can vary wildly. The rule of thumb is that if the columns are of fixed width, then the layouting is quite fast, but if the column widths are driven by the table contents, which I believe is the default, then it becomes a much heavier task. Let's chat about this when we look at the PR together.

Was just going back through this PR and thought I'd document the outcome of this (we 3 chatted on zoom): In short, we're only rendering some subset of data at first (until a user interacts with a "load all" button) so there shouldn't be consequential impact to performance.

…legend width calc

…rst-a11y-slices

src/components/accessibility/partitions_data_table.tsx

src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx

myasonik

🚀

markov00

LGTM, great work here.
I'd like to see reuse of the table/components on other charts as well whenever possible

# [30.2.0](v30.1.0...v30.2.0) (2021-06-10) ### Features * **a11y:** add data table for screen readers (sunburst, treemap, icicle, flame) ([#1155](#1155)) ([87fd75f](87fd75f)), closes [#1154](#1154)

nickofthyme · 2021-06-10T17:03:41Z

🎉 This PR is included in version 30.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

# [30.2.0](elastic/elastic-charts@v30.1.0...v30.2.0) (2021-06-10) ### Features * **a11y:** add data table for screen readers (sunburst, treemap, icicle, flame) ([opensearch-project#1155](elastic/elastic-charts#1155)) ([a1a68fe](elastic/elastic-charts@a1a68fe)), closes [opensearch-project#1154](elastic/elastic-charts#1154)

rshen91 added :partition Partition/PieChart/Donut/Sunburst/Treemap chart related :accessibility Accessibility related issue labels May 10, 2021

rshen91 changed the title ~~feat(a11y): add data table for screen readers (sunbursts)~~ feat(a11y): add data table for screen readers (sunburst and treemap) May 14, 2021

rshen91 added 7 commits May 14, 2021 10:40

feat: add partition data table for screen readers

ebbee47

feat: add percentage row

d70b995

fix: update chart api

b7a030d

feat: add screen reader data data structure

8340b51

feat: add screen reader data table to partitions

863d6dc

feat: refactor utillity function and format data values

547c7af

fix: remove icicle and flame from getscreenreaderData check

2acc45e

rshen91 force-pushed the sunburst-a11y-slices branch from e5491b3 to 2acc45e Compare May 14, 2021 16:43

rshen91 added 3 commits May 14, 2021 13:48

test: update tests

fcd3fc8

fix: add catch for large data charts

15132c7

fix: fix conditional within td tag to display value.valueText

51a8be3

monfera reviewed May 19, 2021

View reviewed changes

rshen91 added 2 commits May 19, 2021 11:49

refactor: add code review clean up

b9b7dfd

fix: update per code review

7cb0d70

rshen91 requested a review from monfera May 19, 2021 19:39

rshen91 changed the title ~~feat(a11y): add data table for screen readers (sunburst and treemap)~~ feat(a11y): add data table for screen readers (sunburst, treemap, icicle, flame) May 19, 2021

test: fix tests

7e69878

rshen91 requested a review from myasonik May 20, 2021 16:00

rshen91 added 3 commits May 24, 2021 14:37

feat: add incremental table rows

d070a6c

fix: fix tr table issues

cdef1c2

style: clean up code

6f74271

rshen91 marked this pull request as ready for review May 24, 2021 20:57

rshen91 added 2 commits May 25, 2021 08:21

fix: fix conditional tfoot for show more cells

99a6d61

fix: clean up code

a14cc62

rshen91 added the wip work in progress label May 25, 2021

rshen91 added 2 commits May 25, 2021 10:00

fix: generate more data on button click

060d4ff

Merge remote-tracking branch 'upstream/master' into sunburst-a11y-slices

77fce3a

rshen91 and others added 12 commits June 4, 2021 12:40

fix: update valueText formatter

17568c6

test: update tests

49165d0

Merge remote-tracking branch 'upstream/master' into sunburst-a11y-slices

dd1ba26

refactor: update table generation

b180214

Merge remote-tracking branch 'upstream/master' into sunburst-a11y-slices

42b4f0e

refactor: small storybook changes and refactor legend labels

3cd61db

fix: getLegendItemsLabels uses all small multiple panel data for the …

be561f9

…legend width calc

chore: removed superseded function

228bc32

feat: add sort and update tests

c3a361d

Merge remote-tracking branch 'upstream/master' into sunburst-a11y-slices

d31fad8

Merge remote-tracking branch 'origin/sunburst-a11y-slices' into sunbu…

5646771

…rst-a11y-slices

feat: update table pagination

3e5cd36

rshen91 requested a review from markov00 June 8, 2021 17:17

myasonik reviewed Jun 8, 2021

View reviewed changes

src/components/accessibility/partitions_data_table.tsx Outdated Show resolved Hide resolved

src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx Outdated Show resolved Hide resolved

rshen91 added 3 commits June 8, 2021 14:09

fix: code review

b0b02fc

feat: add colSpan

82e8219

Merge remote-tracking branch 'upstream/master' into sunburst-a11y-slices

5dabaea

rshen91 requested a review from myasonik June 8, 2021 20:29

rshen91 added 3 commits June 9, 2021 07:06

fix: merge commit

f929208

Merge remote-tracking branch 'upstream/master' into sunburst-a11y-slices

b019245

Merge remote-tracking branch 'upstream/master' into sunburst-a11y-slices

f98dee7

myasonik approved these changes Jun 9, 2021

View reviewed changes

markov00 approved these changes Jun 10, 2021

View reviewed changes

rshen91 merged commit 87fd75f into elastic:master Jun 10, 2021

nickofthyme added the released Issue released publicly label Jun 10, 2021

markov00 mentioned this pull request Jun 29, 2021

Update dependency @elastic/charts to v31.0.0 (master) elastic/kibana#102078

Merged

1 task

rshen91 deleted the sunburst-a11y-slices branch July 28, 2021 15:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(a11y): add data table for screen readers (sunburst, treemap, icicle, flame) #1155

feat(a11y): add data table for screen readers (sunburst, treemap, icicle, flame) #1155

rshen91 commented May 10, 2021 •

edited

Loading

monfera left a comment •

edited

Loading

monfera May 19, 2021 •

edited

Loading

markov00 May 19, 2021

rshen91 May 19, 2021 •

edited

Loading

rshen91 May 19, 2021

rshen91 May 19, 2021

monfera May 20, 2021

myasonik Jun 1, 2021

myasonik left a comment

markov00 left a comment

nickofthyme commented Jun 10, 2021

feat(a11y): add data table for screen readers (sunburst, treemap, icicle, flame) #1155

feat(a11y): add data table for screen readers (sunburst, treemap, icicle, flame) #1155

Conversation

rshen91 commented May 10, 2021 • edited Loading

Summary

Details:

Checklist

monfera left a comment • edited Loading

Choose a reason for hiding this comment

monfera May 19, 2021 • edited Loading

Choose a reason for hiding this comment

markov00 May 19, 2021

Choose a reason for hiding this comment

rshen91 May 19, 2021 • edited Loading

Choose a reason for hiding this comment

rshen91 May 19, 2021

Choose a reason for hiding this comment

rshen91 May 19, 2021

Choose a reason for hiding this comment

monfera May 20, 2021

Choose a reason for hiding this comment

myasonik Jun 1, 2021

Choose a reason for hiding this comment

myasonik left a comment

Choose a reason for hiding this comment

markov00 left a comment

Choose a reason for hiding this comment

nickofthyme commented Jun 10, 2021

rshen91 commented May 10, 2021 •

edited

Loading

monfera left a comment •

edited

Loading

monfera May 19, 2021 •

edited

Loading

rshen91 May 19, 2021 •

edited

Loading