Skip to content

Commit 8640cec

Browse files
yinlianghuiclaude
andauthored
fix(plugin-dashboard): GridLayout shows the retired-format placeholder instead of a silent blank chart (#4612) (#4613)
framework#3320 retired the pre-ADR-0021 inline-analytics widget shape and gave DashboardRenderer a graceful fallback: a visible "This widget uses a retired data format. Edit it to bind a dataset." tile for the stored metadata that still carries it. DashboardGridLayout — separately exported, registered as the `dashboard-grid` SDUI component — had no such sentinel, so the identical widget fell through to its static-data branch with `data: []`: a silent blank chart with no diagnostic and no path to fix, the exact outcome the retirement's own test header says must not happen. The detector and the placeholder now live in one module (`legacyRetiredWidget.ts`) consumed by both surfaces, because one private copy is why the defect existed. DashboardRenderer's observable behaviour is unchanged. The nested `options.data = { provider: 'object', … }` config, dataset widgets and static-data widgets are untouched on both surfaces, pinned by four negative controls. Fixes #4612 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 Co-authored-by: Claude <noreply@anthropic.com>
1 parent c1d939f commit 8640cec

5 files changed

Lines changed: 274 additions & 16 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@object-ui/plugin-dashboard': minor
3+
---
4+
5+
One legacy detector, two dashboard surfaces — the editable grid stops rendering a silent blank chart
6+
7+
framework#3320 retired the pre-ADR-0021 inline-analytics widget shape (top-level `object` + `categoryField` / `valueField` / `aggregate`, pivot `rowField` / `columnField`) and shipped a graceful fallback for the stored metadata that still carries it: a visible tile reading "This widget uses a retired data format. Edit it to bind a dataset." The fallback was applied to `DashboardRenderer` and to nothing else. `DashboardGridLayout` — separately exported, and registered as the `dashboard-grid` SDUI component — had no sentinel at all, so the identical stored widget fell through to its static-data branch with `data: []`. Same metadata, same product, two different outcomes: a rebind prompt on one surface, a silent blank chart on the other. The blank is worse for the author than the pre-retirement state, because it carries no chart, no diagnostic and no path to fix — the exact outcome the retirement's own test header says must not happen.
8+
9+
The fix is not a second copy of the condition, because one copy is why the defect existed. The detector and the placeholder now live in a single module (`legacyRetiredWidget.ts`) that both surfaces import; `DashboardRenderer`'s observable behaviour is unchanged, pinned by its existing suite, and the new grid suite mirrors that suite's structure on the surface nobody had pinned. Four positive cases go from blank to placeholder, two of them the widget shapes stored byte-for-byte in the schema catalog's `filtered-dashboard` entry.
10+
11+
The negative controls are the load-bearing half, because the retired shape is one character away from a live one. `options.data = { provider: 'object', … }` carries its OWN nested `object` and `aggregate`: it is a different, still-live authoring surface, read off the widget's data rather than off the widget top level, and it keeps rendering untouched — as do dataset-bound widgets and static-data widgets, on both surfaces. `DashboardRenderer`'s pivot arm stays deliberately surface-local rather than shared: it returns the placeholder for the entire pivot family because that surface emits no pivot block at all, which is a fact about what it can draw, not about the widget being legacy. The grid does draw pivots, from static data and from the provider config, so exporting that arm would have retired two working branches. A legacy pivot is caught on both surfaces by the shared sentinel instead, via the top-level `object` it carries.

packages/plugin-dashboard/src/DashboardGridLayout.tsx

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { useObjectTranslation, pickLocalized } from '@object-ui/i18n';
88
import type { BaseSchema, DashboardComponentSchema, DashboardWidgetSchema } from '@object-ui/types';
99
import { isObjectProvider } from './utils';
1010
import { classifyWidgetType } from './widgetDispatch';
11+
import { LEGACY_RETIRED_WIDGET_SCHEMA, isLegacyRetiredWidget } from './legacyRetiredWidget';
1112

1213
/** Bridges editMode transitions to the ObjectUI DnD system when a DndProvider is present. */
1314
function DndEditModeBridge({ editMode }: { editMode: boolean }) {
@@ -174,6 +175,16 @@ export const DashboardGridLayout: React.FC<DashboardGridLayoutProps> = ({
174175
const getComponentSchema = React.useCallback((widget: DashboardWidgetSchema) => {
175176
if (widget.component) return widget.component;
176177

178+
// Retired legacy inline-analytics widget (framework#3320) — the SAME
179+
// detector `DashboardRenderer` uses, imported rather than restated
180+
// (objectui#4612). Without it this stored shape reached the branches below
181+
// with no data at all and rendered a silent blank chart / empty table /
182+
// em-dash metric: no diagnostic and no path to fix, which is worse for the
183+
// author than the pre-retirement state. It must be tested BEFORE the
184+
// dispatch branches, exactly as on the sibling surface, because those
185+
// branches are what swallow it.
186+
if (isLegacyRetiredWidget(widget)) return LEGACY_RETIRED_WIDGET_SCHEMA;
187+
177188
const widgetType = widget.type;
178189
const options = (widget.options || {}) as Record<string, any>;
179190
// One shared classification (./widgetDispatch) — this surface used to name

packages/plugin-dashboard/src/DashboardRenderer.tsx

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {
3939
import { CSS } from '@dnd-kit/utilities';
4040
import { isObjectProvider } from './utils';
4141
import { classifyWidgetType, METRIC_LIKE_TYPES } from './widgetDispatch';
42+
import { LEGACY_RETIRED_WIDGET_SCHEMA, isLegacyRetiredWidget } from './legacyRetiredWidget';
4243
import { DatasetWidget } from './DatasetWidget';
4344
import { DashboardFilterBar } from './DashboardFilterBar';
4445

@@ -138,21 +139,16 @@ const FILTERABLE_COMPONENT_TYPES = new Set([
138139
'object-data-table',
139140
]);
140141

141-
/**
142-
* Placeholder schema for a widget that still carries the retired pre-ADR-0021
143-
* inline-analytics binding — top-level `object` + `categoryField`/`valueField`/
144-
* `aggregate` — with no `dataset` and no inline `options.data`. No authoring
145-
* surface emits this shape anymore (framework#3320); any surviving stored
146-
* metadata renders this visible error placeholder (not a blank) so an author can
147-
* rebind the widget to a dataset.
142+
/*
143+
* The retired-widget placeholder and its detector used to be declared right
144+
* here, private to this file — and that is the whole reason objectui#4612
145+
* existed: the sibling surface `DashboardGridLayout` could not consume what it
146+
* could not see, so the graceful fallback covered one of the two surfaces and
147+
* the other rendered a silent blank chart for the identical stored metadata.
148+
* Both now import the single declaration from `./legacyRetiredWidget`; the
149+
* behaviour on this surface is unchanged (pinned by
150+
* `__tests__/DashboardRenderer.legacyRetired.test.tsx`).
148151
*/
149-
const LEGACY_RETIRED_WIDGET_SCHEMA = {
150-
type: 'text',
151-
value: 'This widget uses a retired data format. Edit it to bind a dataset.',
152-
variant: 'caption',
153-
align: 'center',
154-
className: 'flex h-full w-full items-center justify-center rounded border border-dashed border-destructive/40 bg-destructive/5 p-4 text-center text-destructive',
155-
} as const;
156152

157153
/**
158154
* The dashboard renderer's props.
@@ -545,8 +541,10 @@ const DashboardRendererInner = forwardRef<HTMLDivElement, DashboardRendererProps
545541
// the pre-ADR-0021 top-level `object` binding with no `dataset` and no
546542
// inline `options.data`. No authoring surface emits this anymore, so it
547543
// is stale stored metadata — render a visible error placeholder (not a
548-
// blank) prompting a rebind to a dataset.
549-
if (!widgetData && (widget as any).object) {
544+
// blank) prompting a rebind to a dataset. The condition itself lives in
545+
// `./legacyRetiredWidget` so `DashboardGridLayout` applies the SAME one
546+
// (objectui#4612) instead of a hand-copied twin.
547+
if (isLegacyRetiredWidget(widget)) {
550548
return LEGACY_RETIRED_WIDGET_SCHEMA;
551549
}
552550

@@ -704,6 +702,15 @@ const DashboardRendererInner = forwardRef<HTMLDivElement, DashboardRendererProps
704702
// ObjectPivotTable / PivotTable components remain public SDUI blocks for
705703
// other surfaces. A non-dataset pivot reaching here is stale metadata —
706704
// show the retired placeholder rather than a blank grid.
705+
//
706+
// This arm stays surface-LOCAL and is deliberately not part of the
707+
// shared detector (objectui#4612): it is a statement about what THIS
708+
// surface can draw — nothing here emits a pivot block — not about the
709+
// widget being legacy. `DashboardGridLayout` does still draw pivots from
710+
// static data and from the `provider: 'object'` config, so exporting
711+
// this family-wide arm would have retired two live branches over there.
712+
// The legacy pivot SHAPE is covered on both surfaces by the shared
713+
// sentinel above, which its top-level `object` matches.
707714
if (dispatch.family === 'pivot') {
708715
return LEGACY_RETIRED_WIDGET_SCHEMA;
709716
}
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* objectui#4612 — the SECOND dashboard surface gets the same graceful fallback.
11+
*
12+
* `DashboardRenderer` has carried the framework#3320 legacy sentinel since the
13+
* retirement: a widget still holding the pre-ADR-0021 inline-analytics shape
14+
* (top-level `object`, no `dataset`, no inline `options.data`) renders a VISIBLE
15+
* error placeholder prompting a rebind. `DashboardGridLayout` — separately
16+
* exported, and registered as the `dashboard-grid` SDUI component — had none, so
17+
* the very same stored metadata fell through to its static-data branch with
18+
* `data: []`: a silent blank chart, no diagnostic, no path to fix. That is the
19+
* exact outcome `DashboardRenderer.legacyRetired.test.tsx`'s header says must not
20+
* happen, on the surface nobody pinned.
21+
*
22+
* The detector and the placeholder now live in ONE module
23+
* (`../legacyRetiredWidget`) with two consumers, because the defect existed
24+
* precisely because there were two surfaces and one fix.
25+
*
26+
* The negative controls are the binding half. `options.data = { provider:
27+
* 'object', … }` carries its OWN nested `object`/`aggregate` and is a DIFFERENT,
28+
* still-live authoring surface (the #4600 measurement's do-not-conflate note);
29+
* dataset widgets and static-data widgets are live too. None of them may acquire
30+
* the placeholder — on either surface.
31+
*/
32+
33+
import { describe, it, expect, afterEach } from 'vitest';
34+
import React from 'react';
35+
import { render, screen, cleanup } from '@testing-library/react';
36+
import type { DashboardComponentSchema } from '@object-ui/types';
37+
import { DashboardGridLayout } from '../DashboardGridLayout';
38+
39+
afterEach(cleanup);
40+
41+
/**
42+
* `object` / `categoryField` / `aggregate` are no longer part of
43+
* `DashboardWidgetSchema` — the cast mimics stored legacy metadata that predates
44+
* the ADR-0021 dataset shape, which is the only way this shape can still arrive.
45+
*/
46+
const dash = (widget: Record<string, unknown>): DashboardComponentSchema =>
47+
({ type: 'dashboard', widgets: [widget] }) as unknown as DashboardComponentSchema;
48+
49+
describe('DashboardGridLayout retired legacy widgets (#4612)', () => {
50+
it.each([
51+
['chart', { id: 'w1', type: 'bar', object: 'invoices', categoryField: 'month', valueField: 'amount', aggregate: 'sum' }],
52+
// Byte-for-byte `examples/schema-catalog/src/schemas/plugin-dashboard/
53+
// filtered-dashboard.json` → `widgets[0]`: the real stored shape, not a
54+
// fixture invented to match the detector.
55+
['catalog bar', { id: 'invoices_by_status', title: 'Invoices by Status', type: 'bar', object: 'invoices', categoryField: 'status', aggregate: 'count' }],
56+
// The pivot family, whose legacy spelling names `rowField`/`columnField`.
57+
['pivot', { id: 'w1', type: 'pivot', object: 'invoices', rowField: 'region', valueField: 'amount' }],
58+
// `widgets[2]` of the same catalog entry.
59+
['metric', { id: 'w1', type: 'metric', object: 'invoices', aggregate: 'count' }],
60+
])('renders the visible placeholder for a legacy %s widget', (_kind, widget) => {
61+
render(<DashboardGridLayout schema={dash(widget)} />);
62+
expect(screen.getByText(/retired data format/i)).toBeInTheDocument();
63+
});
64+
65+
it('states the rebind affordance verbatim, not merely "something is wrong"', () => {
66+
// The message IS the fix path — a placeholder that does not say what to do
67+
// is only a prettier blank. Pinned verbatim, and identically to the
68+
// DashboardRenderer surface (one shared constant, one wording).
69+
render(<DashboardGridLayout schema={dash({ id: 'w1', type: 'bar', object: 'invoices', aggregate: 'count' })} />);
70+
expect(
71+
screen.getByText('This widget uses a retired data format. Edit it to bind a dataset.'),
72+
).toBeInTheDocument();
73+
});
74+
75+
/**
76+
* The must-not-change half. Each control asserts BOTH that the placeholder is
77+
* absent AND that the widget was really rendered — otherwise "no placeholder"
78+
* would also be satisfied by a grid that threw or drew nothing, and the
79+
* control would pass for the wrong reason.
80+
*/
81+
const renderOne = (widget: Record<string, unknown>) => {
82+
const { container } = render(<DashboardGridLayout schema={dash(widget)} />);
83+
expect(container.querySelector('[data-testid="grid-layout"]')).toBeInTheDocument();
84+
return container;
85+
};
86+
87+
it('does NOT show the placeholder for a dataset-bound widget', () => {
88+
renderOne({ id: 'w1', type: 'bar', dataset: 'invoices', values: ['count'] });
89+
expect(screen.queryByText(/retired data format/i)).not.toBeInTheDocument();
90+
});
91+
92+
it('does NOT show the placeholder for an options.data provider widget', () => {
93+
// The nested `{ provider: 'object', object, aggregate }` config is a
94+
// separate, LIVE surface. Its `object` is read off `widgetData`, never off
95+
// the widget top level — conflating the two would retire a working feature.
96+
renderOne({
97+
id: 'w1',
98+
type: 'bar',
99+
options: { data: { provider: 'object', object: 'invoices', aggregate: { field: 'amount', function: 'sum' } } },
100+
});
101+
expect(screen.queryByText(/retired data format/i)).not.toBeInTheDocument();
102+
});
103+
104+
it('does NOT show the placeholder for a static-data widget', () => {
105+
renderOne({ id: 'w1', type: 'bar', options: { data: [{ name: 'A', value: 1 }] } });
106+
expect(screen.queryByText(/retired data format/i)).not.toBeInTheDocument();
107+
});
108+
109+
it('does NOT show the placeholder for a static-data pivot widget', () => {
110+
// DashboardRenderer's pivot arm returns the placeholder for the WHOLE family
111+
// because that surface has no pivot renderer at all. This surface does, and
112+
// a static-data pivot is a live static-data widget — so the shared detector
113+
// is the inline-analytics sentinel (which the legacy pivot above matches via
114+
// its top-level `object`), NOT the family arm. Mirroring the family arm here
115+
// would have retired a working branch.
116+
renderOne({ id: 'w1', type: 'pivot', options: { data: [{ region: 'EMEA', amount: 1 }] } });
117+
expect(screen.queryByText(/retired data format/i)).not.toBeInTheDocument();
118+
});
119+
});
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* ONE legacy detector and ONE placeholder, shared by every dashboard surface
11+
* (objectui#4612).
12+
*
13+
* framework#3320 retired the pre-ADR-0021 inline-analytics binding — a widget
14+
* carrying top-level `object` + `categoryField` / `valueField` / `aggregate`
15+
* (pivot: `rowField` / `columnField`) instead of a semantic-layer `dataset`. No
16+
* authoring surface emits that shape anymore (`WidgetConfigPanel` actively
17+
* scrubs it on save via `LEGACY_ANALYTICS_KEYS`), so any widget still carrying
18+
* it is stale STORED metadata — and stored metadata is exactly what a renderer
19+
* cannot refuse to receive.
20+
*
21+
* The retirement therefore came with a graceful fallback: render a VISIBLE error
22+
* placeholder naming the fix, never a blank widget. That fallback was applied to
23+
* `DashboardRenderer` and to nothing else, while `DashboardGridLayout` — a
24+
* separately exported surface, registered as the `dashboard-grid` SDUI component
25+
* — kept falling through to its static-data branch with `data: []`. Same stored
26+
* metadata, same product: a rebind prompt on one surface and a silent blank
27+
* chart on the other, which is worse for the user than the pre-retirement state
28+
* (no chart, no diagnostic, no path to fix).
29+
*
30+
* The defect existed *because* there were two surfaces and one fix, so the cure
31+
* is not a second copy of the condition: both surfaces consume this module.
32+
*
33+
* ## What this deliberately does NOT match
34+
*
35+
* `options.data = { provider: 'object', object, aggregate }` is a DIFFERENT and
36+
* still-LIVE authoring surface — the async data-provider config, which carries
37+
* its OWN nested `object`/`aggregate`. Every read of it is on `widgetData`, not
38+
* on the widget top level (`DashboardRenderer.tsx` / `DashboardGridLayout.tsx`,
39+
* the `isObjectProvider` branches). Conflating the two would retire a working
40+
* feature, so the detector requires the absence of any widget-level data before
41+
* it looks at `object` at all — the same order `DashboardRenderer` has always
42+
* used.
43+
*/
44+
45+
import type { DashboardWidgetSchema } from '@object-ui/types';
46+
47+
/**
48+
* The placeholder schema rendered in place of a retired inline-analytics widget.
49+
*
50+
* A `text` node rather than a thrown error or an empty box: it has to reach the
51+
* author *inside the dashboard*, in the tile where the chart used to be, and it
52+
* has to say what to do next — a placeholder that only signals "something is
53+
* wrong" is a prettier blank. The wording is contract (one condition, one
54+
* wording) and is pinned verbatim by both surfaces' `legacyRetired` suites.
55+
*/
56+
export const LEGACY_RETIRED_WIDGET_SCHEMA = {
57+
type: 'text',
58+
value: 'This widget uses a retired data format. Edit it to bind a dataset.',
59+
variant: 'caption',
60+
align: 'center',
61+
className: 'flex h-full w-full items-center justify-center rounded border border-dashed border-destructive/40 bg-destructive/5 p-4 text-center text-destructive',
62+
} as const;
63+
64+
/**
65+
* The four keys this detector reads, and the reason it needs a shape of its own:
66+
* two of them are not on `DashboardWidgetSchema` at all. `object` was removed
67+
* from the widget vocabulary by the retirement — reading it is the whole point
68+
* here — and `data` is a renderer-internal spelling that never had a declaration.
69+
* Naming them once, here, is what keeps `as any` out of both call sites.
70+
*/
71+
type LegacyRetiredReadKeys = {
72+
/** Semantic-layer binding (ADR-0021). Its presence means the widget is current. */
73+
dataset?: unknown;
74+
/** Renderer-internal inline data array (the widget-level spelling). */
75+
data?: unknown;
76+
/** Renderer-internal inline data / nested `provider: 'object'` config. */
77+
options?: { data?: unknown } | null;
78+
/** The retired pre-ADR-0021 top-level object binding. */
79+
object?: unknown;
80+
};
81+
82+
/**
83+
* Does this widget still carry the retired pre-ADR-0021 inline-analytics shape?
84+
*
85+
* True when all three hold, in this order:
86+
*
87+
* 1. no `dataset` — a dataset-bound widget is the CURRENT shape and renders
88+
* through the governed `queryDataset` path;
89+
* 2. no renderer-internal data — neither `widget.data` nor `options.data`, which
90+
* covers both the static inline array and the live `provider: 'object'`
91+
* nested config;
92+
* 3. a top-level `object` — the retired binding itself.
93+
*
94+
* `||` (not `??`) between the two data spellings, preserving `DashboardRenderer`
95+
* byte for byte.
96+
*
97+
* Step 1 is stated here rather than inherited from a caller's render fork:
98+
* `DashboardRenderer` never reached the placeholder for a dataset-bound widget
99+
* because `datasetBound` picks `DatasetWidget` at the render site regardless of
100+
* what `getComponentSchema` returned — so naming the condition costs that
101+
* surface no observable behavior, and it keeps the predicate true on its own
102+
* terms for any surface that has no such fork.
103+
*/
104+
export function isLegacyRetiredWidget(widget: DashboardWidgetSchema | null | undefined): boolean {
105+
if (!widget) return false;
106+
const w = widget as LegacyRetiredReadKeys;
107+
if (w.dataset) return false;
108+
const widgetData = w.data || w.options?.data;
109+
return !widgetData && !!w.object;
110+
}

0 commit comments

Comments
 (0)