Skip to content

Fix wrong narration when legend selected #24903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
krkshitij merged 6 commits into from
Oct 25, 2022
Merged

Fix wrong narration when legend selected #24903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c918d7f
fix wrong narration when legend selected
krkshitij Sep 22, 2022
99aaee1
add change file
krkshitij Sep 22, 2022
17bb0ff
show/hide callout instead of mount/unmount
krkshitij Oct 13, 2022
3bda6cb
fill transparent inside focusRing
krkshitij Oct 13, 2022
3eef252
minor
krkshitij Oct 17, 2022
0d31915
add comments
krkshitij Oct 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/@fluentui-react-charting-1216d12c-7b19-41b7-bbde-99ed3b330d78.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix wrong narration when legend selected",
"packageName": "@fluentui/react-charting",
"email": "[email protected]",
"dependentChangeType": "patch"
}
24 changes: 24 additions & 0 deletions packages/react-charting/src/components/AreaChart/__snapshots__/AreaChart.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ exports[`AreaChart - mouse events Should render callout correctly on mouseover 1
border: 0px;
}
data-is-focusable={true}
hidden={false}
id="toolTipundefined"
style={
Object {
Expand Down Expand Up @@ -907,6 +908,7 @@ exports[`AreaChart - mouse events Should render customized callout on mouseover
border: 0px;
}
data-is-focusable={true}
hidden={false}
id="toolTipundefined"
style={
Object {
Expand Down Expand Up @@ -1335,6 +1337,7 @@ exports[`AreaChart - mouse events Should render customized callout per stack on
border: 0px;
}
data-is-focusable={true}
hidden={false}
id="toolTipundefined"
style={
Object {
Expand Down Expand Up @@ -1725,6 +1728,9 @@ exports[`AreaChart snapShot testing renders Areachart correctly 1`] = `
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;

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

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

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

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

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

Expand Down Expand Up @@ -3592,5 +3613,8 @@ exports[`AreaChart snapShot testing renders yAxisTickFormat correctly 1`] = `
</div>
</div>
</div>
<span
className="ms-layer"
/>
</div>
`;
42 changes: 23 additions & 19 deletions packages/react-charting/src/components/CommonComponents/CartesianChart.base.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,25 +318,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
1 change: 1 addition & 0 deletions packages/react-charting/src/components/DonutChart/Arc/Arc.styles.ts
View file
Open in desktop
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/react-charting/src/components/DonutChart/Arc/Arc.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,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 === '') {
Copy link
Contributor

@AtishayMsft AtishayMsft Oct 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove these checks

Copy link
Contributor Author

@krkshitij krkshitij Oct 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These checks were not completely removed but used in the setState function call to update the visibility of the callout

this.props.hoverOnCallback!(data, mouseEvent);
}
this.props.hoverOnCallback!(data, mouseEvent);
}

private _hoverOff = (): void => {
Expand Down
16 changes: 8 additions & 8 deletions packages/react-charting/src/components/DonutChart/DonutChart.base.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,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 @@ -237,12 +239,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 @@ -256,25 +257,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
8 changes: 4 additions & 4 deletions packages/react-charting/src/components/DonutChart/__snapshots__/DonutChart.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ exports[`DonutChart - mouse events Should render callout correctly on mouseover
onMouseLeave={[Function]}
onMouseMove={[Function]}
onMouseOver={[Function]}
opacity={0.1}
opacity={1}
role="text"
/>
<text
Expand Down Expand Up @@ -364,7 +364,7 @@ exports[`DonutChart - mouse events Should render callout correctly on mouseover
<button
aria-label="first selected"
aria-posinset={1}
aria-selected={true}
aria-selected={false}
aria-setsize={2}
className=

Expand Down Expand Up @@ -649,7 +649,7 @@ exports[`DonutChart - mouse events Should render customized callout on mouseover
onMouseLeave={[Function]}
onMouseMove={[Function]}
onMouseOver={[Function]}
opacity={0.1}
opacity={1}
role="text"
/>
<text
Expand Down Expand Up @@ -825,7 +825,7 @@ exports[`DonutChart - mouse events Should render customized callout on mouseover
<button
aria-label="first selected"
aria-posinset={1}
aria-selected={true}
aria-selected={false}
aria-setsize={2}
className=

Expand Down
63 changes: 31 additions & 32 deletions ...es/react-charting/src/components/GroupedVerticalBarChart/GroupedVerticalBarChart.base.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export interface IGroupedVerticalBarChartState extends IBasestate {
titleForHoverCard: string;
dataPointCalloutProps?: IGVBarChartSeriesPoint;
callOutAccessibilityData?: IAccessibilityProps;
calloutLegend: string;
}

export class GroupedVerticalBarChartBase extends React.Component<
Expand Down Expand Up @@ -92,6 +93,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 @@ -130,7 +132,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 @@ -227,16 +229,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 @@ -266,30 +267,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
Loading