Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
99 changes: 99 additions & 0 deletions packages/plugin-charts/src/AdvancedChartImpl.pieLegend.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* Pie/donut legend (#3135) — the legend must NAME each slice's category, and it
* must obey `showLegend`.
*
* Two defects lived here. The legend resolved its label through the shadcn
* config helper, which read the legend ITEM's own `type`/`color` props before
* the data row — so a category dimension literally named `type` (a dashboard
* staple: "按类型分布") resolved to the legend ICON shape ("rect"), missed the
* config, and drew colour swatches with NO text: a pie of anonymous dots. And
* the pie branch rendered its legend unconditionally, so `showLegend: false`
* did nothing.
*/

import React from 'react';
import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, cleanup, act } from '@testing-library/react';

// Recharts' ResponsiveContainer measures its parent via ResizeObserver, which
// reports 0×0 under the headless DOM — so the chart never paints. Replace it
// with a fixed-size passthrough so the pie (and its legend) actually render.
vi.mock('recharts', async () => {
const actual = await vi.importActual<any>('recharts');
return {
...actual,
ResponsiveContainer: ({ children }: any) =>
React.cloneElement(children, { width: 320, height: 320 }),
};
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

// Recharts registers the pie's legend payload from a layout effect and the
// Legend re-renders off that store update, so read the legend after a tick.
const settle = () => act(async () => { await new Promise((r) => setTimeout(r, 60)); });

const legendText = (c: HTMLElement) =>
c.querySelector('.recharts-legend-wrapper')?.textContent ?? '';

describe('AdvancedChartImpl — pie/donut legend', () => {
// `type` is the exact dimension name from the reported dashboard, and it
// collides with the recharts legend item's own `type` (icon shape) prop.
it.each(['type', 'color', 'urgency'])('labels each slice for a dimension named `%s`', async (dim) => {
const { container } = render(
<AdvancedChartImpl
chartType="pie"
data={[{ [dim]: '设备', total_count: 3 }, { [dim]: '质量', total_count: 2 }]}
xAxisKey={dim}
series={[{ dataKey: 'total_count' }]}
showLegend
isAnimationActive={false}
/>,
);
await settle();
expect(legendText(container)).toContain('设备');
expect(legendText(container)).toContain('质量');
});

// Single-category data is the degenerate case the issue calls out: one full
// circle in one colour is zero information without the category's name.
it('donut: a single category is still named', async () => {
const { container } = render(
<AdvancedChartImpl
chartType="donut"
data={[{ type: '设备', total_count: 3 }]}
xAxisKey="type"
series={[{ dataKey: 'total_count' }]}
showLegend
isAnimationActive={false}
/>,
);
await settle();
expect(legendText(container)).toContain('设备');
});

it('honors showLegend: false', async () => {
const { container } = render(
<AdvancedChartImpl
chartType="pie"
data={[{ type: '设备', total_count: 3 }, { type: '质量', total_count: 2 }]}
xAxisKey="type"
series={[{ dataKey: 'total_count' }]}
showLegend={false}
isAnimationActive={false}
/>,
);
await settle();
expect(container.querySelectorAll('.recharts-legend-wrapper')).toHaveLength(0);
});
});
12 changes: 7 additions & 5 deletions packages/plugin-charts/src/AdvancedChartImpl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -689,11 +689,13 @@ function AdvancedChartImplInner({
return <Cell key={`cell-${index}`} fill={resolveColor(c)} />;
})}
</Pie>
<ChartLegend
verticalAlign="bottom"
wrapperStyle={{ fontSize: isMobile ? '11px' : '12px', paddingTop: '8px' }}
content={<ChartLegendContent nameKey={xAxisKey} className="flex-wrap" />}
/>
{legendVisible ? (
<ChartLegend
verticalAlign="bottom"
wrapperStyle={{ fontSize: isMobile ? '11px' : '12px', paddingTop: '8px' }}
content={<ChartLegendContent nameKey={xAxisKey} className="flex-wrap" />}
/>
) : null}
</PieChart>
</ChartContainer>
);
Expand Down
17 changes: 12 additions & 5 deletions packages/plugin-charts/src/ChartContainerImpl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -401,19 +401,26 @@ function getPayloadConfigFromPayload(

let configLabelKey: string = key

// The DATA ROW wins over the legend/tooltip item's own same-named prop.
// Recharts item entries carry `type` (the legend ICON shape, e.g. "rect"),
// `color`, `dataKey` and `value` of their own, so a pie whose category
// dimension is literally named `type` (`nameKey="type"`) used to resolve to
// "rect", miss the config, and render legend swatches with NO text at all —
// a pie of unlabelled colour dots (#3135). Reading the row first keys off the
// actual category ("设备"), which is what `nameKey` was pointing at.
if (
key in payload &&
typeof payload[key as keyof typeof payload] === "string"
) {
configLabelKey = payload[key as keyof typeof payload] as string
} else if (
payloadPayload &&
key in payloadPayload &&
typeof payloadPayload[key as keyof typeof payloadPayload] === "string"
) {
configLabelKey = payloadPayload[
key as keyof typeof payloadPayload
] as string
} else if (
key in payload &&
typeof payload[key as keyof typeof payload] === "string"
) {
configLabelKey = payload[key as keyof typeof payload] as string
}

return configLabelKey in config
Expand Down
14 changes: 13 additions & 1 deletion packages/plugin-dashboard/src/DatasetWidget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,18 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource:
});
const effectiveCategoryOrder = explicitOrder?.length ? explicitOrder : categoryOrder;

