Skip to content

Commit

Permalink
Cherry-pick fix for wrong narration when legend selected (#25936)
Browse files Browse the repository at this point in the history
* fix wrong narration when legend selected

* Change files

* rename state

* hide callout on legend hover
  • Loading branch information
krkshitij committed Jan 2, 2023
1 parent 64f0a87 commit 59383dc
Show file tree
Hide file tree
Showing 19 changed files with 332 additions and 168 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix wrong narration when legend selected",
"packageName": "@uifabric/charting",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@ export class AreaChartBase extends React.Component<IAreaChartProps, IAreaChartSt
this._onLegendClick(singleChartData.legend);
},
hoverAction: () => {
this._handleChartMouseLeave();
this._onLegendHover(singleChartData.legend);
},
onMouseOutAction: (isLegendSelected?: boolean) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,9 @@ exports[`AreaChart snapShot testing renders Areachart correctly 1`] = `
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;

Expand Down Expand Up @@ -673,6 +676,9 @@ exports[`AreaChart snapShot testing renders enabledLegendsWrapLines correctly 1`
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;

Expand Down Expand Up @@ -855,6 +861,9 @@ exports[`AreaChart snapShot testing renders hideLegend hhh correctly 1`] = `
</g>
</svg>
</div>
<span
className="ms-layer"
/>
</div>
`;

Expand Down Expand Up @@ -1203,6 +1212,9 @@ exports[`AreaChart snapShot testing renders hideTooltip correctly 1`] = `
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;

Expand Down Expand Up @@ -1551,6 +1563,9 @@ exports[`AreaChart snapShot testing renders showXAxisLablesTooltip correctly 1`]
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;

