diff --git a/change/@fluentui-react-charting-7ed503a2-d9d1-4679-8074-c67eb83161e1.json b/change/@fluentui-react-charting-7ed503a2-d9d1-4679-8074-c67eb83161e1.json new file mode 100644 index 00000000000000..2695291386c7f0 --- /dev/null +++ b/change/@fluentui-react-charting-7ed503a2-d9d1-4679-8074-c67eb83161e1.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fixed accessibility issues in pie chart", + "packageName": "@fluentui/react-charting", + "email": "yushsingla@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.styles.ts b/packages/react-charting/src/components/PieChart/Arc/Arc.styles.ts index 33a1f8e766333a..845797bf73a612 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.styles.ts +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.styles.ts @@ -1,8 +1,11 @@ import { IArcProps, IArcStyles } from './Arc.types'; -import { DefaultPalette } from '@fluentui/react/lib/Styling'; -export const getStyles = (props: IArcProps): IArcStyles => { +import { DefaultPalette, ITheme } from '@fluentui/react/lib/Styling'; +export const getStyles = (props: IArcProps, theme: ITheme | undefined): IArcStyles => { const { color } = props; return { - root: { fill: color, stroke: DefaultPalette.white, strokeWidth: 2 }, + arcRoot: { fill: color }, + arcRootFocussed: { fill: color, stroke: theme?.palette.black || DefaultPalette.black, strokeWidth: 2 }, + arc: { outline: 'none' }, + arcText: { fill: theme?.palette.black || DefaultPalette.black, outline: 'none' }, }; }; diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx index 84d216979f829b..0e590743d79225 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -1,46 +1,112 @@ import * as React from 'react'; import * as shape from 'd3-shape'; -import { IArcProps, IArcStyles } from './Arc.types'; -import { classNamesFunction } from '@fluentui/react/lib/Utilities'; +import { IArcProps, IArcState, IArcStyles } from './Arc.types'; +import { classNamesFunction, getId, getRTL } from '@fluentui/react/lib/Utilities'; import { getStyles } from './Arc.styles'; -import { convertToLocaleString } from '../../../utilities/utilities'; +import { wrapContent, convertToLocaleString } from '../../../utilities/utilities'; +import { SVGTooltipText } from '../../../utilities/SVGTooltipText'; -export class Arc extends React.Component { +export class Arc extends React.Component { public static defaultProps: Partial = { arc: shape.arc(), }; - public state: {} = {}; + protected _arcId: string; public static getDerivedStateFromProps(nextProps: Readonly): null { _updateChart(nextProps); return null; } + public constructor(props: IArcProps) { + super(props); + this.state = { + isArcFocused: false, + }; + + this._arcId = getId('piechart_arc'); + } + public updateChart = (newProps: IArcProps) => { _updateChart(newProps); }; public render(): JSX.Element { - const { color, arc } = this.props; + const { arc } = this.props; const getClassNames = classNamesFunction(); - const classNames = getClassNames(getStyles, { color }); - return ; + const classNames = getClassNames(props => getStyles(props, this.props.theme), { ...this.props }); + + return ( + + ); } + + protected _onFocus = () => { + this.setState({ isArcFocused: true }); + }; + + protected _onBlur = () => { + this.setState({ isArcFocused: false }); + }; } export class LabeledArc extends Arc { + private _isRTL = getRTL(); + + public constructor(props: IArcProps) { + super(props); + this._arcId = getId('piechart_arc'); + } + public render(): JSX.Element { - const { data, arc, culture } = this.props; - const [labelX, labelY] = arc.centroid(data); - const labelTranslate = `translate(${labelX}, ${labelY})`; + const { data, culture } = this.props; + const gap = 4; + // placing the labels on the outside arc + const [labelX, labelY] = shape.arc().centroid({ + endAngle: data?.endAngle || 0, + startAngle: data?.startAngle || 0, + padAngle: data?.padAngle, + innerRadius: this.props?.outerRadius || 0, + outerRadius: this.props?.outerRadius || 0 + gap, + }); + + const getClassNames = classNamesFunction(); + const classNames = getClassNames(props => getStyles(props, this.props.theme)); + + const angle = ((data?.startAngle || 0) + (data?.endAngle || 0)) / 2; + + const content = `${data?.data.x}-${convertToLocaleString(data?.data.y, culture)}`; return ( - + {super.render()} - - {data!.data!.x}-{convertToLocaleString(data!.data!.y, culture)} - + Math.PI / 2 && angle < (3 * Math.PI) / 2 ? 'hanging' : 'auto', + textAnchor: (!this._isRTL && angle > Math.PI) || (this._isRTL && angle < Math.PI) ? 'end' : 'start', + 'aria-label': `${data?.data.x}-${convertToLocaleString(data?.data.y, culture)}`, + className: classNames.arcText, + }} + isTooltipVisibleProp={this.state.isArcFocused} + shouldReceiveFocus={false} + maxWidth={40} + wrapContent={wrapContent} + /> ); } diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.types.ts b/packages/react-charting/src/components/PieChart/Arc/Arc.types.ts index 68dafd822f215c..7064f9e17a45ef 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.types.ts +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.types.ts @@ -1,4 +1,4 @@ -import { IStyle } from '@fluentui/react/lib/Styling'; +import { IStyle, ITheme } from '@fluentui/react/lib/Styling'; import { IDataPoint } from '../PieChart.types'; export interface IArcProps { @@ -30,6 +30,17 @@ export interface IArcProps { * The prop used to define the culture to localized the numbers */ culture?: string; + /** + * to pass the theme + */ + theme?: ITheme; +} + +export interface IArcState { + /** + * The state controls, whether the arc needs to be focused or not + */ + isArcFocused?: boolean; } export interface IArcData { @@ -63,5 +74,11 @@ export interface IArcStyles { /** * Style set for the card header component root */ - root: IStyle; + arcRoot: IStyle; + + arc: IStyle; + + arcRootFocussed: IStyle; + + arcText: IStyle; } diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx index 722464fe38a465..3f694880a24a5e 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -4,11 +4,14 @@ import * as scale from 'd3-scale'; import { IPieProps } from './Pie.types'; import { LabeledArc } from '../Arc/Arc'; import { IArcData } from '../Arc/Arc.types'; +import { FocusZone, FocusZoneDirection } from '@fluentui/react-focus'; +import { getColorFromToken, getNextColor } from '../../../utilities/colors'; export class Pie extends React.Component { public static defaultProps: Partial = { pie: shape .pie() + .padAngle(0.01) .sort(null) /* eslint-disable @typescript-eslint/no-explicit-any */ .value((d: any) => d.y), @@ -28,23 +31,36 @@ export class Pie extends React.Component { innerRadius={this.props.innerRadius} outerRadius={this.props.outerRadius} color={`${this.colors(i)}`} + theme={this.props.theme} /> ); }; public render(): JSX.Element { // const getClassNames = classNamesFunction(); - const { pie, colors, data, width, height, chartTitle } = this.props; + const { pie, data, width, height, chartTitle, theme } = this.props; - this.colors = scale.scaleOrdinal().range(colors!); + const defaultColors: Array = []; + if (data && !this.props.colors) { + for (let i = 0; i < data.length; i++) { + defaultColors.push(getNextColor(i, 0, theme?.isInverted)); + } + } + const { colors = defaultColors } = this.props; + + this.colors = scale.scaleOrdinal().range(colors.map(color => getColorFromToken(color))); const piechart = pie(data); const translate = `translate(${width / 2}, ${height / 2})`; return ( - - {piechart.map((d: IArcData, i: number) => this.arcGenerator(d, i))} - + + + + {piechart.map((d: IArcData, i: number) => this.arcGenerator(d, i))} + + + ); } } diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.types.ts b/packages/react-charting/src/components/PieChart/Pie/Pie.types.ts index 4a6ca1140cfbdb..e71a3fec1ec5d0 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.types.ts +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.types.ts @@ -1,4 +1,4 @@ -import { IStyle } from '@fluentui/react/lib/Styling'; +import { IStyle, ITheme } from '@fluentui/react/lib/Styling'; import { IDataPoint } from '../PieChart.types'; export interface IPieProps { @@ -40,6 +40,8 @@ export interface IPieProps { * The prop used to define the culture to localized the numbers */ culture?: string; + + theme?: ITheme; } export interface IPieStyles { diff --git a/packages/react-charting/src/components/PieChart/PieChart.base.tsx b/packages/react-charting/src/components/PieChart/PieChart.base.tsx index da671491182bd7..74ead8471f7294 100644 --- a/packages/react-charting/src/components/PieChart/PieChart.base.tsx +++ b/packages/react-charting/src/components/PieChart/PieChart.base.tsx @@ -30,8 +30,17 @@ export class PieChartBase extends React.Component { height: height!, className, }); - const radius = Math.min(width!, height!) / 2; - const outerRadius = radius - 10; + + const TEXT_MAX_WIDTH = 40; + const TEXT_LINE_HEIGHT = 16; + + /** + * The radius for the pie chart is computed based on the space available inside the svg + * after subtracting the max amount of space that can be used by the text in pie chart + */ + + const radius = Math.min(width! - 2 * TEXT_MAX_WIDTH, height! - 2 * TEXT_LINE_HEIGHT) / 2; + const outerRadius = radius; return !this._isChartEmpty() ? (
@@ -41,10 +50,11 @@ export class PieChartBase extends React.Component { width={width!} height={height!} outerRadius={outerRadius} - innerRadius={0} + innerRadius={1} data={data!} colors={colors!} chartTitle={chartTitle!} + theme={theme} />
) : ( diff --git a/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx b/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx index 3d5cd25ebe576e..368b785f66405e 100644 --- a/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx +++ b/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx @@ -2,6 +2,7 @@ import * as React from 'react'; import { queryAllByAttribute, render, waitFor } from '@testing-library/react'; import { PieChart } from './index'; import { chartPoints, colors } from './PieChart.test'; +import * as utils from '../../utilities/utilities'; describe('Pie chart rendering', () => { test('Should re-render the Pie chart with data', async () => { @@ -11,6 +12,11 @@ describe('Pie chart rendering', () => { // Assert expect(container).toMatchSnapshot(); expect(getById(container, /_PieChart_empty/i)).toHaveLength(1); + + // Mock the implementation of wrapContent as it internally calls a Browser Function like + // getComputedTextLength() which will otherwise lead to a crash if mounted + jest.spyOn(utils, 'wrapContent').mockImplementation(() => false); + // Act rerender(); await waitFor(() => { diff --git a/packages/react-charting/src/components/PieChart/__snapshots__/PieChart.test.tsx.snap b/packages/react-charting/src/components/PieChart/__snapshots__/PieChart.test.tsx.snap index 4ff4835af55382..ae261f4d917a9f 100644 --- a/packages/react-charting/src/components/PieChart/__snapshots__/PieChart.test.tsx.snap +++ b/packages/react-charting/src/components/PieChart/__snapshots__/PieChart.test.tsx.snap @@ -13,81 +13,160 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` width: 670px; } > - - - - - A - - - 50 - - - - + + A-50 + + + - - B - - - 25 - - - - + + B-25 + + + - - C - - - 25 - + + + C-25 + +
- - + + `; @@ -104,80 +183,159 @@ exports[`PieChart snapShot testing renders with colors, width and height data co width: 670px; } > - - - - - A - - - 50 - - - - + + A-50 + + + - - B - - - 25 - - - - + + B-25 + + + - - C - - - 25 - + + + C-25 + + - - + + `; diff --git a/packages/react-charting/src/components/PieChart/__snapshots__/PieChartRTL.test.tsx.snap b/packages/react-charting/src/components/PieChart/__snapshots__/PieChartRTL.test.tsx.snap index e730fc97eced35..70492e99956e88 100644 --- a/packages/react-charting/src/components/PieChart/__snapshots__/PieChartRTL.test.tsx.snap +++ b/packages/react-charting/src/components/PieChart/__snapshots__/PieChartRTL.test.tsx.snap @@ -4,7 +4,7 @@ exports[`Pie chart rendering Should re-render the Pie chart with data 1`] = `
`; @@ -118,81 +180,160 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` width: 670px; } > - - - - - A - - - 50 - - - - + + A-50 + + + - - B - - - 25 - - - - + + B-25 + + + - - C - - - 25 - + + + C-25 + + - - + + `; @@ -209,80 +350,159 @@ exports[`PieChart snapShot testing renders with colors, width and height data co width: 670px; } > - - - - - A - - - 50 - - - - + + A-50 + + + - - B - - - 25 - - - - + + B-25 + + + - - C - - - 25 - + + + C-25 + + - - + + `; diff --git a/packages/react-charting/src/utilities/SVGTooltipText.tsx b/packages/react-charting/src/utilities/SVGTooltipText.tsx index 463ead3dfc257b..eb625317a5465b 100644 --- a/packages/react-charting/src/utilities/SVGTooltipText.tsx +++ b/packages/react-charting/src/utilities/SVGTooltipText.tsx @@ -41,6 +41,22 @@ interface ISVGTooltipTextProps { */ maxHeight?: number; + /** + * Pass false to make prevent the tooptip from receiving focus through keyboard + * Eg: In Pie Chart, the focus should only land on the arcs and not on the text to + * avoid repitition of the same datapoint + * @defaultvalue true + */ + shouldReceiveFocus?: boolean; + + /** + * Pass true to show tooltip directly + * Eg: In Pie Chart, the tooltip is shown when the arc is focussed, so the prop is set to true, + * to directly show the tooltip from this component + * @defaultvalue false + */ + isTooltipVisibleProp?: boolean; + /** * Function to wrap text within specified width and height * and return a boolean value indicating whether the text overflowed @@ -88,7 +104,7 @@ export class SVGTooltipText } public render(): React.ReactNode { - const { content, tooltipProps, textProps } = this.props; + const { content, tooltipProps, textProps, shouldReceiveFocus = true } = this.props; const { isTooltipVisible } = this.state; const tooltipRenderProps: ITooltipProps = { content, @@ -103,7 +119,8 @@ export class SVGTooltipText ...tooltipProps, }; - const showTooltip = isTooltipVisible && !!content; + const showTooltip = + (!!this.props.isTooltipVisibleProp && this.state.isOverflowing && !!content) || (isTooltipVisible && !!content); return ( <> @@ -116,7 +133,7 @@ export class SVGTooltipText onMouseEnter={this._onTooltipMouseEnter} onMouseLeave={this._onTooltipMouseLeave} onKeyDown={this._onTooltipKeyDown} - data-is-focusable={this.state.isOverflowing} + data-is-focusable={shouldReceiveFocus && this.state.isOverflowing} > {content} diff --git a/packages/react-examples/src/react-charting/PieChart/PieChart.Basic.Example.tsx b/packages/react-examples/src/react-charting/PieChart/PieChart.Basic.Example.tsx index 9aa1661cbfda8d..0499be1fc2b45e 100644 --- a/packages/react-examples/src/react-charting/PieChart/PieChart.Basic.Example.tsx +++ b/packages/react-examples/src/react-charting/PieChart/PieChart.Basic.Example.tsx @@ -1,26 +1,65 @@ import * as React from 'react'; import { PieChart, IPieChartProps } from '@fluentui/react-charting'; -import { DefaultPalette } from '@fluentui/react/lib/Styling'; +import { Stack, StackItem } from '@fluentui/react'; -export class PieChartBasicExample extends React.Component { +export class PieChartBasicExample extends React.Component { constructor(props: IPieChartProps) { super(props); + this.state = { + height: 350, + width: 600, + }; } public render(): JSX.Element { const points = [ - { y: 50, x: 'A' }, - { y: 25, x: 'B' }, - { y: 25, x: 'C' }, + { y: 50, x: 'ABCD' }, + { y: 25, x: 'EFGH' }, + { y: 25, x: 'IJKL' }, ]; - const colors = [DefaultPalette.red, DefaultPalette.blue, DefaultPalette.green]; return ( - + + + + + + + + + + + + + ); } + + private _onWidthChange = (e: React.ChangeEvent) => { + this.setState({ width: parseInt(e.target.value, 10) }); + }; + private _onHeightChange = (e: React.ChangeEvent) => { + this.setState({ height: parseInt(e.target.value, 10) }); + }; } diff --git a/packages/react-examples/src/react-charting/PieChart/PieChart.Dynamic.Example.tsx b/packages/react-examples/src/react-charting/PieChart/PieChart.Dynamic.Example.tsx index 1514361e474d55..d79b767c6eef86 100644 --- a/packages/react-examples/src/react-charting/PieChart/PieChart.Dynamic.Example.tsx +++ b/packages/react-examples/src/react-charting/PieChart/PieChart.Dynamic.Example.tsx @@ -1,25 +1,21 @@ import * as React from 'react'; -import { IDataPoint, PieChart, IPieChartProps } from '@fluentui/react-charting'; -import { DefaultPalette } from '@fluentui/react/lib/Styling'; +import { IDataPoint, PieChart, IPieChartProps, DataVizPalette } from '@fluentui/react-charting'; import { DefaultButton } from '@fluentui/react/lib/Button'; +import { Stack, StackItem } from '@fluentui/react'; export interface IExampleState { dynamicData: IDataPoint[]; colors: string[]; + width: number; + height: number; } export class PieChartDynamicExample extends React.Component { private _colors = [ - [ - DefaultPalette.blueLight, - DefaultPalette.blue, - DefaultPalette.tealLight, - DefaultPalette.teal, - DefaultPalette.greenLight, - ], - [DefaultPalette.purpleLight, DefaultPalette.purple, DefaultPalette.magentaLight, DefaultPalette.magenta], - [DefaultPalette.yellowLight, DefaultPalette.yellow, DefaultPalette.orangeLighter, DefaultPalette.orangeLight], - [DefaultPalette.neutralLight, DefaultPalette.neutralQuaternary, DefaultPalette.neutralTertiary], + [DataVizPalette.color1, DataVizPalette.color2, DataVizPalette.color3, DataVizPalette.color4, DataVizPalette.color5], + [DataVizPalette.color6, DataVizPalette.color7, DataVizPalette.color8, DataVizPalette.color9], + [DataVizPalette.color10, DataVizPalette.color11, DataVizPalette.color12, DataVizPalette.color13], + [DataVizPalette.color30], ]; constructor(props: IPieChartProps) { @@ -31,12 +27,9 @@ export class PieChartDynamicExample extends React.Component - + + + + + + + + + + + @@ -82,4 +107,11 @@ export class PieChartDynamicExample extends React.Component) => { + this.setState({ width: parseInt(e.target.value, 10) }); + }; + private _onHeightChange = (e: React.ChangeEvent) => { + this.setState({ height: parseInt(e.target.value, 10) }); + }; }