Skip to content

Commit

Permalink
feat: add code review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
rshen91 committed Apr 6, 2021
1 parent d091a14 commit e7d1230
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 43 deletions.
31 changes: 13 additions & 18 deletions .playground/playground.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<div className="App">
<Chart size={[500, 200]}>
<LineAnnotation
domainType={AnnotationDomainTypes.XDomain}
id="ann"
dataValues={[{ dataValue: 'bags' }]}
marker={<div style={{ background: 'red' }}>hello</div>}
// markerPosition="top"
/>
<LineAnnotation
domainType={AnnotationDomainTypes.YDomain}
id="ann1"
dataValues={generateAnnotationData([30])}
marker={<div style={{ background: 'yellow' }}>Horizontal</div>}
// markerPosition="right"
/>
{/* <AreaSeries id="lines" name="test2" data={[
{ x: 'trousers', y: 300, val: 1232 },
{ x: 'watches', y: 20, val: 1232 },
{ x: 'bags', y: 700, val: 1232 },
{ x: 'cocktail dresses', y: 804, val: 1232 },
]}/>
<LineSeries id="lines2" name="test" data={[
{ x: 'trousers', y: 300, val: 1232 },
{ x: 'watches', y: 20, val: 1232 },
{ x: 'bags', y: 700, val: 1232 },
{ x: 'cocktail dresses', y: 804, val: 1232 },
]}/> */}
<BarSeries
id="bars"
name="amount"
Expand Down
8 changes: 4 additions & 4 deletions integration/tests/accessibility.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 === '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(
Expand All @@ -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');
});
});
29 changes: 29 additions & 0 deletions src/chart_types/xy_chart/renderer/canvas/utils/types.ts
Original file line number Diff line number Diff line change
@@ -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<string, string> = 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';
};
27 changes: 13 additions & 14 deletions src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -162,11 +163,18 @@ class XYChartComponent extends React.Component<XYChartProps> {

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 (
<figure>
<canvas
Expand All @@ -178,21 +186,12 @@ class XYChartComponent extends React.Component<XYChartProps> {
width,
height,
}}
aria-label="Chart"
// eslint-disable-next-line jsx-a11y/no-interactive-element-to-noninteractive-role
role="presentation"
>
<dl className="screen-reader">
{seriesTypes.length > 0 ? (
<dt>
Chart type(s)
{seriesTypes.map((value, index) => {
return <dd key={`${index}`}>{value}</dd>;
})}
</dt>
) : (
<dt>No series in chart</dt>
)}
<dl className="echScreen-reader">
<dt>Chart type</dt>
<dd>{chartSeriesTypes}</dd>
</dl>
</canvas>
</figure>
Expand Down
3 changes: 2 additions & 1 deletion src/chart_types/xy_chart/renderer/dom/_screen_reader.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
.screen-reader {
.echScreen-reader {
position: absolute;
left: -10000px;
top: auto;
width: 1px;
height: 1px;
overflow: hidden;
}
12 changes: 6 additions & 6 deletions src/components/__snapshots__/chart.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,14 @@ exports[`Chart should render the legend name test 1`] = `
<Connect(XYChart) forwardStageRef={{...}}>
<XYChart forwardStageRef={{...}} initialized={true} isChartEmpty={false} debug={true} geometries={{...}} geometriesIndex={{...}} theme={{...}} chartContainerDimensions={{...}} highlightedLegendItem={[undefined]} rotation={0} renderingArea={{...}} chartTransform={{...}} axesSpecs={{...}} perPanelAxisGeoms={{...}} perPanelGridLines={{...}} axesStyles={{...}} annotationDimensions={{...}} annotationSpecs={{...}} panelGeoms={{...}} onChartRendered={[Function (anonymous)]}>
<figure>
<canvas className=\\"echCanvasRenderer\\" width={150} height={200} style={{...}} aria-label=\\"Chart\\" role=\\"presentation\\">
<dl className=\\"screen-reader\\">
<canvas className=\\"echCanvasRenderer\\" width={150} height={200} style={{...}} role=\\"presentation\\">
<dl className=\\"echScreen-reader\\">
<dt>
Chart type(s)
<dd>
bars
</dd>
Chart type
</dt>
<dd>
Bar chart
</dd>
</dl>
</canvas>
</figure>
Expand Down

0 comments on commit e7d1230

Please sign in to comment.