Skip to content
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

[Donut] Legends multi selection for Donut Charts #33447

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

[Donut] Legends multi selection for Donut Charts #33447

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
b0c7663
Legends multi selection for Donut Charts
srmukher Dec 11, 2024
24ce6bf
Making legend multi selection optional
srmukher Dec 12, 2024
434fc6d
Showing total values when multiple legends are selected
srmukher Dec 12, 2024
70b0404
Separating hover and multi selection and adding example
srmukher Dec 12, 2024
addca43
Merge branch 'master' into users/srmukher/LegendsMultiSel
srmukher Dec 12, 2024
7293f33
Taking multiple legend selection from legend component callback
srmukher Dec 13, 2024
ae348e2
Merge branch 'users/srmukher/LegendsMultiSel' of https://github.com/m…
srmukher Dec 13, 2024
88eb247
Modifying handlers
srmukher Dec 17, 2024
db555d3
Merge branch 'master' of https://github.com/microsoft/fluentui into u…
srmukher Dec 17, 2024
4fdc773
Merge branch 'master' into users/srmukher/LegendsMultiSel
srmukher Dec 17, 2024
a1dc7a9
Merge branch 'master' into users/srmukher/LegendsMultiSel
srmukher Dec 18, 2024
13876f3
Removing canSelectMultipleLegends prop
srmukher Dec 18, 2024
cea2d29
Rearranging states and handlers
srmukher Dec 18, 2024
be186ba
Fixing donut labels
srmukher Dec 19, 2024
d4641c5
Add change file
srmukher Dec 19, 2024
0e62c96
Merge branch 'master' into users/srmukher/LegendsMultiSel
srmukher Dec 20, 2024
568ca4e
Fixing tests
srmukher Dec 20, 2024
36e6bdc
Merge branch 'users/srmukher/LegendsMultiSel' of https://github.com/m…
srmukher Dec 20, 2024
a94e9f1
Adding and updating tests
srmukher Dec 23, 2024
cfd09ea
Moving checks inside function
srmukher Dec 23, 2024
0e7fc13
Merge branch 'master' into users/srmukher/LegendsMultiSel
srmukher Dec 23, 2024
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
1 change: 1 addition & 0 deletions packages/charts/react-charting/etc/react-charting.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ export interface IDonutChart {
// @public
export interface IDonutChartProps extends ICartesianChartProps {
calloutProps?: Partial<ICalloutProps>;
canSelectMultipleLegends?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

canSelectMultipleLegends

revert this change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

culture?: string;
data?: IChartProps;
enableGradient?: boolean;
Expand Down
1 change: 1 addition & 0 deletions packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export const transformPlotlyJsonToDonutProps = (
hideLabels,
showLabelsInPercent: firstData.textinfo ? firstData.textinfo === 'percent' : true,
styles,
canSelectMultipleLegends: true,
};
};

Expand Down
15 changes: 9 additions & 6 deletions packages/charts/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 @@ -31,12 +31,11 @@ export class Arc extends React.Component<IArcProps, IArcState> {
}
srmukher marked this conversation as resolved.
Show resolved Hide resolved

public render(): JSX.Element {
const { arc, href, focusedArcId } = this.props;
const { arc, href, focusedArcId, activeArc } = this.props;
const getClassNames = classNamesFunction<IArcStyleProps, IArcStyles>();
const id = this.props.uniqText! + this.props.data!.data.legend!.replace(/\s+/, '') + this.props.data!.data.data;
const opacity: number =
this.props.activeArc === this.props.data!.data.legend || this.props.activeArc === '' ? 1 : 0.1;

const isActive = activeArc?.includes(this.props.data?.data.legend!);
Copy link
Contributor

Choose a reason for hiding this comment

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

const isActive

why repeating this logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so that it can be used to calculate both opacity and data-is-focusable without the need to add same checks again

const opacity: number = activeArc && activeArc.length > 0 ? (isActive ? 1 : 0.1) : 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

activeArc && activeArc.length > 0 ? (isActive ? 1 : 0.1) : 1;

move this logic to isActive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

const startAngle = this.props.data?.startAngle ?? 0;
const endAngle = (this.props.data?.endAngle ?? 0) - startAngle;
const cornerRadius = this.props.roundCorners ? 3 : 0;
Expand Down Expand Up @@ -70,7 +69,10 @@ export class Arc extends React.Component<IArcProps, IArcState> {
d={arc.cornerRadius(cornerRadius)(this.props.data)}
onFocus={this._onFocus.bind(this, this.props.data!.data, id)}
className={classNames.root}
data-is-focusable={this.props.activeArc === this.props.data!.data.legend || this.props.activeArc === ''}
data-is-focusable={
Array.isArray(this.props.activeArc) &&
(this.props.activeArc.includes(this.props.data!.data.legend!) || this.props.activeArc.length === 0)
}
onMouseOver={this._hoverOn.bind(this, this.props.data!.data)}
onMouseMove={this._hoverOn.bind(this, this.props.data!.data)}
onMouseLeave={this._hoverOff}
Expand Down Expand Up @@ -129,7 +131,8 @@ export class Arc extends React.Component<IArcProps, IArcState> {
if (
hideLabels ||
Math.abs(data!.endAngle - data!.startAngle) < Math.PI / 12 ||
(activeArc !== data!.data.legend && activeArc !== '')
!Array.isArray(activeArc) ||
(data!.data.legend && activeArc.includes(data!.data.legend))
Copy link
Contributor

Choose a reason for hiding this comment

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

same logic is repeating

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to a function

) {
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/charts/react-charting/src/components/DonutChart/Arc/Arc.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export interface IArcProps {
/**
* Active Arc for chart
*/
activeArc?: string;
activeArc?: string[];

/**
* internal prop for href
Expand Down
49 changes: 30 additions & 19 deletions packages/charts/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 @@ -27,6 +27,7 @@ export interface IDonutChartState {
selectedLegend: string;
dataPointCalloutProps?: IChartDataPoint;
callOutAccessibilityData?: IAccessibilityProps;
selectedLegends: string[];
Copy link
Contributor

Choose a reason for hiding this comment

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

selectedLegends

can you resolve all addressed comments

}

export class DonutChartBase extends React.Component<IDonutChartProps, IDonutChartState> {
Expand Down Expand Up @@ -69,12 +70,13 @@ export class DonutChartBase extends React.Component<IDonutChartProps, IDonutChar
legend: '',
_width: this.props.width || 200,
_height: this.props.height || 200,
activeLegend: '',
activeLegend: undefined,
color: '',
xCalloutValue: '',
yCalloutValue: '',
selectedLegend: '',
focusedArcId: '',
selectedLegends: [],
};
this._hoverCallback = this._hoverCallback.bind(this);
this._focusCallback = this._focusCallback.bind(this);
Expand Down Expand Up @@ -138,7 +140,9 @@ export class DonutChartBase extends React.Component<IDonutChartProps, IDonutChar
hoverLeaveCallback={this._hoverLeave}
uniqText={this._uniqText}
onBlurCallback={this._onBlur}
activeArc={this._getHighlightedLegend()}
activeArc={this._getHighlightedLegend().filter(
Copy link
Contributor

Choose a reason for hiding this comment

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

_getHighlightedLegend

use isLegendHighlighted here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

(legend): legend is string => legend !== undefined && legend !== '',
)}
focusedArcId={this.state.focusedArcId || ''}
href={this.props.href!}
calloutId={this._calloutId}
Expand Down Expand Up @@ -240,23 +244,17 @@ export class DonutChartBase extends React.Component<IDonutChartProps, IDonutChar
const legend: ILegend = {
title: point.legend!,
color,
action: () => {
if (this.state.selectedLegend === point.legend) {
this.setState({ selectedLegend: '' });
} else {
this.setState({ selectedLegend: point.legend! });
}
},
hoverAction: () => {
this._handleChartMouseLeave();
this.setState({ activeLegend: point.legend! });
},
onMouseOutAction: () => {
this.setState({ activeLegend: '' });
this.setState({ activeLegend: undefined });
Copy link
Contributor

Choose a reason for hiding this comment

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

undefined

setting as empty should be ok here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, checked with a legend having '' title, it does not work in that case

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

},
};
return legend;
});

const legends = (
<Legends
legends={legendDataItems}
Expand All @@ -265,11 +263,17 @@ export class DonutChartBase extends React.Component<IDonutChartProps, IDonutChar
focusZonePropsInHoverCard={this.props.focusZonePropsForLegendsInHoverCard}
overflowText={this.props.legendsOverflowText}
{...this.props.legendProps}
canSelectMultipleLegends={this.props.canSelectMultipleLegends}
onChange={this._onLegendChange}
Copy link
Contributor

Choose a reason for hiding this comment

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

onChange={this._onLegendChange}

keep using the action event handler. As this can be overridden for a specific legend as per the caller.

Dont modify existing behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

modified as discussed offline

Copy link
Contributor

Choose a reason for hiding this comment

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

_onLegendChange

onLegendSelectionChange

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

/>
);
return legends;
}

private _onLegendChange = (selectedLegends: string[]) => {
this.setState({ selectedLegends });
};
Copy link
Contributor

Choose a reason for hiding this comment

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

keep these functions together with getHighlightedLegends

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


private _focusCallback = (data: IChartDataPoint, id: string, element: SVGPathElement): void => {
this._currentHoverElement = element;
this.setState({
Expand Down Expand Up @@ -324,16 +328,23 @@ export class DonutChartBase extends React.Component<IDonutChartProps, IDonutChar
};

private _valueInsideDonut(valueInsideDonut: string | number | undefined, data: IChartDataPoint[]) {
const highlightedLegend = this._getHighlightedLegend();
if (valueInsideDonut !== undefined && (highlightedLegend !== '' || this.state.showHover)) {
let legendValue = valueInsideDonut;
data!.map((point: IChartDataPoint, index: number) => {
if (point.legend === highlightedLegend || (this.state.showHover && point.legend === this.state.legend)) {
legendValue = point.yAxisCalloutData ? point.yAxisCalloutData : point.data!;
const highlightedLegends = this._getHighlightedLegend().filter(legend => legend !== undefined);

if (this.state.showHover) {
const legendValue = data.find(point => point.legend === this.state.legend);
Copy link
Contributor

Choose a reason for hiding this comment

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

legendValue

call this variable point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

calling it pointValue as point is used in data.find

return legendValue
? legendValue.yAxisCalloutData
? legendValue.yAxisCalloutData
: legendValue.data!
: valueInsideDonut;
} else if (highlightedLegends.length > 0) {
let totalValue = 0;
data.forEach(point => {
if (highlightedLegends.includes(point.legend!)) {
totalValue += point.data!;
}
return;
});
return legendValue;
return totalValue;
} else {
return valueInsideDonut;
}
Expand All @@ -354,7 +365,7 @@ export class DonutChartBase extends React.Component<IDonutChartProps, IDonutChar
* Note: This won't work in case of multiple legends selection.
*/
private _getHighlightedLegend() {
Copy link
Contributor

Choose a reason for hiding this comment

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

_getHighlightedLegend

modify this function for multiselect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was already modified

return this.state.selectedLegend || this.state.activeLegend;
Copy link
Contributor

Choose a reason for hiding this comment

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

return this.state.selectedLegend || this.state.activeLegend;

This is an or condition between selected and active.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For multiple select also checking if this.state.selectedLegends is populated other return the this.state.activeLegend
return this.state.selectedLegends.length > 0 ? this.state.selectedLegends : [this.state.activeLegend];

return this.state.selectedLegends.length > 0 ? this.state.selectedLegends : [this.state.activeLegend];
Copy link
Contributor

Choose a reason for hiding this comment

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

return this.state.selectedLegends.length > 0 ? this.state.selectedLegends : [this.state.activeLegend];

this wont work if 2 legend are selected and a third legend is hovered

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But, in line multiple chart, on multiple legend selection, legend hover is disabled. Shall the experience be not the same?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. We will need a different representation. Currently hover behavior is lost once one or more legend is selected. We will plan for it. You can continue with the current behavior for now.

}

private _isChartEmpty(): boolean {
Expand Down
6 changes: 6 additions & 0 deletions packages/charts/react-charting/src/components/DonutChart/DonutChart.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ export interface IDonutChartProps extends ICartesianChartProps {
* @default false
*/
roundCorners?: boolean;

/**
* Prop to enable the multiple legend selection
* @default false
*/
canSelectMultipleLegends?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

canSelectMultipleLegends?: boolean;

not needed. This is an existing property of the legends control

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/charts/react-charting/src/components/DonutChart/Pie/Pie.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export interface IPieProps {
/**
* Active Arc for chart
*/
activeArc?: string;
activeArc?: string[];

/**
* string for callout id
Expand Down
73 changes: 71 additions & 2 deletions packages/react-examples/src/react-charting/DonutChart/DonutChart.Basic.Example.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { Toggle } from '@fluentui/react/lib/Toggle';
interface IDonutChartState {
enableGradient: boolean;
roundCorners: boolean;
legendMultiSelect: boolean;
}

export class DonutChartBasicExample extends React.Component<IDonutChartProps, IDonutChartState> {
Expand All @@ -22,6 +23,7 @@ export class DonutChartBasicExample extends React.Component<IDonutChartProps, ID
this.state = {
enableGradient: false,
roundCorners: false,
legendMultiSelect: false,
};
}

Expand All @@ -41,6 +43,62 @@ export class DonutChartBasicExample extends React.Component<IDonutChartProps, ID
gradient: getGradientFromToken(DataVizGradientPalette.gradient2),
xAxisCalloutData: '2020/04/20',
},
{
legend: 'third',
data: 12000,
color: getColorFromToken(DataVizPalette.color3),
gradient: getGradientFromToken(DataVizGradientPalette.gradient2),
xAxisCalloutData: '2020/04/20',
},
{
legend: 'fourth',
data: 2000,
color: getColorFromToken(DataVizPalette.color4),
gradient: getGradientFromToken(DataVizGradientPalette.gradient2),
xAxisCalloutData: '2020/04/20',
},
{
legend: 'fifth',
data: 5000,
color: getColorFromToken(DataVizPalette.color5),
gradient: getGradientFromToken(DataVizGradientPalette.gradient2),
xAxisCalloutData: '2020/04/20',
},
{
legend: 'sixth',
data: 6000,
color: getColorFromToken(DataVizPalette.color6),
gradient: getGradientFromToken(DataVizGradientPalette.gradient2),
xAxisCalloutData: '2020/04/20',
},
{
legend: 'seventh',
data: 7000,
color: getColorFromToken(DataVizPalette.color7),
gradient: getGradientFromToken(DataVizGradientPalette.gradient2),
xAxisCalloutData: '2020/04/20',
},
{
legend: 'eighth',
data: 8000,
color: getColorFromToken(DataVizPalette.color8),
gradient: getGradientFromToken(DataVizGradientPalette.gradient2),
xAxisCalloutData: '2020/04/20',
},
{
legend: 'ninth',
data: 9000,
color: getColorFromToken(DataVizPalette.color9),
gradient: getGradientFromToken(DataVizGradientPalette.gradient2),
xAxisCalloutData: '2020/04/20',
},
{
legend: 'tenth',
data: 10000,
color: getColorFromToken(DataVizPalette.color10),
gradient: getGradientFromToken(DataVizGradientPalette.gradient2),
xAxisCalloutData: '2020/04/20',
},
];

const data: IChartProps = {
Expand All @@ -66,6 +124,14 @@ export class DonutChartBasicExample extends React.Component<IDonutChartProps, ID
onChange={this._onToggleRoundCorners}
checked={this.state.roundCorners}
/>
&nbsp;&nbsp;
<Toggle
label="Select Multiple Legends"
onText="ON"
offText="OFF"
onChange={this._onToggleLegendMultiSelect}
checked={this.state.legendMultiSelect}
/>
</div>

<DonutChart
Expand All @@ -75,11 +141,10 @@ export class DonutChartBasicExample extends React.Component<IDonutChartProps, ID
href={'https://developer.microsoft.com/en-us/'}
legendsOverflowText={'overflow Items'}
hideLegend={false}
height={220}
width={176}
valueInsideDonut={39000}
enableGradient={this.state.enableGradient}
roundCorners={this.state.roundCorners}
canSelectMultipleLegends={this.state.legendMultiSelect}
/>
</>
);
Expand All @@ -92,4 +157,8 @@ export class DonutChartBasicExample extends React.Component<IDonutChartProps, ID
private _onToggleRoundCorners = (ev: React.MouseEvent<HTMLElement>, checked: boolean) => {
this.setState({ roundCorners: checked });
};

private _onToggleLegendMultiSelect = (ev: React.MouseEvent<HTMLElement>, checked: boolean) => {
this.setState({ legendMultiSelect: checked });
};
}
Loading