From 3558657226550961c39bf1d88c09c39bca6c4c53 Mon Sep 17 00:00:00 2001 From: rshen91 Date: Thu, 1 Apr 2021 10:57:40 -0600 Subject: [PATCH 1/7] feat: a11y chart figure --- .../xy_chart/renderer/canvas/xy_chart.tsx | 43 +++++++++++++------ .../xy_chart/renderer/dom/_index.scss | 1 + .../xy_chart/renderer/dom/_screen_reader.scss | 7 +++ .../__snapshots__/chart.test.tsx.snap | 13 +++++- 4 files changed, 50 insertions(+), 14 deletions(-) create mode 100644 src/chart_types/xy_chart/renderer/dom/_screen_reader.scss diff --git a/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx b/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx index 94be7d259e..fe8d044296 100644 --- a/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx +++ b/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx @@ -152,6 +152,7 @@ class XYChartComponent extends React.Component { initialized, isChartEmpty, chartContainerDimensions: { width, height }, + geometries, } = this.props; if (!initialized || isChartEmpty) { @@ -159,20 +160,36 @@ class XYChartComponent extends React.Component { return null; } + const seriesTypes: string[] = []; + Object.entries(geometries).forEach((value) => { + if (value[1].length > 0) { + seriesTypes.push(value[0]); + } + }); + return ( - +
+ +
+
Chart type(s)
+ {seriesTypes.map((value, index) => { + return
{value}
; + })} +
+ +
); } } diff --git a/src/chart_types/xy_chart/renderer/dom/_index.scss b/src/chart_types/xy_chart/renderer/dom/_index.scss index 548fb3f7de..df6c1c3101 100644 --- a/src/chart_types/xy_chart/renderer/dom/_index.scss +++ b/src/chart_types/xy_chart/renderer/dom/_index.scss @@ -1,3 +1,4 @@ @import 'highlighter'; @import 'crosshair'; +@import 'screen_reader'; @import 'annotations/index'; diff --git a/src/chart_types/xy_chart/renderer/dom/_screen_reader.scss b/src/chart_types/xy_chart/renderer/dom/_screen_reader.scss new file mode 100644 index 0000000000..f75ee6409b --- /dev/null +++ b/src/chart_types/xy_chart/renderer/dom/_screen_reader.scss @@ -0,0 +1,7 @@ +.screen-reader { + position: absolute; + left: -10000px; + top: auto; + width: 1px; + height: 1px; +} diff --git a/src/components/__snapshots__/chart.test.tsx.snap b/src/components/__snapshots__/chart.test.tsx.snap index 496feecdee..ee58029cf0 100644 --- a/src/components/__snapshots__/chart.test.tsx.snap +++ b/src/components/__snapshots__/chart.test.tsx.snap @@ -73,7 +73,18 @@ exports[`Chart should render the legend name test 1`] = ` - +
+ +
+
+ Chart type(s) +
+
+ bars +
+
+ +
From 7e499b953ab5f2776d06d24723397914f75ee1cb Mon Sep 17 00:00:00 2001 From: rshen91 Date: Thu, 1 Apr 2021 15:19:45 -0600 Subject: [PATCH 2/7] test: add a11y tests --- integration/tests/accessibility.test.ts | 23 ++++++++++++++----- .../xy_chart/renderer/canvas/xy_chart.tsx | 14 +++++++---- .../__snapshots__/chart.test.tsx.snap | 6 ++--- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/integration/tests/accessibility.test.ts b/integration/tests/accessibility.test.ts index 4173895c30..332d7e235d 100644 --- a/integration/tests/accessibility.test.ts +++ b/integration/tests/accessibility.test.ts @@ -20,15 +20,26 @@ import { common } from '../page_objects/common'; describe('Accessibility tree', () => { - it('should show the aria-label for the canvas element in the accessibility tree', async () => { + it('should include the series types if one type of series', async () => { const tree = await common.testAccessibilityTree( 'http://localhost:9001/iframe.html?id=annotations-lines--x-continuous-domain', - '#story-root', + '.echCanvasRenderer', ); - // digging into the accessibility tree for the canvas element - const expectedAriaLabel = tree.children.filter((value) => { - return value.name === 'Chart'; + // the legend has bars and lines as value.descriptions not value.name + const hasTextOfChartTypes = tree.children.filter((value) => { + return value.name === 'bars'; }); - expect(expectedAriaLabel[0].name).toBe('Chart'); + expect(hasTextOfChartTypes.length).toBe(1); + }); + it('should include the series types if multiple types of series', async () => { + const tree = await common.testAccessibilityTree( + 'http://localhost:9001/iframe.html?id=mixed-charts--bars-and-lines', + '.echCanvasRenderer', + ); + // the legend has bars and lines as value.descriptions not value.name + const hasTextOfChartTypes = tree.children.filter((value) => { + return value.name === 'bars' || value.name === 'lines'; + }); + expect(hasTextOfChartTypes.length).toBe(2); }); }); diff --git a/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx b/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx index fe8d044296..b4da9cce59 100644 --- a/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx +++ b/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx @@ -183,10 +183,16 @@ class XYChartComponent extends React.Component { role="presentation" >
-
Chart type(s)
- {seriesTypes.map((value, index) => { - return
{value}
; - })} + {seriesTypes.length > 0 ? ( +
+ Chart type(s) + {seriesTypes.map((value, index) => { + return
{value}
; + })} + + ) : ( +
No series in chart
+ )}
diff --git a/src/components/__snapshots__/chart.test.tsx.snap b/src/components/__snapshots__/chart.test.tsx.snap index ee58029cf0..b05cbf61d1 100644 --- a/src/components/__snapshots__/chart.test.tsx.snap +++ b/src/components/__snapshots__/chart.test.tsx.snap @@ -78,10 +78,10 @@ exports[`Chart should render the legend name test 1`] = `
Chart type(s) +
+ bars +
-
- bars -
From e7d123043f6fb6d2aeb5e5c9f8227ae2882bd3de Mon Sep 17 00:00:00 2001 From: rshen91 Date: Tue, 6 Apr 2021 16:18:23 -0600 Subject: [PATCH 3/7] feat: add code review changes --- .playground/playground.tsx | 31 ++++++++----------- integration/tests/accessibility.test.ts | 8 ++--- .../xy_chart/renderer/canvas/utils/types.ts | 29 +++++++++++++++++ .../xy_chart/renderer/canvas/xy_chart.tsx | 27 ++++++++-------- .../xy_chart/renderer/dom/_screen_reader.scss | 3 +- .../__snapshots__/chart.test.tsx.snap | 12 +++---- 6 files changed, 67 insertions(+), 43 deletions(-) create mode 100644 src/chart_types/xy_chart/renderer/canvas/utils/types.ts diff --git a/.playground/playground.tsx b/.playground/playground.tsx index 5332b367ea..e20a53cb0b 100644 --- a/.playground/playground.tsx +++ b/.playground/playground.tsx @@ -19,30 +19,25 @@ import React from 'react'; -import { Chart, BarSeries, ScaleType, LineAnnotation, AnnotationDomainTypes, LineAnnotationDatum } from '../src'; +import { Chart, BarSeries, ScaleType } from '../src'; -function generateAnnotationData(values: any[]): LineAnnotationDatum[] { - return values.map((value, index) => ({ dataValue: value, details: `detail-${index}` })); -} export class Playground extends React.Component { render() { return (
- hello
} - // markerPosition="top" - /> - Horizontal} - // markerPosition="right" - /> + {/* + */} { ); // the legend has bars and lines as value.descriptions not value.name const hasTextOfChartTypes = tree.children.filter((value) => { - return value.name === 'bars'; + return value.name === 'Bar chart'; }); - expect(hasTextOfChartTypes.length).toBe(1); + expect(hasTextOfChartTypes[0].name).toBe('Bar chart'); }); it('should include the series types if multiple types of series', async () => { const tree = await common.testAccessibilityTree( @@ -38,8 +38,8 @@ describe('Accessibility tree', () => { ); // the legend has bars and lines as value.descriptions not value.name const hasTextOfChartTypes = tree.children.filter((value) => { - return value.name === 'bars' || value.name === 'lines'; + return value.name === 'Mixed chart: Bar chart and Line chart'; }); - expect(hasTextOfChartTypes.length).toBe(2); + expect(hasTextOfChartTypes[0].name).toBe('Mixed chart: Bar chart and Line chart'); }); }); diff --git a/src/chart_types/xy_chart/renderer/canvas/utils/types.ts b/src/chart_types/xy_chart/renderer/canvas/utils/types.ts new file mode 100644 index 0000000000..1ffb61651d --- /dev/null +++ b/src/chart_types/xy_chart/renderer/canvas/utils/types.ts @@ -0,0 +1,29 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** @internal */ +export const getNameFunction = (key: string): string => { + const screenReader: Map = new Map(); + screenReader + .set('bars', 'Bar chart') + .set('areas', 'Area chart') + .set('lines', 'Line chart') + .set('bubbles', 'Bubble chart'); + return screenReader.get(key) ?? 'unknown chart'; +}; diff --git a/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx b/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx index b4da9cce59..f3dacb848e 100644 --- a/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx +++ b/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx @@ -55,6 +55,7 @@ import { LinesGrid } from '../../utils/grid_lines'; import { IndexedGeometryMap } from '../../utils/indexed_geometry_map'; import { AxisSpec, AnnotationSpec } from '../../utils/specs'; import { renderXYChartCanvas2d } from './renderers'; +import { getNameFunction } from './utils/types'; /** @internal */ export interface ReactiveChartStateProps { @@ -162,11 +163,18 @@ class XYChartComponent extends React.Component { const seriesTypes: string[] = []; Object.entries(geometries).forEach((value) => { - if (value[1].length > 0) { - seriesTypes.push(value[0]); + if (value[1].length > 0 && value[0]) { + seriesTypes.push(getNameFunction(value[0])); } }); + const series: string[] = []; + seriesTypes.map((value: string, index: number) => { + return index < seriesTypes.length - 1 ? series.push(value, ' and') : series.push(value); + }); + + const chartSeriesTypes = seriesTypes.length > 1 ? `Mixed chart: ${series.join(' ')}` : `${seriesTypes[0]}`; + return (
{ width, height, }} - aria-label="Chart" // eslint-disable-next-line jsx-a11y/no-interactive-element-to-noninteractive-role role="presentation" > -
- {seriesTypes.length > 0 ? ( -
- Chart type(s) - {seriesTypes.map((value, index) => { - return
{value}
; - })} - - ) : ( -
No series in chart
- )} +
+
Chart type
+
{chartSeriesTypes}
diff --git a/src/chart_types/xy_chart/renderer/dom/_screen_reader.scss b/src/chart_types/xy_chart/renderer/dom/_screen_reader.scss index f75ee6409b..bad8459562 100644 --- a/src/chart_types/xy_chart/renderer/dom/_screen_reader.scss +++ b/src/chart_types/xy_chart/renderer/dom/_screen_reader.scss @@ -1,7 +1,8 @@ -.screen-reader { +.echScreen-reader { position: absolute; left: -10000px; top: auto; width: 1px; height: 1px; + overflow: hidden; } diff --git a/src/components/__snapshots__/chart.test.tsx.snap b/src/components/__snapshots__/chart.test.tsx.snap index b05cbf61d1..5724b466a9 100644 --- a/src/components/__snapshots__/chart.test.tsx.snap +++ b/src/components/__snapshots__/chart.test.tsx.snap @@ -74,14 +74,14 @@ exports[`Chart should render the legend name test 1`] = `
- -
+ +
- Chart type(s) -
- bars -
+ Chart type +
+ Bar chart +
From 01a46af381bca81817a589d44cacd00f6223e7bb Mon Sep 17 00:00:00 2001 From: rshen91 Date: Tue, 6 Apr 2021 16:31:08 -0600 Subject: [PATCH 4/7] style: add spaces in the
tags for voiceover readability --- src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx | 2 +- src/components/__snapshots__/chart.test.tsx.snap | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx b/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx index f3dacb848e..4c9a80f42c 100644 --- a/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx +++ b/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx @@ -190,7 +190,7 @@ class XYChartComponent extends React.Component { role="presentation" >
-
Chart type
+
Chart type
{chartSeriesTypes}
diff --git a/src/components/__snapshots__/chart.test.tsx.snap b/src/components/__snapshots__/chart.test.tsx.snap index 5724b466a9..5f7ccc1fad 100644 --- a/src/components/__snapshots__/chart.test.tsx.snap +++ b/src/components/__snapshots__/chart.test.tsx.snap @@ -77,7 +77,7 @@ exports[`Chart should render the legend name test 1`] = `
- Chart type + Chart type
Bar chart From 9e036d3b85387a8745500b962318510642cfa956 Mon Sep 17 00:00:00 2001 From: rshen91 Date: Wed, 7 Apr 2021 15:08:10 -0600 Subject: [PATCH 5/7] refactor: add selector --- .playground/playground.tsx | 18 +++++++++---- .../xy_chart/renderer/canvas/xy_chart.tsx | 27 +++++++------------ .../selectors/get_series_types.ts} | 23 +++++++++------- .../__snapshots__/chart.test.tsx.snap | 4 +-- 4 files changed, 39 insertions(+), 33 deletions(-) rename src/chart_types/xy_chart/{renderer/canvas/utils/types.ts => state/selectors/get_series_types.ts} (61%) diff --git a/.playground/playground.tsx b/.playground/playground.tsx index e20a53cb0b..c163367dd2 100644 --- a/.playground/playground.tsx +++ b/.playground/playground.tsx @@ -19,25 +19,33 @@ import React from 'react'; -import { Chart, BarSeries, ScaleType } from '../src'; +import { Chart, AreaSeries, LineSeries, BarSeries, ScaleType } from '../src'; export class Playground extends React.Component { render() { return (
- {/* - + */} + ]} + /> ; annotationSpecs: AnnotationSpec[]; panelGeoms: PanelGeoms; + seriesTypes: Set; } interface ReactiveChartDispatchProps { @@ -153,27 +154,17 @@ class XYChartComponent extends React.Component { initialized, isChartEmpty, chartContainerDimensions: { width, height }, - geometries, + seriesTypes, } = this.props; if (!initialized || isChartEmpty) { this.ctx = null; return null; } - - const seriesTypes: string[] = []; - Object.entries(geometries).forEach((value) => { - if (value[1].length > 0 && value[0]) { - seriesTypes.push(getNameFunction(value[0])); - } - }); - - const series: string[] = []; - seriesTypes.map((value: string, index: number) => { - return index < seriesTypes.length - 1 ? series.push(value, ' and') : series.push(value); - }); - - const chartSeriesTypes = seriesTypes.length > 1 ? `Mixed chart: ${series.join(' ')}` : `${seriesTypes[0]}`; + const multipleSeriesTypes: string[] = []; + seriesTypes.forEach((value) => multipleSeriesTypes.push(value)); + const chartSeriesTypes = + seriesTypes.size > 1 ? `Mixed chart: ${[...seriesTypes].join(' and ')} chart` : `${[...seriesTypes]} chart`; return (
@@ -246,6 +237,7 @@ const DEFAULT_PROPS: ReactiveChartStateProps = { annotationDimensions: new Map(), annotationSpecs: [], panelGeoms: [], + seriesTypes: new Set(), }; const mapStateToProps = (state: GlobalChartState): ReactiveChartStateProps => { @@ -274,6 +266,7 @@ const mapStateToProps = (state: GlobalChartState): ReactiveChartStateProps => { annotationDimensions: computeAnnotationDimensionsSelector(state), annotationSpecs: getAnnotationSpecsSelector(state), panelGeoms: computePanelsSelectors(state), + seriesTypes: getSeriesTypes(state), }; }; diff --git a/src/chart_types/xy_chart/renderer/canvas/utils/types.ts b/src/chart_types/xy_chart/state/selectors/get_series_types.ts similarity index 61% rename from src/chart_types/xy_chart/renderer/canvas/utils/types.ts rename to src/chart_types/xy_chart/state/selectors/get_series_types.ts index 1ffb61651d..e6ce67d61c 100644 --- a/src/chart_types/xy_chart/renderer/canvas/utils/types.ts +++ b/src/chart_types/xy_chart/state/selectors/get_series_types.ts @@ -17,13 +17,18 @@ * under the License. */ +import createCachedSelector from 're-reselect'; + +import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; +import { SeriesType } from '../../utils/specs'; +import { getSeriesSpecsSelector } from './get_specs'; + /** @internal */ -export const getNameFunction = (key: string): string => { - const screenReader: Map = new Map(); - screenReader - .set('bars', 'Bar chart') - .set('areas', 'Area chart') - .set('lines', 'Line chart') - .set('bubbles', 'Bubble chart'); - return screenReader.get(key) ?? 'unknown chart'; -}; +export const getSeriesTypes = createCachedSelector( + [getSeriesSpecsSelector], + (specs): Set => { + const seriesTypes = new Set(); + specs.forEach((value) => seriesTypes.add(value.seriesType)); + return seriesTypes; + }, +)(getChartIdSelector); diff --git a/src/components/__snapshots__/chart.test.tsx.snap b/src/components/__snapshots__/chart.test.tsx.snap index 5f7ccc1fad..b473c82edb 100644 --- a/src/components/__snapshots__/chart.test.tsx.snap +++ b/src/components/__snapshots__/chart.test.tsx.snap @@ -72,7 +72,7 @@ exports[`Chart should render the legend name test 1`] = ` - +
@@ -80,7 +80,7 @@ exports[`Chart should render the legend name test 1`] = ` Chart type
- Bar chart + bar chart
From bf0e01322caaf51612241d8b772be3e0c1d44702 Mon Sep 17 00:00:00 2001 From: rshen91 Date: Wed, 7 Apr 2021 15:57:55 -0600 Subject: [PATCH 6/7] test: update tests --- integration/tests/accessibility.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/integration/tests/accessibility.test.ts b/integration/tests/accessibility.test.ts index 21dbaa2097..b832bd184f 100644 --- a/integration/tests/accessibility.test.ts +++ b/integration/tests/accessibility.test.ts @@ -27,9 +27,9 @@ describe('Accessibility tree', () => { ); // the legend has bars and lines as value.descriptions not value.name const hasTextOfChartTypes = tree.children.filter((value) => { - return value.name === 'Bar chart'; + return value.name === 'bar chart'; }); - expect(hasTextOfChartTypes[0].name).toBe('Bar chart'); + expect(hasTextOfChartTypes[0].name).toBe('bar chart'); }); it('should include the series types if multiple types of series', async () => { const tree = await common.testAccessibilityTree( @@ -38,8 +38,8 @@ describe('Accessibility tree', () => { ); // the legend has bars and lines as value.descriptions not value.name const hasTextOfChartTypes = tree.children.filter((value) => { - return value.name === 'Mixed chart: Bar chart and Line chart'; + return value.name === 'Mixed chart: bar and line chart'; }); - expect(hasTextOfChartTypes[0].name).toBe('Mixed chart: Bar chart and Line chart'); + expect(hasTextOfChartTypes[0].name).toBe('Mixed chart: bar and line chart'); }); }); From 9b11e8501646658d41ed78378a5015ac8459c444 Mon Sep 17 00:00:00 2001 From: rshen91 Date: Mon, 12 Apr 2021 08:14:46 -0600 Subject: [PATCH 7/7] fix: add code review feedback --- src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx | 5 ++--- src/chart_types/xy_chart/renderer/dom/_screen_reader.scss | 2 +- src/components/__snapshots__/chart.test.tsx.snap | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx b/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx index 743ae49b19..40a174a841 100644 --- a/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx +++ b/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx @@ -161,8 +161,7 @@ class XYChartComponent extends React.Component { this.ctx = null; return null; } - const multipleSeriesTypes: string[] = []; - seriesTypes.forEach((value) => multipleSeriesTypes.push(value)); + const chartSeriesTypes = seriesTypes.size > 1 ? `Mixed chart: ${[...seriesTypes].join(' and ')} chart` : `${[...seriesTypes]} chart`; @@ -181,7 +180,7 @@ class XYChartComponent extends React.Component { role="presentation" >
-
Chart type
+
Chart type
{chartSeriesTypes}
diff --git a/src/chart_types/xy_chart/renderer/dom/_screen_reader.scss b/src/chart_types/xy_chart/renderer/dom/_screen_reader.scss index bad8459562..0bd8bafaf9 100644 --- a/src/chart_types/xy_chart/renderer/dom/_screen_reader.scss +++ b/src/chart_types/xy_chart/renderer/dom/_screen_reader.scss @@ -1,4 +1,4 @@ -.echScreen-reader { +.echScreenReaderOnly { position: absolute; left: -10000px; top: auto; diff --git a/src/components/__snapshots__/chart.test.tsx.snap b/src/components/__snapshots__/chart.test.tsx.snap index b473c82edb..6c1e2f571a 100644 --- a/src/components/__snapshots__/chart.test.tsx.snap +++ b/src/components/__snapshots__/chart.test.tsx.snap @@ -77,7 +77,7 @@ exports[`Chart should render the legend name test 1`] = `
- Chart type + Chart type
bar chart