// `chartConfig.showLegend` (#3135). The widget's chart config never reached
// the renderer — this component read `options` and nothing else — so an author
// who wrote `showLegend: false` still got a legend, and one who wrote `true`
// only got one because "on" happens to be the renderer's default. Lower the
// one flag the renderer already honors; the rest of `chartConfig` stays
// unforwarded (the renderer derives axes/series from the dataset selection).
const chartConfig: Record<string, unknown> =
widget?.chartConfig && typeof widget.chartConfig === 'object' && !Array.isArray(widget.chartConfig)
? (widget.chartConfig as Record<string, unknown>)
: {};
const showLegend = typeof chartConfig.showLegend === 'boolean' ? chartConfig.showLegend : undefined;

// Map a clicked chart segment back to its dataset row, then drill through to
// the underlying records — same governed path the table/pivot rows use.
const handleChartDrill = (ev: { category?: string; series?: string; value?: number }) => {
Expand All @@ -617,7 +629,7 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource:
// measurement churn, can freeze there — bars never draw until an unrelated
// re-render (#2756, follow-up to #2727's ineffective settle re-mount).
// Turning the tween off makes the first paint deterministic.
schema={{ type: 'chart', chartType, data: chartData, xAxisKey, series, isAnimationActive: false, ...(categoryColors ? { categoryColors } : {}), ...(effectiveCategoryOrder ? { categoryOrder: effectiveCategoryOrder } : {}) } as any}
schema={{ type: 'chart', chartType, data: chartData, xAxisKey, series, isAnimationActive: false, ...(showLegend != null ? { showLegend } : {}), ...(categoryColors ? { categoryColors } : {}), ...(effectiveCategoryOrder ? { categoryOrder: effectiveCategoryOrder } : {}) } as any}
onChartClick={chartDrill}
onSegmentClick={chartDrill}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* #3135 — a dataset widget's `chartConfig.showLegend` must reach the chart
* renderer. It never did: this component read `options` and nothing else, so
* the flag was inert metadata — `false` still drew a legend, and `true` only
* "worked" because the renderer defaults to showing one.
*
* Asserted at the source (the schema handed to the renderer), via a stubbed
* SchemaRenderer — the same seam DatasetWidget.animation uses.
*/

import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, cleanup, waitFor } from '@testing-library/react';

let lastChartSchema: any = null;

vi.mock('@object-ui/react', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
SchemaRenderer: (props: any) => {
lastChartSchema = props.schema;
return null;
},
}));

import { DatasetWidget } from '../DatasetWidget';

afterEach(() => {
cleanup();
lastChartSchema = null;
});

const rows = [
{ type: '设备', total_count: 5 },
{ type: '质量', total_count: 3 },
];

const renderWidget = (chartConfig?: Record<string, unknown>) => {
const src = { queryDataset: vi.fn(async () => ({ rows })) };
render(
<DatasetWidget
widget={{
type: 'pie',
dataset: 'andon_metrics',
dimensions: ['type'],
values: ['total_count'],
...(chartConfig ? { chartConfig } : {}),
}}
dataSource={src}
/>,
);
};

describe('DatasetWidget — chartConfig.showLegend (#3135)', () => {
it('forwards showLegend: true', async () => {
renderWidget({ type: 'pie', showLegend: true, showDataLabels: false });
await waitFor(() => expect(lastChartSchema).not.toBeNull());
expect(lastChartSchema.chartType).toBe('pie');
expect(lastChartSchema.showLegend).toBe(true);
});

it('forwards showLegend: false', async () => {
renderWidget({ type: 'pie', showLegend: false });
await waitFor(() => expect(lastChartSchema).not.toBeNull());
expect(lastChartSchema.showLegend).toBe(false);
});

// No declaration → the key stays off the schema so the renderer's own default
// (legend shown) still decides. Every existing widget keeps its behaviour.
it('omits the key when the widget declares no chartConfig', async () => {
renderWidget();
await waitFor(() => expect(lastChartSchema).not.toBeNull());
expect('showLegend' in lastChartSchema).toBe(false);
});
});
Loading