From 167359d0caea888d0a0f763b2b7140d1142ccaf8 Mon Sep 17 00:00:00 2001 From: srmukher Date: Mon, 9 Jan 2023 17:34:39 +0530 Subject: [PATCH 1/4] Accessibility fix for empty vertical bar chart --- ...-9fc3fab7-e316-48f3-8b7b-1d07cd7c6853.json | 7 +++ .../VerticalBarChart.base.tsx | 47 +++++++++++++++---- 2 files changed, 44 insertions(+), 10 deletions(-) create mode 100644 change/@fluentui-react-charting-9fc3fab7-e316-48f3-8b7b-1d07cd7c6853.json diff --git a/change/@fluentui-react-charting-9fc3fab7-e316-48f3-8b7b-1d07cd7c6853.json b/change/@fluentui-react-charting-9fc3fab7-e316-48f3-8b7b-1d07cd7c6853.json new file mode 100644 index 0000000000000..12192e870743e --- /dev/null +++ b/change/@fluentui-react-charting-9fc3fab7-e316-48f3-8b7b-1d07cd7c6853.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Accessibility bug fix", + "packageName": "@fluentui/react-charting", + "email": "srmukher@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.base.tsx b/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.base.tsx index 6f660ad768145..e516118105c46 100644 --- a/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.base.tsx +++ b/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.base.tsx @@ -69,6 +69,7 @@ export class VerticalBarChartBase extends React.Component 0 ? (getTypeOfAxis(this.props.data![0].x, true) as XAxisTypes) : XAxisTypes.StringAxis; + this._isChartEmpty = false; + } + + public componentDidMount(): void { + const isChartEmpty = + d3Max(this._points, (point: IVerticalBarChartDataPoint) => point.y)! <= 0 && !this._isHavingLine; + if (isChartEmpty || this._isChartEmpty) { + d3Select('body') + .append('div') + .attr('role', 'alert') + .attr('id', 'ariaLabel_VerticalBarChart') + .style('opacity', 0) + .attr('aria-label', 'Graph has no data to display') + .attr('tabIndex', 0); + } } public render(): JSX.Element { @@ -154,12 +170,14 @@ export class VerticalBarChartBase extends React.Component { return ( - <> - {this._bars} - {this._isHavingLine && ( - {this._createLine(props.xScale!, props.yScale!, props.containerHeight, props.containerWidth)} - )} - + !this._isChartEmpty && ( + <> + {this._bars} + {this._isHavingLine && ( + {this._createLine(props.xScale!, props.yScale!, props.containerHeight, props.containerWidth)} + )} + + ) ); }} /> @@ -318,10 +336,14 @@ export class VerticalBarChartBase extends React.Component { - return (this._bars = - this._xAxisType === XAxisTypes.NumericAxis - ? this._createNumericBars(containerHeight, containerWidth, xElement!) - : this._createStringBars(containerHeight, containerWidth, xElement!)); + if (!this._isChartEmpty) { + return (this._bars = + this._xAxisType === XAxisTypes.NumericAxis + ? this._createNumericBars(containerHeight, containerWidth, xElement!) + : this._createStringBars(containerHeight, containerWidth, xElement!)); + } else { + return []; + } }; private _createColors(): D3ScaleLinear | ColorScale { @@ -681,6 +703,11 @@ export class VerticalBarChartBase extends React.Component { if (yAxisData && yAxisData.yAxisDomainValues.length) { const { yAxisDomainValues: domainValue } = yAxisData; + const domainRange = + d3Max(domainValue, (domain: number) => domain)! - d3Min(domainValue, (domain: number) => domain)!; + if (domainRange === 0) { + this._isChartEmpty = true; + } this._yMax = Math.max(domainValue[domainValue.length - 1], this.props.yMaxValue || 0); } }; From c9d5da942d06678182aed96c400d48ab42f23af6 Mon Sep 17 00:00:00 2001 From: srmukher Date: Mon, 6 Feb 2023 13:41:52 +0530 Subject: [PATCH 2/4] Attaching the empty aria label component to the chart --- .../VerticalBarChart.base.tsx | 84 ++++++++++--------- 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.base.tsx b/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.base.tsx index e516118105c46..f4e6d5e748898 100644 --- a/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.base.tsx +++ b/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.base.tsx @@ -70,6 +70,7 @@ export class VerticalBarChartBase extends React.Component point.y)! <= 0 && !this._isHavingLine; - if (isChartEmpty || this._isChartEmpty) { - d3Select('body') + this._isChartEmpty = + this._isChartEmpty || + (d3Max(this._points, (point: IVerticalBarChartDataPoint) => point.y)! <= 0 && !this._isHavingLine); + + if (this._isChartEmpty) { + const emptyVBC = d3Select('#' + this._idVBC); + emptyVBC .append('div') .attr('role', 'alert') - .attr('id', 'ariaLabel_VerticalBarChart') + .attr('id', getId('_emptyVBC_')) .style('opacity', 0) - .attr('aria-label', 'Graph has no data to display') - .attr('tabIndex', 0); + .attr('aria-label', 'Graph has no data to display'); } } @@ -149,38 +153,40 @@ export class VerticalBarChartBase extends React.Component { - return ( - !this._isChartEmpty && ( - <> - {this._bars} - {this._isHavingLine && ( - {this._createLine(props.xScale!, props.yScale!, props.containerHeight, props.containerWidth)} - )} - - ) - ); - }} - /> +
+ { + return ( + !this._isChartEmpty && ( + <> + {this._bars} + {this._isHavingLine && ( + {this._createLine(props.xScale!, props.yScale!, props.containerHeight, props.containerWidth)} + )} + + ) + ); + }} + /> +
); } From d538fcff5c670b13632ccc062f1efeccc4e2ce6c Mon Sep 17 00:00:00 2001 From: srmukher Date: Tue, 7 Feb 2023 16:27:46 +0530 Subject: [PATCH 3/4] Rendering empty aria label based on state and adding respective snapshot tests --- .../VerticalBarChart.base.tsx | 103 +- .../VerticalBarChart.test.tsx | 40 + .../VerticalBarChart.test.tsx.snap | 6612 ++++++++--------- 3 files changed, 3372 insertions(+), 3383 deletions(-) diff --git a/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.base.tsx b/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.base.tsx index 5702a53e9b00d..b46e6366f1e7f 100644 --- a/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.base.tsx +++ b/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.base.tsx @@ -49,6 +49,7 @@ export interface IVerticalBarChartState extends IBasestate { hoverXValue?: string | number | null; callOutAccessibilityData?: IAccessibilityProps; calloutLegend: string; + emptyChart?: boolean; } type ColorScale = (_p?: number) => string; @@ -70,7 +71,6 @@ export class VerticalBarChartBase extends React.Component point.y)! <= 0 && !this._isHavingLine); - - if (this._isChartEmpty) { - const emptyVBC = d3Select('#' + this._idVBC); - emptyVBC - .append('div') - .attr('role', 'alert') - .attr('id', getId('_emptyVBC_')) - .style('opacity', 0) - .attr('aria-label', 'Graph has no data to display'); + if (this.state.emptyChart !== this._isChartEmpty) { + this.setState({ emptyChart: this._isChartEmpty }); } } @@ -152,41 +146,41 @@ export class VerticalBarChartBase extends React.Component - { - return ( - !this._isChartEmpty && ( - <> - {this._bars} - {this._isHavingLine && ( - {this._createLine(props.xScale!, props.yScale!, props.containerHeight, props.containerWidth)} - )} - - ) - ); - }} - /> - + return !this.state.emptyChart ? ( + { + return ( + !this._isChartEmpty && ( + <> + {this._bars} + {this._isHavingLine && ( + {this._createLine(props.xScale!, props.yScale!, props.containerHeight, props.containerWidth)} + )} + + ) + ); + }} + /> + ) : ( +
); } @@ -342,14 +336,10 @@ export class VerticalBarChartBase extends React.Component { - if (!this._isChartEmpty) { - return (this._bars = - this._xAxisType === XAxisTypes.NumericAxis - ? this._createNumericBars(containerHeight, containerWidth, xElement!) - : this._createStringBars(containerHeight, containerWidth, xElement!)); - } else { - return []; - } + return (this._bars = + this._xAxisType === XAxisTypes.NumericAxis + ? this._createNumericBars(containerHeight, containerWidth, xElement!) + : this._createStringBars(containerHeight, containerWidth, xElement!)); }; private _createColors(): D3ScaleLinear | ColorScale { @@ -711,11 +701,6 @@ export class VerticalBarChartBase extends React.Component { if (yAxisData && yAxisData.yAxisDomainValues.length) { const { yAxisDomainValues: domainValue } = yAxisData; - const domainRange = - d3Max(domainValue, (domain: number) => domain)! - d3Min(domainValue, (domain: number) => domain)!; - if (domainRange === 0) { - this._isChartEmpty = true; - } this._yMax = Math.max(domainValue[domainValue.length - 1], this.props.yMaxValue || 0); } }; diff --git a/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.test.tsx b/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.test.tsx index 032471acff81b..3c943ef3d61e8 100644 --- a/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.test.tsx +++ b/packages/react-charting/src/components/VerticalBarChart/VerticalBarChart.test.tsx @@ -227,3 +227,43 @@ describe('VerticalBarChart - mouse events', () => { expect(tree).toMatchSnapshot(); }); }); + +describe('Render empty chart aria label div when chart is empty', () => { + it('No empty chart aria label div rendered', () => { + wrapper = mount( + , + ); + const renderedDOM = wrapper.findWhere(node => node.prop('aria-label') === 'Graph has no data to display'); + expect(renderedDOM!.length).toBe(0); + }); + + it('Empty chart aria label div rendered', () => { + wrapper = mount(); + const renderedDOM = wrapper.findWhere(node => node.prop('aria-label') === 'Graph has no data to display'); + expect(renderedDOM!.length).toBe(1); + }); +}); + +describe('Render empty chart calling with respective to props', () => { + it('No prop changes', () => { + const renderMock = jest.spyOn(VerticalBarChartBase.prototype, 'render'); + const props = { + data: chartPoints, + }; + const component = mount(); + component.setProps({ ...props }); + expect(renderMock).toHaveBeenCalledTimes(2); + renderMock.mockRestore(); + }); + + it('Prop changes', () => { + const renderMock = jest.spyOn(VerticalBarChartBase.prototype, 'render'); + const props = { + data: [], + }; + const component = mount(); + component.setProps({ ...props }); + expect(renderMock).toHaveBeenCalledTimes(3); + renderMock.mockRestore(); + }); +}); diff --git a/packages/react-charting/src/components/VerticalBarChart/__snapshots__/VerticalBarChart.test.tsx.snap b/packages/react-charting/src/components/VerticalBarChart/__snapshots__/VerticalBarChart.test.tsx.snap index dc9b044c5a098..12b76532effc7 100644 --- a/packages/react-charting/src/components/VerticalBarChart/__snapshots__/VerticalBarChart.test.tsx.snap +++ b/packages/react-charting/src/components/VerticalBarChart/__snapshots__/VerticalBarChart.test.tsx.snap @@ -2,578 +2,612 @@ exports[`VerticalBarChart - mouse events Should render callout correctly on mouseover 1`] = ` +