From 4fb4fbde57c4b43c77ec18bc8bcaf5c6763ef1c3 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Thu, 6 Jul 2023 11:41:09 +0530 Subject: [PATCH 01/32] changes for pie chart accessibiilty issues --- .../src/components/PieChart/Arc/Arc.styles.ts | 9 +- .../src/components/PieChart/Arc/Arc.tsx | 74 +++- .../src/components/PieChart/Arc/Arc.types.ts | 29 +- .../src/components/PieChart/Pie/Pie.tsx | 47 ++- .../src/components/PieChart/Pie/Pie.types.ts | 6 +- .../src/components/PieChart/PieChart.base.tsx | 19 +- .../__snapshots__/PieChart.test.tsx.snap | 364 ++++++++++++------ .../PieChart/PieChart.Dynamic.Example.tsx | 23 +- 8 files changed, 401 insertions(+), 170 deletions(-) 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 33a1f8e766333..23a246798eb41 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 }, + pieRoot: { fill: color, stroke: theme?.palette.white || DefaultPalette.white, strokeWidth: 2 }, + pieRootFocussed: { fill: color, stroke: theme?.palette.black || DefaultPalette.black, strokeWidth: 3 }, + pie: { outline: 'none' }, + pieText: { fill: theme?.palette.black || DefaultPalette.black }, }; }; diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx index 84d216979f829..cdc8190ef3e45 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -1,44 +1,96 @@ import * as React from 'react'; import * as shape from 'd3-shape'; -import { IArcProps, IArcStyles } from './Arc.types'; +import { IArcProps, IArcState, IArcStyles } from './Arc.types'; import { classNamesFunction } from '@fluentui/react/lib/Utilities'; import { getStyles } from './Arc.styles'; import { convertToLocaleString } from '../../../utilities/utilities'; -export class Arc extends React.Component { +export class Arc extends React.Component { public static defaultProps: Partial = { arc: shape.arc(), }; - public state: {} = {}; - public static getDerivedStateFromProps(nextProps: Readonly): null { _updateChart(nextProps); return null; } + public constructor(props: IArcProps) { + super(props); + this.state = { + isArcFocussed: false, + }; + } + 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 ( + <> + + + {this.props.data!.data!.x}-{convertToLocaleString(this.props.data!.data!.y, this.props.culture)} + + + {this.state.isArcFocussed ? : null} + + ); } + + protected _onFocus = () => { + this.setState({ isArcFocussed: true }); + this.props.onPieFocussed(JSON.stringify(this.props.data)); + }; + + protected _onBlur = () => { + this.setState({ isArcFocussed: false }); + this.props.onPieFocussed(JSON.stringify(this.props.data)); + }; } export class LabeledArc extends Arc { + public constructor(props: IArcProps) { + super(props); + } + public render(): JSX.Element { - const { data, arc, culture } = this.props; - const [labelX, labelY] = arc.centroid(data); + const { data, culture } = this.props; + const gap = 20; + const [labelX, labelY] = shape.arc().centroid({ + endAngle: data?.endAngle || 0, + startAngle: data?.startAngle || 0, + padAngle: data?.padAngle, + innerRadius: this.props.outerRadius, + outerRadius: this.props.outerRadius + gap, + }); const labelTranslate = `translate(${labelX}, ${labelY})`; + const getClassNames = classNamesFunction(); + const classNames = getClassNames(props => getStyles(props, this.props.theme)); + + const angleBisector = ((data?.startAngle || 0) + (data?.endAngle || 0)) / 2; + return ( - + {super.render()} - + {data!.data!.x}-{convertToLocaleString(data!.data!.y, culture)} 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 68dafd822f215..d6423f21b78f9 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 { @@ -16,11 +16,11 @@ export interface IArcProps { /** * innerRadius of the Arc. */ - innerRadius?: number; + innerRadius: number; /** * outerRadius of the Arc. */ - outerRadius?: number; + outerRadius: number; /** * Color for the Arc. */ @@ -30,6 +30,21 @@ export interface IArcProps { * The prop used to define the culture to localized the numbers */ culture?: string; + /** + * to pass the theme + */ + theme?: ITheme; + /** + * Callback function to be triggered when the focus is on the element + */ + onPieFocussed: (id: string) => void; +} + +export interface IArcState { + /** + * The state controls, whether the arc needs to be focussed or not + */ + isArcFocussed?: boolean; } export interface IArcData { @@ -63,5 +78,11 @@ export interface IArcStyles { /** * Style set for the card header component root */ - root: IStyle; + pieRoot: IStyle; + + pie: IStyle; + + pieRootFocussed: IStyle; + + pieText: IStyle; } diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx index 722464fe38a46..b94e16eeec3ea 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -1,11 +1,11 @@ import * as React from 'react'; import * as shape from 'd3-shape'; -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'; -export class Pie extends React.Component { +export class Pie extends React.Component { public static defaultProps: Partial = { pie: shape .pie() @@ -13,38 +13,43 @@ export class Pie extends React.Component { /* eslint-disable @typescript-eslint/no-explicit-any */ .value((d: any) => d.y), }; - private colors: scale.ScaleOrdinal; constructor(props: IPieProps) { super(props); + this.state = { focussedPie: '' }; } - public arcGenerator = (d: IArcData, i: number): JSX.Element => { - return ( - - ); - }; - public render(): JSX.Element { // const getClassNames = classNamesFunction(); const { pie, colors, data, width, height, chartTitle } = this.props; - this.colors = scale.scaleOrdinal().range(colors!); - 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) => ( + + ))} + + + + ); } + + private _onPieFocussed = (id: string) => { + this.setState({ focussedPie: id }); + }; } 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 4a6ca1140cfbd..3434f7e920477 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 { @@ -25,7 +25,7 @@ export interface IPieProps { /** * colors to render in the Pie. */ - colors?: string[]; + colors: string[]; /** * Title to apply to the whole chart. */ @@ -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 9b500672680c1..23fb73df1eee1 100644 --- a/packages/react-charting/src/components/PieChart/PieChart.base.tsx +++ b/packages/react-charting/src/components/PieChart/PieChart.base.tsx @@ -3,6 +3,7 @@ import { classNamesFunction, getId } from '@fluentui/react/lib/Utilities'; import { IPieChartProps, IPieChartStyleProps, IPieChartStyles } from './PieChart.types'; import { Pie } from './Pie/Pie'; import { IProcessedStyleSet } from '@fluentui/react/lib/Styling'; +import { getColorFromToken, getNextColor } from '../../utilities/colors'; const getClassNames = classNamesFunction(); export interface IPieChartState { @@ -38,7 +39,20 @@ export class PieChartBase extends React.Component @@ -58,8 +72,9 @@ export class PieChartBase extends React.Component ) : ( 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 4ff4835af5538..17a62eb0afe13 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,153 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` width: 670px; } > - - - - - A - - - 50 - - - - + + A + - + 50 + + + + A + - + 50 + + + - - B - - - 25 - - - - + + B + - + 25 + + + + B + - + 25 + + + - - C - - - 25 - + + + C + - + 25 + + + + C + - + 25 + + + - - + + `; @@ -104,80 +176,152 @@ exports[`PieChart snapShot testing renders with colors, width and height data co width: 670px; } > - - - - - A - - - 50 - - - - + + A + - + 50 + + + + A + - + 50 + + + - - B - - - 25 - - - - + + B + - + 25 + + + + B + - + 25 + + + - - C - - - 25 - + + + C + - + 25 + + + + C + - + 25 + + + - - + + `; 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 1514361e474d5..f877a8f737ff1 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,5 +1,5 @@ import * as React from 'react'; -import { IDataPoint, PieChart, IPieChartProps } from '@fluentui/react-charting'; +import { IDataPoint, PieChart, IPieChartProps, DataVizPalette } from '@fluentui/react-charting'; import { DefaultPalette } from '@fluentui/react/lib/Styling'; import { DefaultButton } from '@fluentui/react/lib/Button'; @@ -10,16 +10,10 @@ export interface IExampleState { 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, DataVizPalette.color28, DataVizPalette.color25], ]; constructor(props: IPieChartProps) { @@ -31,12 +25,7 @@ export class PieChartDynamicExample extends React.Component Date: Fri, 7 Jul 2023 10:54:35 +0530 Subject: [PATCH 02/32] pie chart accessibility --- .../react-charting/src/components/PieChart/Arc/Arc.tsx | 7 ++++--- .../react-charting/src/components/PieChart/Pie/Pie.tsx | 2 +- .../src/components/PieChart/PieChart.base.tsx | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx index cdc8190ef3e45..9f3b5ee346122 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -65,7 +65,7 @@ export class LabeledArc extends Arc { public render(): JSX.Element { const { data, culture } = this.props; - const gap = 20; + const gap = 10; const [labelX, labelY] = shape.arc().centroid({ endAngle: data?.endAngle || 0, startAngle: data?.startAngle || 0, @@ -78,7 +78,7 @@ export class LabeledArc extends Arc { const getClassNames = classNamesFunction(); const classNames = getClassNames(props => getStyles(props, this.props.theme)); - const angleBisector = ((data?.startAngle || 0) + (data?.endAngle || 0)) / 2; + const angle = ((data?.startAngle || 0) + (data?.endAngle || 0)) / 2; return ( Math.PI ? 'end' : 'start'} + dominantBaseline={angle > Math.PI / 2 && angle < (3 * Math.PI) / 2 ? 'hanging' : 'auto'} > {super.render()} diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx index b94e16eeec3ea..2ecb721d57e51 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -29,7 +29,7 @@ export class Pie extends React.Component { return ( - + {piechart.map((d: IArcData, i: number) => ( From cf47e6e6db846b3f0ccddf034115fbd125e3afa7 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Fri, 7 Jul 2023 18:42:57 +0530 Subject: [PATCH 03/32] pie chart accessibility issue fixed, also example width adjustment added --- .../src/components/PieChart/Arc/Arc.styles.ts | 4 +- .../src/components/PieChart/Arc/Arc.tsx | 67 ++++++++++++++----- .../src/components/PieChart/Arc/Arc.types.ts | 12 ++-- .../src/components/PieChart/Pie/Pie.tsx | 15 +++-- .../src/components/PieChart/PieChart.base.tsx | 4 +- 5 files changed, 69 insertions(+), 33 deletions(-) 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 23a246798eb41..8e09533181012 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.styles.ts +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.styles.ts @@ -4,8 +4,8 @@ export const getStyles = (props: IArcProps, theme: ITheme | undefined): IArcStyl const { color } = props; return { pieRoot: { fill: color, stroke: theme?.palette.white || DefaultPalette.white, strokeWidth: 2 }, - pieRootFocussed: { fill: color, stroke: theme?.palette.black || DefaultPalette.black, strokeWidth: 3 }, + pieRootFocused: { fill: color, stroke: theme?.palette.black || DefaultPalette.black, strokeWidth: 3 }, pie: { outline: 'none' }, - pieText: { fill: theme?.palette.black || DefaultPalette.black }, + pieText: { 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 9f3b5ee346122..4d1dfdcc5d777 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -1,9 +1,11 @@ import * as React from 'react'; import * as shape from 'd3-shape'; import { IArcProps, IArcState, IArcStyles } from './Arc.types'; -import { classNamesFunction } from '@fluentui/react/lib/Utilities'; +import { classNamesFunction, getId } from '@fluentui/react/lib/Utilities'; import { getStyles } from './Arc.styles'; import { convertToLocaleString } from '../../../utilities/utilities'; +import { SVGTooltipText } from '../../../utilities/SVGTooltipText'; +import { select as d3Select } from 'd3-selection'; export class Arc extends React.Component { public static defaultProps: Partial = { @@ -18,7 +20,7 @@ export class Arc extends React.Component { public constructor(props: IArcProps) { super(props); this.state = { - isArcFocussed: false, + isArcFocused: false, }; } @@ -35,37 +37,40 @@ export class Arc extends React.Component { <> - + {/* {this.props.data!.data!.x}-{convertToLocaleString(this.props.data!.data!.y, this.props.culture)} - + */} - {this.state.isArcFocussed ? : null} + {this.state.isArcFocused ? : null} ); } protected _onFocus = () => { - this.setState({ isArcFocussed: true }); - this.props.onPieFocussed(JSON.stringify(this.props.data)); + this.setState({ isArcFocused: true }); + this.props.onPieFocused(this.state.id); }; protected _onBlur = () => { - this.setState({ isArcFocussed: false }); - this.props.onPieFocussed(JSON.stringify(this.props.data)); + this.setState({ isArcFocused: false }); + this.props.onPieFocused(''); }; } export class LabeledArc extends Arc { public constructor(props: IArcProps) { super(props); + this.state = { + id: getId(), + }; } public render(): JSX.Element { const { data, culture } = this.props; - const gap = 10; + const gap = 4; const [labelX, labelY] = shape.arc().centroid({ endAngle: data?.endAngle || 0, startAngle: data?.startAngle || 0, @@ -73,7 +78,6 @@ export class LabeledArc extends Arc { innerRadius: this.props.outerRadius, outerRadius: this.props.outerRadius + gap, }); - const labelTranslate = `translate(${labelX}, ${labelY})`; const getClassNames = classNamesFunction(); const classNames = getClassNames(props => getStyles(props, this.props.theme)); @@ -86,17 +90,44 @@ export class LabeledArc extends Arc { onBlur={this._onBlur} className={classNames.pie} data-is-focusable={true} - id={JSON.stringify(this.props.data)} - textAnchor={angle > Math.PI ? 'end' : 'start'} - dominantBaseline={angle > Math.PI / 2 && angle < (3 * Math.PI) / 2 ? 'hanging' : 'auto'} + id={this.state.id} > {super.render()} - - {data!.data!.x}-{convertToLocaleString(data!.data!.y, culture)} - + Math.PI / 2 && angle < (3 * Math.PI) / 2 ? 'hanging' : 'auto', + textAnchor: angle > Math.PI ? 'end' : 'start', + role: 'img', + 'aria-label': `${data?.data.x}-${convertToLocaleString(data?.data.y, culture)}`, + className: classNames.pieText, + }} + maxWidth={40} + wrapContent={this._wrapContent} + /> ); } + + private _wrapContent = (content: string, id: string, maxWidth: number) => { + const textElement = d3Select(`#${id}`); + textElement.text(content); + if (!textElement.node()) { + return false; + } + + let isOverflowing = false; + let textLength = textElement.node()!.getComputedTextLength(); + while (textLength > maxWidth && content.length > 0) { + content = content.slice(0, -1); + textElement.text(content + '...'); + isOverflowing = true; + textLength = textElement.node()!.getComputedTextLength(); + } + return isOverflowing; + }; } function _updateChart(newProps: IArcProps): void { 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 d6423f21b78f9..3de5f6ca5969c 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.types.ts +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.types.ts @@ -37,14 +37,18 @@ export interface IArcProps { /** * Callback function to be triggered when the focus is on the element */ - onPieFocussed: (id: string) => void; + onPieFocused: (id: string) => void; } export interface IArcState { /** - * The state controls, whether the arc needs to be focussed or not + * The state controls, whether the arc needs to be focused or not */ - isArcFocussed?: boolean; + isArcFocused?: boolean; + /** + * Unique Identifier for each arc + */ + id: string; } export interface IArcData { @@ -82,7 +86,7 @@ export interface IArcStyles { pie: IStyle; - pieRootFocussed: IStyle; + pieRootFocused: IStyle; pieText: IStyle; } diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx index 2ecb721d57e51..7f393462ede6b 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -4,8 +4,9 @@ import { IPieProps } from './Pie.types'; import { LabeledArc } from '../Arc/Arc'; import { IArcData } from '../Arc/Arc.types'; import { FocusZone, FocusZoneDirection } from '@fluentui/react-focus'; +import { getId } from '@fluentui/react'; -export class Pie extends React.Component { +export class Pie extends React.Component { public static defaultProps: Partial = { pie: shape .pie() @@ -16,7 +17,7 @@ export class Pie extends React.Component { constructor(props: IPieProps) { super(props); - this.state = { focussedPie: '' }; + this.state = { focusedPie: '' }; } public render(): JSX.Element { @@ -29,7 +30,7 @@ export class Pie extends React.Component { return ( - + {piechart.map((d: IArcData, i: number) => ( { outerRadius={this.props.outerRadius} color={colors[i]} theme={this.props.theme} - onPieFocussed={this._onPieFocussed} + onPieFocused={this._onPieFocused} /> ))} - + ); } - private _onPieFocussed = (id: string) => { - this.setState({ focussedPie: id }); + private _onPieFocused = (id: string) => { + this.setState({ focusedPie: id }); }; } diff --git a/packages/react-charting/src/components/PieChart/PieChart.base.tsx b/packages/react-charting/src/components/PieChart/PieChart.base.tsx index 77fbde9a4cf6e..f65f499096414 100644 --- a/packages/react-charting/src/components/PieChart/PieChart.base.tsx +++ b/packages/react-charting/src/components/PieChart/PieChart.base.tsx @@ -59,8 +59,8 @@ export class PieChartBase extends React.Component From 8d31ab2756be7f6225cc77dc355cd9391f4243c9 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Fri, 7 Jul 2023 18:44:37 +0530 Subject: [PATCH 04/32] examples made width and height adjustable --- .../PieChart/PieChart.Basic.Example.tsx | 65 +++++++++++++++---- .../PieChart/PieChart.Dynamic.Example.tsx | 46 ++++++++++++- 2 files changed, 98 insertions(+), 13 deletions(-) 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 9aa1661cbfda8..4929f3128880b 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,67 @@ import * as React from 'react'; import { PieChart, IPieChartProps } from '@fluentui/react-charting'; -import { DefaultPalette } from '@fluentui/react/lib/Styling'; +import { Stack, StackItem, ThemeProvider } from '@fluentui/react'; +import { DarkTheme, DefaultTheme } from '@fluentui/theme-samples'; -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: 'AAAA' }, + { y: 25, x: 'BBVB' }, + { y: 25, x: 'CDFDF' }, ]; - 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 f877a8f737ff1..83a441c1724b0 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 @@ -2,10 +2,13 @@ import * as React from 'react'; import { IDataPoint, PieChart, IPieChartProps, DataVizPalette } from '@fluentui/react-charting'; import { DefaultPalette } from '@fluentui/react/lib/Styling'; 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 { @@ -26,6 +29,8 @@ export class PieChartDynamicExample extends React.Component - + + + + + + + + + + + @@ -71,4 +108,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) }); + }; } From 3e797b0e4753c4469049131010c726646149373e Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Mon, 10 Jul 2023 12:52:41 +0530 Subject: [PATCH 05/32] fixed svg tooltip text --- .../src/components/PieChart/Arc/Arc.tsx | 10 +++++++--- .../src/utilities/SVGTooltipText.tsx | 17 ++++++++++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx index 4d1dfdcc5d777..807f7914c698b 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -84,17 +84,19 @@ export class LabeledArc extends Arc { const angle = ((data?.startAngle || 0) + (data?.endAngle || 0)) / 2; + const content = `${data?.data.x}-${convertToLocaleString(data?.data.y, culture)}`; + return ( {super.render()} diff --git a/packages/react-charting/src/utilities/SVGTooltipText.tsx b/packages/react-charting/src/utilities/SVGTooltipText.tsx index c8a8c14875158..dad02649cd948 100644 --- a/packages/react-charting/src/utilities/SVGTooltipText.tsx +++ b/packages/react-charting/src/utilities/SVGTooltipText.tsx @@ -41,6 +41,16 @@ interface ISVGTooltipTextProps { */ maxHeight?: number; + /** + * Pass false to make the data not focusable, though keyboard. true by default + */ + shouldReceiveFocus?: boolean; + + /** + * Pass true to show tooltip directly, false by default + */ + showTooltip?: boolean; + /** * Function to wrap text within specified width and height * and return a boolean value indicating whether the text overflowed @@ -88,7 +98,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 = { @@ -104,7 +114,8 @@ export class SVGTooltipText ...tooltipProps, }; - const showTooltip = isTooltipVisible && !!content; + const showTooltip = + (!!this.props.showTooltip && this.state.isOverflowing && !!content) || (isTooltipVisible && !!content); return ( <> @@ -117,7 +128,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} From 27600986fb7f5d2b55ee392353e21fa774ecaf74 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Tue, 11 Jul 2023 14:12:22 +0530 Subject: [PATCH 06/32] added role to multistacked bar chart --- .../src/components/PieChart/Arc/Arc.tsx | 11 ++++++----- .../src/components/PieChart/PieChart.base.tsx | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx index 807f7914c698b..b7e22681f5e4a 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -39,11 +39,12 @@ export class Arc extends React.Component { d={arc(this.props.data)} className={`${this.state.isArcFocused ? classNames.pieRootFocused : classNames.pieRoot}`} onClick={this.props.data?.data.onClick} - > - {/* - {this.props.data!.data!.x}-{convertToLocaleString(this.props.data!.data!.y, this.props.culture)} - */} - + role="img" + aria-label={`${this.props.data!.data!.x}-${convertToLocaleString( + this.props.data!.data!.y, + this.props.culture, + )}`} + /> {this.state.isArcFocused ? : null} ); diff --git a/packages/react-charting/src/components/PieChart/PieChart.base.tsx b/packages/react-charting/src/components/PieChart/PieChart.base.tsx index f65f499096414..a1bac3b5fca91 100644 --- a/packages/react-charting/src/components/PieChart/PieChart.base.tsx +++ b/packages/react-charting/src/components/PieChart/PieChart.base.tsx @@ -63,7 +63,7 @@ export class PieChartBase extends React.Component +
{this.props.chartTitle &&

{this.props.chartTitle}

} Date: Tue, 11 Jul 2023 15:23:03 +0530 Subject: [PATCH 07/32] pie chart build fixes and snapshot updates --- ...-7ed503a2-d9d1-4679-8074-c67eb83161e1.json | 7 + .../src/components/PieChart/Arc/Arc.tsx | 10 +- .../src/components/PieChart/Pie/Pie.tsx | 3 +- .../src/components/PieChart/PieChart.base.tsx | 2 +- .../__snapshots__/PieChart.test.tsx.snap | 202 ++++++++++-------- .../PieChart/PieChart.Dynamic.Example.tsx | 1 - 6 files changed, 129 insertions(+), 96 deletions(-) create mode 100644 change/@fluentui-react-charting-7ed503a2-d9d1-4679-8074-c67eb83161e1.json 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 0000000000000..bfdfd748222f2 --- /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": "email not defined", + "dependentChangeType": "patch" +} diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx index b7e22681f5e4a..763b63c97b43a 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -21,6 +21,7 @@ export class Arc extends React.Component { super(props); this.state = { isArcFocused: false, + id: getId(), }; } @@ -39,13 +40,7 @@ export class Arc extends React.Component { d={arc(this.props.data)} className={`${this.state.isArcFocused ? classNames.pieRootFocused : classNames.pieRoot}`} onClick={this.props.data?.data.onClick} - role="img" - aria-label={`${this.props.data!.data!.x}-${convertToLocaleString( - this.props.data!.data!.y, - this.props.culture, - )}`} /> - {this.state.isArcFocused ? : null} ); } @@ -94,6 +89,8 @@ export class LabeledArc extends Arc { id={this.state.id} onFocus={this._onFocus} onBlur={this._onBlur} + aria-label={content} + role="img" > {super.render()} Math.PI / 2 && angle < (3 * Math.PI) / 2 ? 'hanging' : 'auto', textAnchor: angle > Math.PI ? 'end' : 'start', - role: 'img', 'aria-label': `${data?.data.x}-${convertToLocaleString(data?.data.y, culture)}`, className: classNames.pieText, }} diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx index 7f393462ede6b..4fb5ad63c6f03 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -4,7 +4,6 @@ import { IPieProps } from './Pie.types'; import { LabeledArc } from '../Arc/Arc'; import { IArcData } from '../Arc/Arc.types'; import { FocusZone, FocusZoneDirection } from '@fluentui/react-focus'; -import { getId } from '@fluentui/react'; export class Pie extends React.Component { public static defaultProps: Partial = { @@ -29,7 +28,7 @@ export class Pie extends React.Component { return ( - + {piechart.map((d: IArcData, i: number) => ( +
{this.props.chartTitle &&

{this.props.chartTitle}

} - - A - - - 50 - - + d="M0,-159A159,159,0,1,1,0,159L0,0Z" + /> - A - - - 50 + A-50 - - B - - - 25 - - + d="M0,159A159,159,0,0,1,-159,0L0,0Z" + /> - B - - - 25 + B-25 - - C - - - 25 - - + d="M-159,0A159,159,0,0,1,0,-159L0,0Z" + /> - C - - - 25 + C-25 - - A - - - 50 - - + d="M0,-134A134,134,0,1,1,0,134L0,0Z" + /> - A - - - 50 + A-50 - - B - - - 25 - - + d="M0,134A134,134,0,0,1,-134,0L0,0Z" + /> - B - - - 25 + B-25 - - C - - - 25 - - + d="M-134,0A134,134,0,0,1,0,-134L0,0Z" + /> - C - - - 25 + C-25 Date: Tue, 11 Jul 2023 16:45:45 +0530 Subject: [PATCH 08/32] screen reader bug fixes --- packages/react-charting/src/components/PieChart/Pie/Pie.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx index 4fb5ad63c6f03..ee60f814153de 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -28,7 +28,7 @@ export class Pie extends React.Component { return ( - + {piechart.map((d: IArcData, i: number) => ( { onPieFocused={this._onPieFocused} /> ))} - + From 9d8d81ea987701bb56eec55d50a017194b2d9306 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Wed, 12 Jul 2023 16:29:03 +0530 Subject: [PATCH 09/32] pie chart POC --- .../src/components/PieChart/Arc/Arc.tsx | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx index 763b63c97b43a..b10267a68c144 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -35,13 +35,11 @@ export class Arc extends React.Component { const classNames = getClassNames(props => getStyles(props, this.props.theme), { ...this.props }); return ( - <> - - + ); } @@ -67,6 +65,7 @@ export class LabeledArc extends Arc { public render(): JSX.Element { 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, From b321e752b01888cce87789273d57f5abb187b918 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Wed, 12 Jul 2023 17:48:13 +0530 Subject: [PATCH 10/32] pie chart spacing added betweeen the pie --- .../src/components/PieChart/Arc/Arc.styles.ts | 2 +- .../react-charting/src/components/PieChart/Arc/Arc.tsx | 2 -- .../src/components/PieChart/Arc/Arc.types.ts | 4 ---- .../react-charting/src/components/PieChart/Pie/Pie.tsx | 7 +------ .../src/components/PieChart/PieChart.base.tsx | 2 +- 5 files changed, 3 insertions(+), 14 deletions(-) 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 8e09533181012..85432cfac68ae 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.styles.ts +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.styles.ts @@ -3,7 +3,7 @@ import { DefaultPalette, ITheme } from '@fluentui/react/lib/Styling'; export const getStyles = (props: IArcProps, theme: ITheme | undefined): IArcStyles => { const { color } = props; return { - pieRoot: { fill: color, stroke: theme?.palette.white || DefaultPalette.white, strokeWidth: 2 }, + pieRoot: { fill: color, stroke: theme?.palette.white || DefaultPalette.white, strokeWidth: 0 }, pieRootFocused: { fill: color, stroke: theme?.palette.black || DefaultPalette.black, strokeWidth: 3 }, pie: { outline: 'none' }, pieText: { 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 b10267a68c144..d2a09ad4cbbf3 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -45,12 +45,10 @@ export class Arc extends React.Component { protected _onFocus = () => { this.setState({ isArcFocused: true }); - this.props.onPieFocused(this.state.id); }; protected _onBlur = () => { this.setState({ isArcFocused: false }); - this.props.onPieFocused(''); }; } 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 3de5f6ca5969c..203810812a8ec 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.types.ts +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.types.ts @@ -34,10 +34,6 @@ export interface IArcProps { * to pass the theme */ theme?: ITheme; - /** - * Callback function to be triggered when the focus is on the element - */ - onPieFocused: (id: string) => void; } export interface IArcState { diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx index ee60f814153de..f4700869bb4dc 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -9,6 +9,7 @@ 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), @@ -39,17 +40,11 @@ export class Pie extends React.Component { outerRadius={this.props.outerRadius} color={colors[i]} theme={this.props.theme} - onPieFocused={this._onPieFocused} /> ))} - ); } - - private _onPieFocused = (id: string) => { - this.setState({ focusedPie: id }); - }; } diff --git a/packages/react-charting/src/components/PieChart/PieChart.base.tsx b/packages/react-charting/src/components/PieChart/PieChart.base.tsx index f65f499096414..9dec9b4685079 100644 --- a/packages/react-charting/src/components/PieChart/PieChart.base.tsx +++ b/packages/react-charting/src/components/PieChart/PieChart.base.tsx @@ -70,7 +70,7 @@ export class PieChartBase extends React.Component Date: Wed, 12 Jul 2023 18:13:55 +0530 Subject: [PATCH 11/32] snapshots updated --- .../__snapshots__/PieChart.test.tsx.snap | 60 ++++++++----------- 1 file changed, 26 insertions(+), 34 deletions(-) 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 dd80c1f7d1b55..0ff6028e12cad 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 @@ -26,7 +26,6 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` > A-50 @@ -96,10 +95,10 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` { fill: #0078d4; - stroke-width: 2px; + stroke-width: 0px; stroke: #ffffff; } - d="M0,159A159,159,0,0,1,-159,0L0,0Z" + d="M0,159A159,159,0,0,1,-158.996,1.193L-0.704,0.71Z" /> B-25 @@ -142,10 +141,10 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` { fill: #107c10; - stroke-width: 2px; + stroke-width: 0px; stroke: #ffffff; } - d="M-159,0A159,159,0,0,1,0,-159L0,0Z" + d="M-159,-0.398A159,159,0,0,1,-0.795,-158.998L-0.708,-0.706Z" /> C-25 -
@@ -205,7 +201,6 @@ exports[`PieChart snapShot testing renders with colors, width and height data co > A-50 @@ -275,10 +270,10 @@ exports[`PieChart snapShot testing renders with colors, width and height data co { fill: #0078d4; - stroke-width: 2px; + stroke-width: 0px; stroke: #ffffff; } - d="M0,134A134,134,0,0,1,-134,0L0,0Z" + d="M0,134A134,134,0,0,1,-133.996,1.005L-0.741,0.672A1,1,0,0,0,-0.666,0.746Z" /> B-25 @@ -321,10 +316,10 @@ exports[`PieChart snapShot testing renders with colors, width and height data co { fill: #107c10; - stroke-width: 2px; + stroke-width: 0px; stroke: #ffffff; } - d="M-134,0A134,134,0,0,1,0,-134L0,0Z" + d="M-134,-0.335A134,134,0,0,1,-0.67,-133.998L-0.67,-0.742A1,1,0,0,0,-0.744,-0.668Z" /> C-25 -
From a5a1221ccd31074ad889c945f8e27d0b2745e86f Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Thu, 13 Jul 2023 11:48:57 +0530 Subject: [PATCH 12/32] removed unused imports --- .../src/react-charting/PieChart/PieChart.Basic.Example.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 4929f3128880b..6bb0db6184512 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,7 +1,6 @@ import * as React from 'react'; import { PieChart, IPieChartProps } from '@fluentui/react-charting'; -import { Stack, StackItem, ThemeProvider } from '@fluentui/react'; -import { DarkTheme, DefaultTheme } from '@fluentui/theme-samples'; +import { Stack, StackItem } from '@fluentui/react'; export class PieChartBasicExample extends React.Component { constructor(props: IPieChartProps) { From 515bad496dcc6666a49e30b6eee3885a822a9fd2 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Thu, 13 Jul 2023 15:43:55 +0530 Subject: [PATCH 13/32] examples fixed --- .../src/react-charting/PieChart/PieChart.Basic.Example.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 6bb0db6184512..83c5f5df2dae8 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 @@ -13,9 +13,9 @@ export class PieChartBasicExample extends React.Component From 8fde75ab1f6ad246fa0a96ca22d0f10c6dfe23d6 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Fri, 14 Jul 2023 12:03:22 +0530 Subject: [PATCH 14/32] fixed changes post review --- .../src/components/PieChart/Arc/Arc.styles.ts | 4 ++-- .../src/components/PieChart/PieChart.base.tsx | 16 ++++++++++++---- .../PieChart/PieChart.Basic.Example.tsx | 3 +-- .../PieChart/PieChart.Dynamic.Example.tsx | 2 +- 4 files changed, 16 insertions(+), 9 deletions(-) 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 85432cfac68ae..94f1a9a1c0ad0 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.styles.ts +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.styles.ts @@ -3,8 +3,8 @@ import { DefaultPalette, ITheme } from '@fluentui/react/lib/Styling'; export const getStyles = (props: IArcProps, theme: ITheme | undefined): IArcStyles => { const { color } = props; return { - pieRoot: { fill: color, stroke: theme?.palette.white || DefaultPalette.white, strokeWidth: 0 }, - pieRootFocused: { fill: color, stroke: theme?.palette.black || DefaultPalette.black, strokeWidth: 3 }, + pieRoot: { fill: color }, + pieRootFocused: { fill: color, stroke: theme?.palette.black || DefaultPalette.black, strokeWidth: 2 }, pie: { outline: 'none' }, pieText: { fill: theme?.palette.black || DefaultPalette.black, outline: 'none' }, }; diff --git a/packages/react-charting/src/components/PieChart/PieChart.base.tsx b/packages/react-charting/src/components/PieChart/PieChart.base.tsx index 9dec9b4685079..88289e2e62a02 100644 --- a/packages/react-charting/src/components/PieChart/PieChart.base.tsx +++ b/packages/react-charting/src/components/PieChart/PieChart.base.tsx @@ -43,12 +43,11 @@ export class PieChartBase extends React.Component { @@ -52,7 +52,6 @@ export class PieChartBasicExample extends React.Component - ; ); } 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 e38c95a8e5e27..d79b767c6eef8 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 @@ -15,7 +15,7 @@ export class PieChartDynamicExample extends React.Component Date: Fri, 14 Jul 2023 12:26:46 +0530 Subject: [PATCH 15/32] upddated snaphots --- .../PieChart/__snapshots__/PieChart.test.tsx.snap | 12 ------------ 1 file changed, 12 deletions(-) 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 0ff6028e12cad..abd99a8cf1968 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 @@ -49,8 +49,6 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` { fill: #e81123; - stroke-width: 0px; - stroke: #ffffff; } d="M0.795,-158.998A159,159,0,0,1,1.59,158.992L0.798,0.603A1,1,0,0,0,0.795,-0.607Z" /> @@ -95,8 +93,6 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` { fill: #0078d4; - stroke-width: 0px; - stroke: #ffffff; } d="M0,159A159,159,0,0,1,-158.996,1.193L-0.704,0.71Z" /> @@ -141,8 +137,6 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` { fill: #107c10; - stroke-width: 0px; - stroke: #ffffff; } d="M-159,-0.398A159,159,0,0,1,-0.795,-158.998L-0.708,-0.706Z" /> @@ -224,8 +218,6 @@ exports[`PieChart snapShot testing renders with colors, width and height data co { fill: #e81123; - stroke-width: 0px; - stroke: #ffffff; } d="M0.67,-133.998A134,134,0,0,1,1.34,133.993L0.674,0.739A1,1,0,0,0,0.67,-0.742Z" /> @@ -270,8 +262,6 @@ exports[`PieChart snapShot testing renders with colors, width and height data co { fill: #0078d4; - stroke-width: 0px; - stroke: #ffffff; } d="M0,134A134,134,0,0,1,-133.996,1.005L-0.741,0.672A1,1,0,0,0,-0.666,0.746Z" /> @@ -316,8 +306,6 @@ exports[`PieChart snapShot testing renders with colors, width and height data co { fill: #107c10; - stroke-width: 0px; - stroke: #ffffff; } d="M-134,-0.335A134,134,0,0,1,-0.67,-133.998L-0.67,-0.742A1,1,0,0,0,-0.744,-0.668Z" /> From 75f836e82116c9c88c5b8d9490cfe693dcd91838 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Fri, 14 Jul 2023 15:47:35 +0530 Subject: [PATCH 16/32] lint fix --- packages/react-charting/src/components/PieChart/Pie/Pie.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx index f4700869bb4dc..0f273606d7fa8 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -5,7 +5,7 @@ import { LabeledArc } from '../Arc/Arc'; import { IArcData } from '../Arc/Arc.types'; import { FocusZone, FocusZoneDirection } from '@fluentui/react-focus'; -export class Pie extends React.Component { +export class Pie extends React.Component { public static defaultProps: Partial = { pie: shape .pie() @@ -17,7 +17,6 @@ export class Pie extends React.Component { constructor(props: IPieProps) { super(props); - this.state = { focusedPie: '' }; } public render(): JSX.Element { From b76477b91f8b5766ad5dde78e163d9ef85f5f3ff Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Fri, 14 Jul 2023 15:49:59 +0530 Subject: [PATCH 17/32] lint fix --- .../src/react-charting/PieChart/PieChart.Basic.Example.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3e1a41b8c4100..0499be1fc2b45 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,5 +1,5 @@ import * as React from 'react'; -import { PieChart, IPieChartProps, DataVizPalette } from '@fluentui/react-charting'; +import { PieChart, IPieChartProps } from '@fluentui/react-charting'; import { Stack, StackItem } from '@fluentui/react'; export class PieChartBasicExample extends React.Component { From 9fbea376d862a102b9ae92155bc9ec429efc6d0d Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Fri, 14 Jul 2023 16:19:03 +0530 Subject: [PATCH 18/32] fixed pie chart color behaviour --- .../src/components/PieChart/Pie/Pie.tsx | 31 +++++++++++-------- .../src/components/PieChart/Pie/Pie.types.ts | 2 +- .../src/components/PieChart/PieChart.base.tsx | 21 +++++-------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx index 0f273606d7fa8..386916bb97221 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -1,5 +1,6 @@ import * as React from 'react'; import * as shape from 'd3-shape'; +import * as scale from 'd3-scale'; import { IPieProps } from './Pie.types'; import { LabeledArc } from '../Arc/Arc'; import { IArcData } from '../Arc/Arc.types'; @@ -14,34 +15,38 @@ export class Pie extends React.Component { /* eslint-disable @typescript-eslint/no-explicit-any */ .value((d: any) => d.y), }; + private colors: scale.ScaleOrdinal; constructor(props: IPieProps) { super(props); } + public arcGenerator = (d: IArcData, i: number): JSX.Element => { + return ( + + ); + }; + public render(): JSX.Element { // const getClassNames = classNamesFunction(); const { pie, colors, data, width, height, chartTitle } = this.props; + this.colors = scale.scaleOrdinal().range(colors!); + const piechart = pie(data); const translate = `translate(${width / 2}, ${height / 2})`; return ( - - {piechart.map((d: IArcData, i: number) => ( - - ))} - + {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 3434f7e920477..e71a3fec1ec5d 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.types.ts +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.types.ts @@ -25,7 +25,7 @@ export interface IPieProps { /** * colors to render in the Pie. */ - colors: string[]; + colors?: string[]; /** * Title to apply to the whole chart. */ diff --git a/packages/react-charting/src/components/PieChart/PieChart.base.tsx b/packages/react-charting/src/components/PieChart/PieChart.base.tsx index 88289e2e62a02..980130fe96324 100644 --- a/packages/react-charting/src/components/PieChart/PieChart.base.tsx +++ b/packages/react-charting/src/components/PieChart/PieChart.base.tsx @@ -39,19 +39,7 @@ export class PieChartBase extends React.Component = []; + if (data) { + for (let i = 0; i < data.length; i++) { + defaultColors.push(getNextColor(i, 0, theme?.isInverted)); + } + } + const TEXT_MAX_WIDTH = 40; const TEXT_LINE_HEIGHT = 16; @@ -80,7 +75,7 @@ export class PieChartBase extends React.Component getColorFromToken(color)) : defaultColors} chartTitle={chartTitle!} theme={theme} /> From 177e567af8a9cccf4126e43fe9a143de6e037a60 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Mon, 24 Jul 2023 16:18:49 +0530 Subject: [PATCH 19/32] fixed PR issues --- packages/react-charting/src/components/PieChart/Arc/Arc.tsx | 6 +++--- .../react-charting/src/components/PieChart/Arc/Arc.types.ts | 4 ++-- packages/react-charting/src/utilities/SVGTooltipText.tsx | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx index d2a09ad4cbbf3..4dbd5e50ba9a1 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -68,8 +68,8 @@ export class LabeledArc extends Arc { endAngle: data?.endAngle || 0, startAngle: data?.startAngle || 0, padAngle: data?.padAngle, - innerRadius: this.props.outerRadius, - outerRadius: this.props.outerRadius + gap, + innerRadius: this.props?.outerRadius || 0, + outerRadius: this.props?.outerRadius || 0 + gap, }); const getClassNames = classNamesFunction(); @@ -100,7 +100,7 @@ export class LabeledArc extends Arc { 'aria-label': `${data?.data.x}-${convertToLocaleString(data?.data.y, culture)}`, className: classNames.pieText, }} - showTooltip={this.state.isArcFocused} + isTooltipVisibleProp={this.state.isArcFocused} shouldReceiveFocus={false} maxWidth={40} wrapContent={this._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 203810812a8ec..0472caa2ff9a6 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.types.ts +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.types.ts @@ -16,11 +16,11 @@ export interface IArcProps { /** * innerRadius of the Arc. */ - innerRadius: number; + innerRadius?: number; /** * outerRadius of the Arc. */ - outerRadius: number; + outerRadius?: number; /** * Color for the Arc. */ diff --git a/packages/react-charting/src/utilities/SVGTooltipText.tsx b/packages/react-charting/src/utilities/SVGTooltipText.tsx index dad02649cd948..a46919abed29b 100644 --- a/packages/react-charting/src/utilities/SVGTooltipText.tsx +++ b/packages/react-charting/src/utilities/SVGTooltipText.tsx @@ -49,7 +49,7 @@ interface ISVGTooltipTextProps { /** * Pass true to show tooltip directly, false by default */ - showTooltip?: boolean; + isTooltipVisibleProp?: boolean; /** * Function to wrap text within specified width and height @@ -115,7 +115,7 @@ export class SVGTooltipText }; const showTooltip = - (!!this.props.showTooltip && this.state.isOverflowing && !!content) || (isTooltipVisible && !!content); + (!!this.props.isTooltipVisibleProp && this.state.isOverflowing && !!content) || (isTooltipVisible && !!content); return ( <> From 9fe14c665753202d30dc0e197126674c74033859 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Mon, 24 Jul 2023 16:27:11 +0530 Subject: [PATCH 20/32] snapshot updates --- .../__snapshots__/PieChart.test.tsx.snap | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) 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 abd99a8cf1968..60a4ee7645866 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 @@ -69,8 +69,8 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` onMouseEnter={[Function]} onMouseLeave={[Function]} textAnchor="start" - x={160.99949687526205} - y={-0.40249958072932485} + x={158.9995031252588} + y={-0.3974995859376562} > A-50 @@ -113,8 +113,8 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` onMouseEnter={[Function]} onMouseLeave={[Function]} textAnchor="end" - x={-113.41647658644041} - y={114.27030602355721} + x={-112.00757625617408} + y={112.85079911643227} > B-25 @@ -157,8 +157,8 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` onMouseEnter={[Function]} onMouseLeave={[Function]} textAnchor="end" - x={-113.98640803292615} - y={-113.70179762761569} + x={-112.57042780891464} + y={-112.2893529365894} > C-25 @@ -238,8 +238,8 @@ exports[`PieChart snapShot testing renders with colors, width and height data co onMouseEnter={[Function]} onMouseLeave={[Function]} textAnchor="start" - x={135.99957500022134} - y={-0.3399996458334669} + x={133.99958125021809} + y={-0.3349996510417983} > A-50 @@ -282,8 +282,8 @@ exports[`PieChart snapShot testing renders with colors, width and height data co onMouseEnter={[Function]} onMouseLeave={[Function]} textAnchor="end" - x={-95.80522245811116} - y={96.52646968449552} + x={-94.39632212784481} + y={95.1069627773706} > B-25 @@ -326,8 +326,8 @@ exports[`PieChart snapShot testing renders with colors, width and height data co onMouseEnter={[Function]} onMouseLeave={[Function]} textAnchor="end" - x={-96.28665523278234} - y={-96.04623898978716} + x={-94.87067500877083} + y={-94.63379429876088} > C-25 From 8de0c124c5affa4ab8b7a0cb2037c4b6c4dbb9cd Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Tue, 25 Jul 2023 16:50:21 +0530 Subject: [PATCH 21/32] testing with adding the component to the application --- apps/vr-tests/package.json | 1 + .../vr-tests/src/stories/PieChart.stories.tsx | 35 +++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 apps/vr-tests/src/stories/PieChart.stories.tsx diff --git a/apps/vr-tests/package.json b/apps/vr-tests/package.json index cc8c03918903b..f4d70dcecba7b 100644 --- a/apps/vr-tests/package.json +++ b/apps/vr-tests/package.json @@ -27,6 +27,7 @@ "@fluentui/react-hooks": "*", "@fluentui/react-icons-mdl2": "*", "@fluentui/storybook": "*", + "@fluentui/react-charting": "*", "react": "17.0.2", "react-dom": "17.0.2", "tslib": "^2.1.0" diff --git a/apps/vr-tests/src/stories/PieChart.stories.tsx b/apps/vr-tests/src/stories/PieChart.stories.tsx new file mode 100644 index 0000000000000..683fc01fd581f --- /dev/null +++ b/apps/vr-tests/src/stories/PieChart.stories.tsx @@ -0,0 +1,35 @@ +import * as React from 'react'; +import { Steps, StoryWright } from 'storywright'; +import { storiesOf } from '@storybook/react'; +import { TestWrapperDecorator } from '../utilities/index'; +import { PieChart } from '@fluentui/react-charting'; +import { DefaultPalette } from '@fluentui/react/lib/Styling'; + +storiesOf('PieChart', module) + .addDecorator(TestWrapperDecorator) + .addDecorator(story => + // prettier-ignore + + {story()} + , + ) + .addStory('Root', () => { + const points = [ + { y: 50, x: 'A' }, + { y: 25, x: 'B' }, + { y: 25, x: 'C' }, + ]; + const colors = [DefaultPalette.red, DefaultPalette.blue, DefaultPalette.green]; + return ( + + ); + }); From a59ff6cce9f1415b22342104f5d82ffdbcd0cdc3 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Tue, 25 Jul 2023 18:18:57 +0530 Subject: [PATCH 22/32] removed vr test --- apps/vr-tests/package.json | 1 - .../vr-tests/src/stories/PieChart.stories.tsx | 35 ------------------- 2 files changed, 36 deletions(-) delete mode 100644 apps/vr-tests/src/stories/PieChart.stories.tsx diff --git a/apps/vr-tests/package.json b/apps/vr-tests/package.json index f4d70dcecba7b..cc8c03918903b 100644 --- a/apps/vr-tests/package.json +++ b/apps/vr-tests/package.json @@ -27,7 +27,6 @@ "@fluentui/react-hooks": "*", "@fluentui/react-icons-mdl2": "*", "@fluentui/storybook": "*", - "@fluentui/react-charting": "*", "react": "17.0.2", "react-dom": "17.0.2", "tslib": "^2.1.0" diff --git a/apps/vr-tests/src/stories/PieChart.stories.tsx b/apps/vr-tests/src/stories/PieChart.stories.tsx deleted file mode 100644 index 683fc01fd581f..0000000000000 --- a/apps/vr-tests/src/stories/PieChart.stories.tsx +++ /dev/null @@ -1,35 +0,0 @@ -import * as React from 'react'; -import { Steps, StoryWright } from 'storywright'; -import { storiesOf } from '@storybook/react'; -import { TestWrapperDecorator } from '../utilities/index'; -import { PieChart } from '@fluentui/react-charting'; -import { DefaultPalette } from '@fluentui/react/lib/Styling'; - -storiesOf('PieChart', module) - .addDecorator(TestWrapperDecorator) - .addDecorator(story => - // prettier-ignore - - {story()} - , - ) - .addStory('Root', () => { - const points = [ - { y: 50, x: 'A' }, - { y: 25, x: 'B' }, - { y: 25, x: 'C' }, - ]; - const colors = [DefaultPalette.red, DefaultPalette.blue, DefaultPalette.green]; - return ( - - ); - }); From c0a16e0640d71535ddfc93f2439eacb2aefd5674 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Fri, 28 Jul 2023 17:08:36 +0530 Subject: [PATCH 23/32] added pie chart fixes --- .../src/components/PieChart/Arc/Arc.tsx | 2 +- .../src/components/PieChart/Pie/Pie.tsx | 13 +++++++++++-- .../src/components/PieChart/PieChart.base.tsx | 10 +--------- .../react-charting/src/utilities/SVGTooltipText.tsx | 8 ++++++-- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx index 4dbd5e50ba9a1..edaed373749df 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -21,7 +21,7 @@ export class Arc extends React.Component { super(props); this.state = { isArcFocused: false, - id: getId(), + id: getId('pie_chart_arc'), }; } diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx index 386916bb97221..0904f66760e63 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -5,6 +5,7 @@ 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 = { @@ -36,9 +37,17 @@ export class Pie extends React.Component { 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})`; diff --git a/packages/react-charting/src/components/PieChart/PieChart.base.tsx b/packages/react-charting/src/components/PieChart/PieChart.base.tsx index 980130fe96324..4b32dd18a798c 100644 --- a/packages/react-charting/src/components/PieChart/PieChart.base.tsx +++ b/packages/react-charting/src/components/PieChart/PieChart.base.tsx @@ -3,7 +3,6 @@ import { classNamesFunction, getId } from '@fluentui/react/lib/Utilities'; import { IPieChartProps, IPieChartStyleProps, IPieChartStyles } from './PieChart.types'; import { Pie } from './Pie/Pie'; import { IProcessedStyleSet } from '@fluentui/react/lib/Styling'; -import { getColorFromToken, getNextColor } from '../../utilities/colors'; const getClassNames = classNamesFunction(); export interface IPieChartState { @@ -47,13 +46,6 @@ export class PieChartBase extends React.Component = []; - if (data) { - for (let i = 0; i < data.length; i++) { - defaultColors.push(getNextColor(i, 0, theme?.isInverted)); - } - } - const TEXT_MAX_WIDTH = 40; const TEXT_LINE_HEIGHT = 16; @@ -75,7 +67,7 @@ export class PieChartBase extends React.Component getColorFromToken(color)) : defaultColors} + colors={colors!} chartTitle={chartTitle!} theme={theme} /> diff --git a/packages/react-charting/src/utilities/SVGTooltipText.tsx b/packages/react-charting/src/utilities/SVGTooltipText.tsx index a46919abed29b..36673f0a5118d 100644 --- a/packages/react-charting/src/utilities/SVGTooltipText.tsx +++ b/packages/react-charting/src/utilities/SVGTooltipText.tsx @@ -42,12 +42,16 @@ interface ISVGTooltipTextProps { maxHeight?: number; /** - * Pass false to make the data not focusable, though keyboard. true by default + * 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, false by default + * 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; From 1403a05a9ff6d75df83cb789159ff1c78c74b5b9 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Fri, 28 Jul 2023 18:52:01 +0530 Subject: [PATCH 24/32] pr issue resolution --- packages/react-charting/src/components/PieChart/Pie/Pie.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx index 0904f66760e63..e14238bf2b517 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -55,7 +55,9 @@ export class Pie extends React.Component { return ( - {piechart.map((d: IArcData, i: number) => this.arcGenerator(d, i))} + + {piechart.map((d: IArcData, i: number) => this.arcGenerator(d, i))} + ); From 7330024b9f83c3f758388283886c4e066b05d256 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Fri, 28 Jul 2023 18:56:34 +0530 Subject: [PATCH 25/32] updated snapshots --- .../components/PieChart/__snapshots__/PieChart.test.tsx.snap | 2 ++ 1 file changed, 2 insertions(+) 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 60a4ee7645866..d1772539905e7 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 @@ -29,6 +29,7 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` width={600} > Date: Fri, 28 Jul 2023 19:09:38 +0530 Subject: [PATCH 26/32] fixed eslint error --- packages/react-charting/src/utilities/SVGTooltipText.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react-charting/src/utilities/SVGTooltipText.tsx b/packages/react-charting/src/utilities/SVGTooltipText.tsx index 36673f0a5118d..8260cd65b49e1 100644 --- a/packages/react-charting/src/utilities/SVGTooltipText.tsx +++ b/packages/react-charting/src/utilities/SVGTooltipText.tsx @@ -43,14 +43,16 @@ interface ISVGTooltipTextProps { /** * 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 + * 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 + * 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; From b461958d1ea6cfdc5369c71d130e10e8621605f8 Mon Sep 17 00:00:00 2001 From: yush singla Date: Thu, 10 Aug 2023 19:57:32 +0530 Subject: [PATCH 27/32] fixed dark theme bug in pie chart --- packages/react-charting/src/components/PieChart/Pie/Pie.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx index e14238bf2b517..7daea9b6c3d39 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -31,6 +31,7 @@ export class Pie extends React.Component { innerRadius={this.props.innerRadius} outerRadius={this.props.outerRadius} color={`${this.colors(i)}`} + theme={this.props.theme} /> ); }; From 30767f9f78adcd5f1a273ac116e25fb7a0b0bed1 Mon Sep 17 00:00:00 2001 From: yush singla Date: Fri, 11 Aug 2023 13:01:25 +0530 Subject: [PATCH 28/32] added mock function to snapshot test for pie chart --- ...-7ed503a2-d9d1-4679-8074-c67eb83161e1.json | 2 +- .../src/components/PieChart/Arc/Arc.tsx | 23 +- .../components/PieChart/PieChartRTL.test.tsx | 6 + .../__snapshots__/PieChart.test.tsx.snap | 28 +- .../__snapshots__/PieChartRTL.test.tsx.snap | 546 ++++++++++++------ .../react-charting/src/utilities/utilities.ts | 18 + 6 files changed, 424 insertions(+), 199 deletions(-) diff --git a/change/@fluentui-react-charting-7ed503a2-d9d1-4679-8074-c67eb83161e1.json b/change/@fluentui-react-charting-7ed503a2-d9d1-4679-8074-c67eb83161e1.json index bfdfd748222f2..2695291386c7f 100644 --- a/change/@fluentui-react-charting-7ed503a2-d9d1-4679-8074-c67eb83161e1.json +++ b/change/@fluentui-react-charting-7ed503a2-d9d1-4679-8074-c67eb83161e1.json @@ -2,6 +2,6 @@ "type": "patch", "comment": "fixed accessibility issues in pie chart", "packageName": "@fluentui/react-charting", - "email": "email not defined", + "email": "yushsingla@microsoft.com", "dependentChangeType": "patch" } diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx index edaed373749df..a54b00c5de86f 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -3,9 +3,8 @@ import * as shape from 'd3-shape'; import { IArcProps, IArcState, IArcStyles } from './Arc.types'; import { classNamesFunction, getId } from '@fluentui/react/lib/Utilities'; import { getStyles } from './Arc.styles'; -import { convertToLocaleString } from '../../../utilities/utilities'; +import { shouldWrapContent, convertToLocaleString } from '../../../utilities/utilities'; import { SVGTooltipText } from '../../../utilities/SVGTooltipText'; -import { select as d3Select } from 'd3-selection'; export class Arc extends React.Component { public static defaultProps: Partial = { @@ -103,29 +102,11 @@ export class LabeledArc extends Arc { isTooltipVisibleProp={this.state.isArcFocused} shouldReceiveFocus={false} maxWidth={40} - wrapContent={this._wrapContent} + wrapContent={shouldWrapContent} /> ); } - - private _wrapContent = (content: string, id: string, maxWidth: number) => { - const textElement = d3Select(`#${id}`); - textElement.text(content); - if (!textElement.node()) { - return false; - } - - let isOverflowing = false; - let textLength = textElement.node()!.getComputedTextLength(); - while (textLength > maxWidth && content.length > 0) { - content = content.slice(0, -1); - textElement.text(content + '...'); - isOverflowing = true; - textLength = textElement.node()!.getComputedTextLength(); - } - return isOverflowing; - }; } function _updateChart(newProps: IArcProps): void { diff --git a/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx b/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx index 3d5cd25ebe576..f6c40885272ec 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 shouldWrapContent as it internally calls a Browser Function like + // getComputedTextLength() which will otherwise lead to a crash if mounted + jest.spyOn(utils, 'shouldWrapContent').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 d1772539905e7..9b4762b09a5e5 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 @@ -19,7 +19,7 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` &:focus { outline: none; } - data-focuszone-id="FocusZone0" + data-focuszone-id="FocusZone1" onFocus={[Function]} onKeyDown={[Function]} onMouseDownCapture={[Function]} @@ -40,7 +40,7 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` outline: none; } data-is-focusable={true} - id="id__2" + id="id__3" onBlur={[Function]} onFocus={[Function]} role="img" @@ -63,7 +63,7 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` } data-is-focusable={false} dominantBaseline="auto" - id="tooltip-host3" + id="tooltip-host4" onBlurCapture={[Function]} onFocusCapture={[Function]} onKeyDown={[Function]} @@ -84,7 +84,7 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` outline: none; } data-is-focusable={true} - id="id__5" + id="id__6" onBlur={[Function]} onFocus={[Function]} role="img" @@ -107,7 +107,7 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` } data-is-focusable={false} dominantBaseline="hanging" - id="tooltip-host6" + id="tooltip-host7" onBlurCapture={[Function]} onFocusCapture={[Function]} onKeyDown={[Function]} @@ -128,7 +128,7 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` outline: none; } data-is-focusable={true} - id="id__8" + id="id__9" onBlur={[Function]} onFocus={[Function]} role="img" @@ -151,7 +151,7 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` } data-is-focusable={false} dominantBaseline="auto" - id="tooltip-host9" + id="tooltip-host10" onBlurCapture={[Function]} onFocusCapture={[Function]} onKeyDown={[Function]} @@ -189,7 +189,7 @@ exports[`PieChart snapShot testing renders with colors, width and height data co &:focus { outline: none; } - data-focuszone-id="FocusZone10" + data-focuszone-id="FocusZone12" onFocus={[Function]} onKeyDown={[Function]} onMouseDownCapture={[Function]} @@ -210,7 +210,7 @@ exports[`PieChart snapShot testing renders with colors, width and height data co outline: none; } data-is-focusable={true} - id="id__12" + id="id__14" onBlur={[Function]} onFocus={[Function]} role="img" @@ -233,7 +233,7 @@ exports[`PieChart snapShot testing renders with colors, width and height data co } data-is-focusable={false} dominantBaseline="auto" - id="tooltip-host13" + id="tooltip-host15" onBlurCapture={[Function]} onFocusCapture={[Function]} onKeyDown={[Function]} @@ -254,7 +254,7 @@ exports[`PieChart snapShot testing renders with colors, width and height data co outline: none; } data-is-focusable={true} - id="id__15" + id="id__17" onBlur={[Function]} onFocus={[Function]} role="img" @@ -277,7 +277,7 @@ exports[`PieChart snapShot testing renders with colors, width and height data co } data-is-focusable={false} dominantBaseline="hanging" - id="tooltip-host16" + id="tooltip-host18" onBlurCapture={[Function]} onFocusCapture={[Function]} onKeyDown={[Function]} @@ -298,7 +298,7 @@ exports[`PieChart snapShot testing renders with colors, width and height data co outline: none; } data-is-focusable={true} - id="id__18" + id="id__20" onBlur={[Function]} onFocus={[Function]} role="img" @@ -321,7 +321,7 @@ exports[`PieChart snapShot testing renders with colors, width and height data co } data-is-focusable={false} dominantBaseline="auto" - id="tooltip-host19" + id="tooltip-host21" onBlurCapture={[Function]} onFocusCapture={[Function]} onKeyDown={[Function]} 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 e730fc97eced3..418a5b7b240a9 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/utilities.ts b/packages/react-charting/src/utilities/utilities.ts index 54488a1f10bbf..b0e1cf2807217 100644 --- a/packages/react-charting/src/utilities/utilities.ts +++ b/packages/react-charting/src/utilities/utilities.ts @@ -1390,6 +1390,24 @@ export function wrapTextInsideDonut(selectorClass: string, maxWidth: number) { }); } +export function shouldWrapContent(content: string, id: string, maxWidth: number) { + const textElement = d3Select(`#${id}`); + textElement.text(content); + if (!textElement.node()) { + return false; + } + + let isOverflowing = false; + let textLength = textElement.node()!.getComputedTextLength(); + while (textLength > maxWidth && content.length > 0) { + content = content.slice(0, -1); + textElement.text(content + '...'); + isOverflowing = true; + textLength = textElement.node()!.getComputedTextLength(); + } + return isOverflowing; +} + export function formatValueWithSIPrefix(value: number) { let specifier: string; if (value < 1000) { From e4fbf26a4cf20369f855b47d8568576c3306e0eb Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Wed, 16 Aug 2023 13:39:40 +0530 Subject: [PATCH 29/32] fixing pr comments --- .../src/components/PieChart/Arc/Arc.styles.ts | 8 +++---- .../src/components/PieChart/Arc/Arc.tsx | 21 ++++++++++--------- .../src/components/PieChart/Arc/Arc.types.ts | 12 ++++------- .../src/components/PieChart/Pie/Pie.tsx | 2 +- .../components/PieChart/PieChartRTL.test.tsx | 2 +- .../react-charting/src/utilities/utilities.ts | 18 ---------------- 6 files changed, 21 insertions(+), 42 deletions(-) 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 94f1a9a1c0ad0..845797bf73a61 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.styles.ts +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.styles.ts @@ -3,9 +3,9 @@ import { DefaultPalette, ITheme } from '@fluentui/react/lib/Styling'; export const getStyles = (props: IArcProps, theme: ITheme | undefined): IArcStyles => { const { color } = props; return { - pieRoot: { fill: color }, - pieRootFocused: { fill: color, stroke: theme?.palette.black || DefaultPalette.black, strokeWidth: 2 }, - pie: { outline: 'none' }, - pieText: { fill: theme?.palette.black || DefaultPalette.black, outline: 'none' }, + 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 a54b00c5de86f..fa03fe8d4c6da 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -3,7 +3,7 @@ import * as shape from 'd3-shape'; import { IArcProps, IArcState, IArcStyles } from './Arc.types'; import { classNamesFunction, getId } from '@fluentui/react/lib/Utilities'; import { getStyles } from './Arc.styles'; -import { shouldWrapContent, convertToLocaleString } from '../../../utilities/utilities'; +import { wrapContent, convertToLocaleString } from '../../../utilities/utilities'; import { SVGTooltipText } from '../../../utilities/SVGTooltipText'; export class Arc extends React.Component { @@ -11,6 +11,8 @@ export class Arc extends React.Component { arc: shape.arc(), }; + protected _arcId: string; + public static getDerivedStateFromProps(nextProps: Readonly): null { _updateChart(nextProps); return null; @@ -20,8 +22,9 @@ export class Arc extends React.Component { super(props); this.state = { isArcFocused: false, - id: getId('pie_chart_arc'), }; + + this._arcId = getId('piechart_arc'); } public updateChart = (newProps: IArcProps) => { @@ -36,7 +39,7 @@ export class Arc extends React.Component { return ( ); @@ -54,9 +57,7 @@ export class Arc extends React.Component { export class LabeledArc extends Arc { public constructor(props: IArcProps) { super(props); - this.state = { - id: getId(), - }; + this._arcId = getId('piechart_arc'); } public render(): JSX.Element { @@ -80,9 +81,9 @@ export class LabeledArc extends Arc { return ( Math.PI / 2 && angle < (3 * Math.PI) / 2 ? 'hanging' : 'auto', textAnchor: angle > Math.PI ? 'end' : 'start', 'aria-label': `${data?.data.x}-${convertToLocaleString(data?.data.y, culture)}`, - className: classNames.pieText, + className: classNames.arcText, }} isTooltipVisibleProp={this.state.isArcFocused} shouldReceiveFocus={false} maxWidth={40} - wrapContent={shouldWrapContent} + 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 0472caa2ff9a6..7064f9e17a45e 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.types.ts +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.types.ts @@ -41,10 +41,6 @@ export interface IArcState { * The state controls, whether the arc needs to be focused or not */ isArcFocused?: boolean; - /** - * Unique Identifier for each arc - */ - id: string; } export interface IArcData { @@ -78,11 +74,11 @@ export interface IArcStyles { /** * Style set for the card header component root */ - pieRoot: IStyle; + arcRoot: IStyle; - pie: IStyle; + arc: IStyle; - pieRootFocused: IStyle; + arcRootFocussed: IStyle; - pieText: 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 7daea9b6c3d39..3f694880a24a5 100644 --- a/packages/react-charting/src/components/PieChart/Pie/Pie.tsx +++ b/packages/react-charting/src/components/PieChart/Pie/Pie.tsx @@ -56,7 +56,7 @@ export class Pie extends React.Component { return ( - + {piechart.map((d: IArcData, i: number) => this.arcGenerator(d, i))} diff --git a/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx b/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx index f6c40885272ec..82ec8b688d17f 100644 --- a/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx +++ b/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx @@ -15,7 +15,7 @@ describe('Pie chart rendering', () => { // Mock the implementation of shouldWrapContent as it internally calls a Browser Function like // getComputedTextLength() which will otherwise lead to a crash if mounted - jest.spyOn(utils, 'shouldWrapContent').mockImplementation(() => false); + jest.spyOn(utils, 'wrapContent').mockImplementation(() => false); // Act rerender(); diff --git a/packages/react-charting/src/utilities/utilities.ts b/packages/react-charting/src/utilities/utilities.ts index 3fa37c778bc4b..7d924c721310f 100644 --- a/packages/react-charting/src/utilities/utilities.ts +++ b/packages/react-charting/src/utilities/utilities.ts @@ -1408,24 +1408,6 @@ export function wrapTextInsideDonut(selectorClass: string, maxWidth: number) { }); } -export function shouldWrapContent(content: string, id: string, maxWidth: number) { - const textElement = d3Select(`#${id}`); - textElement.text(content); - if (!textElement.node()) { - return false; - } - - let isOverflowing = false; - let textLength = textElement.node()!.getComputedTextLength(); - while (textLength > maxWidth && content.length > 0) { - content = content.slice(0, -1); - textElement.text(content + '...'); - isOverflowing = true; - textLength = textElement.node()!.getComputedTextLength(); - } - return isOverflowing; -} - export function formatValueWithSIPrefix(value: number) { let specifier: string; if (value < 1000) { From 6fdafce3c3f4df52b7508620cd0ce70ed986baba Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Wed, 16 Aug 2023 14:11:48 +0530 Subject: [PATCH 30/32] fixed RTL bug --- packages/react-charting/src/components/PieChart/Arc/Arc.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx index fa03fe8d4c6da..9791cfcab7c2e 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import * as shape from 'd3-shape'; import { IArcProps, IArcState, IArcStyles } from './Arc.types'; -import { classNamesFunction, getId } from '@fluentui/react/lib/Utilities'; +import { classNamesFunction, getId, getRTL } from '@fluentui/react/lib/Utilities'; import { getStyles } from './Arc.styles'; import { wrapContent, convertToLocaleString } from '../../../utilities/utilities'; import { SVGTooltipText } from '../../../utilities/SVGTooltipText'; @@ -55,6 +55,8 @@ export class Arc extends React.Component { } export class LabeledArc extends Arc { + private _isRTL = getRTL(); + public constructor(props: IArcProps) { super(props); this._arcId = getId('piechart_arc'); @@ -96,7 +98,7 @@ export class LabeledArc extends Arc { x: labelX, y: labelY, dominantBaseline: angle > Math.PI / 2 && angle < (3 * Math.PI) / 2 ? 'hanging' : 'auto', - textAnchor: angle > Math.PI ? 'end' : 'start', + 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, }} From 1ae4489e4314ba2b8731cef763c0301351ff6956 Mon Sep 17 00:00:00 2001 From: Yush Singla Date: Wed, 16 Aug 2023 16:49:08 +0530 Subject: [PATCH 31/32] updated snapshots --- .../components/PieChart/PieChartRTL.test.tsx | 2 +- .../__snapshots__/PieChart.test.tsx.snap | 16 ++++++------- .../__snapshots__/PieChartRTL.test.tsx.snap | 24 +++++++++---------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx b/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx index 82ec8b688d17f..368b785f66405 100644 --- a/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx +++ b/packages/react-charting/src/components/PieChart/PieChartRTL.test.tsx @@ -13,7 +13,7 @@ describe('Pie chart rendering', () => { expect(container).toMatchSnapshot(); expect(getById(container, /_PieChart_empty/i)).toHaveLength(1); - // Mock the implementation of shouldWrapContent as it internally calls a Browser Function like + // 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); 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 9b4762b09a5e5..60399751dcec6 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 @@ -29,7 +29,7 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` width={600} > @@ -90,7 +90,7 @@ exports[`Pie chart rendering Should re-render the Pie chart with data 2`] = ` outline: none; } data-is-focusable="true" - id="id__51" + id="piechart_arc51" role="img" tabindex="-1" > @@ -129,7 +129,7 @@ exports[`Pie chart rendering Should re-render the Pie chart with data 2`] = ` outline: none; } data-is-focusable="true" - id="id__54" + id="piechart_arc54" role="img" tabindex="-1" > @@ -196,7 +196,7 @@ exports[`PieChart snapShot testing renders PieChart correctly 1`] = ` width={600} > Date: Thu, 24 Aug 2023 12:54:17 +0530 Subject: [PATCH 32/32] added classname arc --- .../src/components/PieChart/Arc/Arc.tsx | 2 +- .../__snapshots__/PieChart.test.tsx.snap | 12 ++++++------ .../__snapshots__/PieChartRTL.test.tsx.snap | 18 +++++++++--------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx index 9791cfcab7c2e..0e590743d7922 100644 --- a/packages/react-charting/src/components/PieChart/Arc/Arc.tsx +++ b/packages/react-charting/src/components/PieChart/Arc/Arc.tsx @@ -83,7 +83,7 @@ export class LabeledArc extends Arc { return (