Expand Down Expand Up @@ -1899,6 +1914,9 @@ exports[`AreaChart snapShot testing renders wrapXAxisLables correctly 1`] = `
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;

Expand Down Expand Up @@ -2247,5 +2265,8 @@ exports[`AreaChart snapShot testing renders yAxisTickFormat correctly 1`] = `
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -281,25 +281,29 @@ export class CartesianChartBase extends React.Component<IModifiedCartesianChartP
{this.props.legendBars}
</div>
)}
{!this.props.hideTooltip && calloutProps!.isCalloutVisible && (
<Callout {...calloutProps}>
{/** Given custom callout, then it will render */}
{this.props.customizedCallout && this.props.customizedCallout}
{/** single x point its corresponding y points of all the bars/lines in chart will render in callout */}
{!this.props.customizedCallout && this.props.isCalloutForStack && this._multiValueCallout(calloutProps)}
{/** single x point its corresponding y point of single line/bar in the chart will render in callout */}
{!this.props.customizedCallout && !this.props.isCalloutForStack && (
<ChartHoverCard
XValue={calloutProps.XValue}
Legend={calloutProps.legend!}
YValue={calloutProps.YValue!}
color={calloutProps.color!}
culture={this.props.culture}
{...chartHoverProps}
/>
)}
</Callout>
)}
{/** The callout is used for narration, so keep it mounted on the DOM */}
<Callout
hidden={!(!this.props.hideTooltip && calloutProps!.isCalloutVisible)}
/** Keep the callout updated with details of focused/hovered chart element */
shouldUpdateWhenHidden={true}
{...calloutProps}
>
{/** Given custom callout, then it will render */}
{this.props.customizedCallout && this.props.customizedCallout}
{/** single x point its corresponding y points of all the bars/lines in chart will render in callout */}
{!this.props.customizedCallout && this.props.isCalloutForStack && this._multiValueCallout(calloutProps)}
{/** single x point its corresponding y point of single line/bar in the chart will render in callout */}
{!this.props.customizedCallout && !this.props.isCalloutForStack && (
<ChartHoverCard
XValue={calloutProps.XValue}
Legend={calloutProps.legend!}
YValue={calloutProps.YValue!}
color={calloutProps.color!}
culture={this.props.culture}
{...chartHoverProps}
/>
)}
</Callout>
</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export const getStyles = (props: IArcProps): IArcStyles => {
focusRing: {
stroke: theme.semanticColors.focusBorder,
strokeWidth: 4,
fill: 'transparent',
},
insideDonutString: {
fontSize: FontSizes.large,
Expand Down
4 changes: 1 addition & 3 deletions packages/charting/src/components/DonutChart/Arc/Arc.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ export class Arc extends React.Component<IArcProps, IArcState> {

private _hoverOn(data: IChartDataPoint, mouseEvent: React.MouseEvent<SVGPathElement>): void {
mouseEvent.persist();
if (this.props.activeArc === this.props.data!.data.legend || this.props.activeArc === '') {
this.props.hoverOnCallback!(data, mouseEvent);
}
this.props.hoverOnCallback!(data, mouseEvent);
}

private _hoverOff = (): void => {
Expand Down
17 changes: 9 additions & 8 deletions packages/charting/src/components/DonutChart/DonutChart.base.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ export class DonutChartBase extends React.Component<IDonutChartProps, IDonutChar
id={this._calloutId}
onDismiss={this._closeCallout}
preventDismissOnLostFocus={true}
/** Keep the callout updated with details of focused/hovered arc */
shouldUpdateWhenHidden={true}
{...this.props.calloutProps!}
{...getAccessibleDataObject(this.state.callOutAccessibilityData, 'text', false)}
>
Expand Down Expand Up @@ -206,6 +208,7 @@ export class DonutChartBase extends React.Component<IDonutChartProps, IDonutChar
}
},
hoverAction: () => {
this._handleChartMouseLeave();
if (this.state.activeLegend !== point.legend || this.state.activeLegend === '') {
this.setState({ activeLegend: point.legend! });
} else {
Expand Down Expand Up @@ -238,12 +241,11 @@ export class DonutChartBase extends React.Component<IDonutChartProps, IDonutChar
private _focusCallback = (data: IChartDataPoint, id: string, element: SVGPathElement): void => {
this._currentHoverElement = element;
this.setState({
showHover: true,
/** Show the callout if highlighted arc is focused and Hide it if unhighlighted arc is focused */
showHover: this.state.activeLegend === data.legend || this.state.activeLegend === '',
value: data.data!.toString(),
legend: data.legend,
activeLegend: data.legend,
color: data.color!,
selectedLegend: data.legend!,
xCalloutValue: data.xAxisCalloutData!,
yCalloutValue: data.yAxisCalloutData!,
focusedArcId: id,
Expand All @@ -257,25 +259,24 @@ export class DonutChartBase extends React.Component<IDonutChartProps, IDonutChar
this._calloutAnchorPoint = data;
this._currentHoverElement = e;
this.setState({
showHover: true,
/** Show the callout if highlighted arc is hovered and Hide it if unhighlighted arc is hovered */
showHover: this.state.activeLegend === data.legend || this.state.activeLegend === '',
value: data.data!.toString(),
selectedLegend: data.legend!,
legend: data.legend,
color: data.color!,
xCalloutValue: data.xAxisCalloutData!,
yCalloutValue: data.yAxisCalloutData!,
activeLegend: data.legend,
dataPointCalloutProps: data,
callOutAccessibilityData: data.callOutAccessibilityData!,
});
}
};
private _onBlur = (): void => {
this.setState({ showHover: false, focusedArcId: '', activeLegend: '', selectedLegend: 'none' });
this.setState({ focusedArcId: '' });
};

private _hoverLeave(): void {
this.setState({ activeLegend: '', selectedLegend: 'none', focusedArcId: '' });
/**/
}

private _handleChartMouseLeave = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export interface IGroupedVerticalBarChartState extends IBasestate {
titleForHoverCard: string;
dataPointCalloutProps?: IGVBarChartSeriesPoint;
callOutAccessibilityData?: IAccessibilityProps;
calloutLegend: string;
}

export class GroupedVerticalBarChartBase extends React.Component<
Expand Down Expand Up @@ -98,6 +99,7 @@ export class GroupedVerticalBarChartBase extends React.Component<
yCalloutValue: '',
YValueHover: [],
hoverXValue: '',
calloutLegend: '',
};
warnDeprecations(COMPONENT_NAME, props, {
showYAxisGridLines: 'Dont use this property. Lines are drawn by default',
Expand Down Expand Up @@ -136,7 +138,7 @@ export class GroupedVerticalBarChartBase extends React.Component<
isBeakVisible: false,
setInitialFocus: true,
color: this.state.color,
Legend: this.state.titleForHoverCard,
Legend: this.state.calloutLegend,
XValue: this.state.xCalloutValue,
YValue: this.state.yCalloutValue ? this.state.yCalloutValue : this.state.dataForHoverCard,
YValueHover: this.state.YValueHover,
Expand Down Expand Up @@ -233,16 +235,15 @@ export class GroupedVerticalBarChartBase extends React.Component<
mouseEvent: React.MouseEvent<SVGElement>,
): void => {
mouseEvent.persist();
if (
(this.state.isLegendSelected === false ||
(this.state.isLegendSelected && this.state.titleForHoverCard === pointData.legend)) &&
this._calloutAnchorPoint !== pointData
) {
if (this._calloutAnchorPoint !== pointData) {
this._calloutAnchorPoint = pointData;
this.setState({
refSelected: mouseEvent,
isCalloutVisible: true,
titleForHoverCard: pointData.legend,
/** Show the callout if highlighted bar is hovered and Hide it if unhighlighted bar is hovered */
isCalloutVisible:
this.state.isLegendSelected === false ||
(this.state.isLegendSelected === true && this.state.titleForHoverCard === pointData.legend),
calloutLegend: pointData.legend,
dataForHoverCard: pointData.data,
color: pointData.color,
xCalloutValue: pointData.xAxisCalloutData,
Expand Down Expand Up @@ -272,30 +273,28 @@ export class GroupedVerticalBarChartBase extends React.Component<
groupData: any,
refArrayIndexNumber: number,
): void => {
if (
this.state.isLegendSelected === false ||
(this.state.isLegendSelected && this.state.titleForHoverCard === pointData.legend)
) {
this._refArray.forEach((obj: IRefArrayData, index: number) => {
if (obj.index === pointData.legend && refArrayIndexNumber === index) {
this.setState({
refSelected: obj.refElement,
isCalloutVisible: true,
titleForHoverCard: pointData.legend,
dataForHoverCard: pointData.data,
color: pointData.color,
xCalloutValue: pointData.xAxisCalloutData,
yCalloutValue: pointData.yAxisCalloutData,
dataPointCalloutProps: pointData,
callOutAccessibilityData: this.props.isCalloutForStack
? groupData.stackCallOutAccessibilityData
: pointData.callOutAccessibilityData,
YValueHover: groupData.groupSeries,
hoverXValue: pointData.xAxisCalloutData,
});
}
});
}
this._refArray.forEach((obj: IRefArrayData, index: number) => {
if (obj.index === pointData.legend && refArrayIndexNumber === index) {
this.setState({
refSelected: obj.refElement,
/** Show the callout if highlighted bar is focused and Hide it if unhighlighted bar is focused */
isCalloutVisible:
this.state.isLegendSelected === false ||
(this.state.isLegendSelected === true && this.state.titleForHoverCard === pointData.legend),
calloutLegend: pointData.legend,
dataForHoverCard: pointData.data,
color: pointData.color,
xCalloutValue: pointData.xAxisCalloutData,
yCalloutValue: pointData.yAxisCalloutData,
dataPointCalloutProps: pointData,
callOutAccessibilityData: this.props.isCalloutForStack
? groupData.stackCallOutAccessibilityData
: pointData.callOutAccessibilityData,
YValueHover: groupData.groupSeries,
hoverXValue: pointData.xAxisCalloutData,
});
}
});
};

private _redirectToUrl = (href: string | undefined): void => {
Expand Down Expand Up @@ -507,6 +506,7 @@ export class GroupedVerticalBarChartBase extends React.Component<
this._onLegendClick(point.legend);
},
hoverAction: () => {
this._handleChartMouseLeave();
this._onLegendHover(point.legend);
},
onMouseOutAction: (isLegendSelected?: boolean) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,9 @@ exports[`GroupedVerticalBarChart snapShot testing renders GroupedVerticalBarChar
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;

Expand Down Expand Up @@ -1141,6 +1144,9 @@ exports[`GroupedVerticalBarChart snapShot testing renders enabledLegendsWrapLine
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;

Expand Down Expand Up @@ -1379,6 +1385,9 @@ exports[`GroupedVerticalBarChart snapShot testing renders hideLegend correctly 1
</g>
</svg>
</div>
<span
className="ms-layer"
/>
</div>
`;

Expand Down Expand Up @@ -1961,6 +1970,9 @@ exports[`GroupedVerticalBarChart snapShot testing renders hideTooltip correctly
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;

Expand Down Expand Up @@ -2543,6 +2555,9 @@ exports[`GroupedVerticalBarChart snapShot testing renders showXAxisLablesTooltip
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;

Expand Down Expand Up @@ -3125,6 +3140,9 @@ exports[`GroupedVerticalBarChart snapShot testing renders wrapXAxisLables correc
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;

Expand Down Expand Up @@ -3707,5 +3725,8 @@ exports[`GroupedVerticalBarChart snapShot testing renders yAxisTickFormat correc
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;
Loading

0 comments on commit 59383dc

Please sign in to comment.