fix(dashboards): isolate pie chart slice labels per widget (#21034)
## Summary Fixes [#21014](https://github.com/twentyhq/twenty/issues/21014). When two pie chart widgets shared the same group-by field (and therefore the same slice ids) but used different aggregation operators (e.g. `count` vs `sum`), the arc-link labels would mirror between the two charts — both ending up showing either the count or the sum values, depending on render order. Center metrics stayed correct. **Root cause.** Nivo's `ArcLinkLabelsLayer` and `ArcsLayer` (from `@nivo/arcs`) wire `react-spring`'s `useTransition` with `keys: e => e.id`. When two `<ResponsivePie>` instances render with overlapping ids, the transitioned data bleeds across charts. The center metric is unaffected because it's computed by a separate hook (`usePieChartCenterMetricData`). **Fix.** Namespace the Nivo-computed slice id per widget by passing an `id` accessor to `<ResponsivePie>`: ```tsx id={(datum) => `${id}:${String(datum.id)}`} ``` Lookups inside the widget switch to `datum.data.id` (the original, un-namespaced id stored on the raw datum), so value/percentage formatting, the custom tooltip, and the legend hover-dim behavior all keep working. Touched files: - `GraphWidgetPieChart.tsx` — add `id` accessor - `CustomArcsLayer.tsx` — compare legend highlight against `datum.data.id` - `getPieChartFormattedValue.ts`, `getPieChartTooltipData.ts` — match on `datum.data.id` - Tests for both utils get a regression case covering the namespaced computed id ## Test plan - [ ] `npx jest getPieChartFormattedValue` ✅ - [ ] `npx jest getPieChartTooltipData` ✅ - [ ] `npx tsc --noEmit` ✅ - [ ] Manual: dashboard with two pies on the same group-by field, one `count` and one `sum`, "Display data label" on for both — confirm each chart shows its own metric on the slices, and the central total is unchanged. - [ ] Manual: hover a legend item — the matching slice in that chart stays solid while the others dim, and the sibling chart is not affected. - [ ] Manual: clicking a slice still drills into the correctly filtered view.
This commit is contained in:
+1
-1
@@ -38,7 +38,7 @@ export const CustomArcsLayer = ({
|
||||
{transition((_, datum) => {
|
||||
const isDimmed =
|
||||
isDefined(graphWidgetHighlightedLegendId) &&
|
||||
String(graphWidgetHighlightedLegendId) !== String(datum.id);
|
||||
String(graphWidgetHighlightedLegendId) !== String(datum.data.id);
|
||||
const arcLength = datum.arc.endAngle - datum.arc.startAngle;
|
||||
const padAngleRadians = (padAngle * Math.PI) / 180;
|
||||
const clampedPadAngle = Math.min(
|
||||
|
||||
+1
@@ -194,6 +194,7 @@ export const GraphWidgetPieChart = ({
|
||||
enableArcLabels={false}
|
||||
tooltip={() => null}
|
||||
layers={[ArcsLayer, 'arcLinkLabels']}
|
||||
id={(datum: PieChartDataItem) => `${id}:${String(datum.id)}`}
|
||||
arcLinkLabel={(datum: ComputedDatum<PieChartDataItem>) => {
|
||||
const formattedValue = getPieChartFormattedValue({
|
||||
datum,
|
||||
|
||||
+17
-1
@@ -47,10 +47,12 @@ describe('getPieChartFormattedValue', () => {
|
||||
|
||||
const createMockDatum = (
|
||||
id: string,
|
||||
options?: { computedId?: string },
|
||||
): ComputedDatum<PieChartDataItemWithColor> =>
|
||||
({
|
||||
id,
|
||||
id: options?.computedId ?? id,
|
||||
value: 0,
|
||||
data: { id, value: 0 },
|
||||
}) as unknown as ComputedDatum<PieChartDataItemWithColor>;
|
||||
|
||||
const defaultFormatOptions = {
|
||||
@@ -81,6 +83,20 @@ describe('getPieChartFormattedValue', () => {
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it('should match by datum.data.id when computed id is namespaced per widget', () => {
|
||||
const datum = createMockDatum('slice1', {
|
||||
computedId: 'widget-abc:slice1',
|
||||
});
|
||||
|
||||
const result = getPieChartFormattedValue({
|
||||
datum,
|
||||
enrichedData: mockEnrichedData,
|
||||
formatOptions: defaultFormatOptions,
|
||||
});
|
||||
|
||||
expect(result).toContain('30');
|
||||
});
|
||||
});
|
||||
|
||||
describe('percentage display type', () => {
|
||||
|
||||
+18
-1
@@ -47,10 +47,12 @@ describe('getPieChartTooltipData', () => {
|
||||
|
||||
const createMockDatum = (
|
||||
id: string,
|
||||
options?: { computedId?: string },
|
||||
): ComputedDatum<PieChartDataItemWithColor> =>
|
||||
({
|
||||
id,
|
||||
id: options?.computedId ?? id,
|
||||
value: 0,
|
||||
data: { id, value: 0 },
|
||||
}) as unknown as ComputedDatum<PieChartDataItemWithColor>;
|
||||
|
||||
const defaultFormatOptions = {
|
||||
@@ -148,6 +150,21 @@ describe('getPieChartTooltipData', () => {
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it('should match by datum.data.id when computed id is namespaced per widget', () => {
|
||||
const datum = createMockDatum('Product A', {
|
||||
computedId: 'widget-xyz:Product A',
|
||||
});
|
||||
|
||||
const result = getPieChartTooltipData({
|
||||
datum,
|
||||
enrichedData: mockEnrichedData,
|
||||
formatOptions: defaultFormatOptions,
|
||||
});
|
||||
|
||||
expect(result?.tooltipItem.key).toBe('Product A');
|
||||
expect(result?.tooltipItem.value).toBe(500);
|
||||
});
|
||||
|
||||
it('should return null when enrichedData is empty', () => {
|
||||
const datum = createMockDatum('Product A');
|
||||
|
||||
|
||||
+1
-1
@@ -22,7 +22,7 @@ export const getPieChartFormattedValue = ({
|
||||
displayType,
|
||||
}: GetPieChartFormattedValueParams): string | null => {
|
||||
const item = enrichedData.find(
|
||||
(enrichedDataItem) => enrichedDataItem.id === datum.id,
|
||||
(enrichedDataItem) => enrichedDataItem.id === datum.data.id,
|
||||
);
|
||||
if (!isDefined(item)) return null;
|
||||
|
||||
|
||||
+1
-1
@@ -23,7 +23,7 @@ export const getPieChartTooltipData = ({
|
||||
tooltipItem: GraphWidgetTooltipItem;
|
||||
} | null => {
|
||||
const item = enrichedData.find(
|
||||
(enrichedDataItem) => enrichedDataItem.id === datum.id,
|
||||
(enrichedDataItem) => enrichedDataItem.id === datum.data.id,
|
||||
);
|
||||
if (!isDefined(item)) return null;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user