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
19 changes: 19 additions & 0 deletions .changeset/wild-charts-explain-themselves.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'@object-ui/plugin-charts': patch
---

`ObjectChart` now renders a self-describing empty state when its query succeeds
and returns no rows, instead of falling through to a bare chart frame.

The frame was measured in a browser rather than assumed: recharts derives its
ticks from the data, so with an empty result the bar and line families emit two
hairline axis rules and no `text` nodes at all, and pie/donut emit nothing —
there are no labelled axes to tell the reader what would have been plotted.
Beside the component's own red "Failed to load chart data" box, a blank tile
gives the reader nothing to distinguish a young chart from a broken one.

The copy is the one `plugin-dashboard` already shows on the dataset-bound path
("No data yet" / the load succeeded / the source name), so the same chart over
the same empty result no longer reads two different ways depending on which
widget drew it. Charts with inline authored data are unchanged — they ran no
query to report on.
33 changes: 33 additions & 0 deletions packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,14 @@ const AUTHORED_PROPS = {
* #4428 shipped a six-key first pass because a schema-only measurement cannot
* see it. Answering with empty results keeps every data-bound target on its
* real render path rather than an error state.
*
* ⚠️ That last sentence has ONE measured exception, and it is not an error
* state: since objectui#7130 an empty result is a real render path for
* `ObjectChart` that is not its CHART markup — it renders a self-describing
* empty state. The two object-bound chart targets therefore author their own
* rows; see {@link OBJECT_CHART_EXTRAS}. Read that before widening this
* adapter: handing rows to all ~200 targets would move many of them off the
* branch they are pinned on.
*/
const FAKE_ADAPTER = {
find: async () => [],
Expand Down Expand Up @@ -473,11 +481,36 @@ const CHART_DATA = [
{ name: 'Feb', sales: 300, revenue: 139, value: 300 },
];
const CHART_SERIES = [{ dataKey: 'sales' }, { dataKey: 'revenue' }];
/**
* The two OBJECT-BOUND chart targets (`plugin-charts:object-chart`,
* `view:chart`). They stay object-bound — `objectName` is what makes them a
* different registry path from the six inline chart targets above, which reach
* `ObjectChart` through `ObjectChartBlock` and its `ElementDataSourceGate`.
*
* `data` / `series` are authored on TOP of that binding (`data` is a declared
* registry input on this component: "Optional static data") because
* {@link FAKE_ADAPTER} answers every query with no rows, and since objectui#7130
* `ObjectChart` renders a self-describing empty state on an empty result
* instead of an empty chart frame. Without rows these two targets stop at that
* empty state, `[data-slot="chart"]` never matches, and the readiness guard
* below refuses to scan — correctly, because an empty state is not the chart
* markup this sweep exists to scan.
*
* This is objectui#5630's lesson in a third dress: a data-bound target whose
* clean reading would otherwise cover only its empty-state placeholder. That
* card answered it with a populated host where a host was needed, and with a
* plain `schemaExtras` change where the populated branch was reachable from
* pure schema (`element:definition-list`'s `items`). This is the latter case,
* and it also makes these two scan STRICTLY MORE markup than before: the
* pre-#7130 reading swept a chart frame with no marks in it.
*/
const OBJECT_CHART_EXTRAS = {
objectName: 'accounts',
chartType: 'bar',
categoryField: 'name',
valueField: 'amount',
data: CHART_DATA,
series: CHART_SERIES,
};
const CALENDAR_OBJECT_EXTRAS = {
objectName: 'accounts',
Expand Down
122 changes: 122 additions & 0 deletions packages/plugin-charts/src/ObjectChart.emptyResult.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/**
* 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.
*/

/**
* objectui#7130 — `ObjectChart` over an empty result renders a self-describing
* empty state instead of a bare frame.
*
* ## What the bare frame was, measured
*
* Before this branch the component fell through to `ChartRenderer` with
* `data: []`. Rendered in a real browser at 220c18d05, recharts emitted an SVG
* with TWO hairline axis rules and ZERO `<text>` nodes for bar/line, and
* nothing at all for pie — ticks are derived from the data, so an empty domain
* labels nothing. The filing hypothesis ("a chart frame with axes is arguably
* self-describing") is false: there are no labels on an empty chart.
*
* ## Why the assertions are shaped this way
*
* The maintainer's bar (hotcrm#1212) is *distinguishable from a load failure at
* a glance*, so the pin is not "an empty state exists" — it is that the empty
* state and the failure state are DIFFERENT, checked on the ARIA roles that
* carry that difference (`status` vs `alert`), plus the three directions this
* branch could be wrong in: firing over real rows, firing before the fetch
* resolves, and swallowing an error. Each is a separate `it` so an ablation
* reports which arm moved.
*/
import React from 'react';
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { render, screen, cleanup, waitFor } from '@testing-library/react';

vi.mock('recharts', async () => {
const actual = await vi.importActual<any>('recharts');
return {
...actual,
ResponsiveContainer: ({ children }: any) =>
React.cloneElement(children, { width: 480, height: 320 }),
};
});

import { ObjectChart } from './ObjectChart';

beforeEach(() => {
vi.stubGlobal('fetch', vi.fn(async () => ({ ok: true, json: async () => ({}) })));
});
afterEach(() => {
vi.unstubAllGlobals();
cleanup();
});

const schema = {
type: 'object-chart',
chartType: 'bar' as const,
objectName: 'crm_opportunity',
xAxisKey: 'stage',
series: [{ dataKey: 'amount', label: 'Amount' }],
isAnimationActive: false,
};

const renderWith = (dataSource: any, overrides: Record<string, unknown> = {}) =>
render(<ObjectChart schema={{ ...schema, ...overrides }} dataSource={dataSource} />);

describe('ObjectChart — empty result (objectui#7130)', () => {
it('renders a self-describing empty state when the query returns no rows', async () => {
renderWith({ find: vi.fn().mockResolvedValue([]) });

const box = await screen.findByTestId('chart-empty-state');
// The copy states that the load SUCCEEDED — the fact a blank tile cannot
// give the reader — and promises no recovery (no "loading", no "retry").
expect(box).toHaveTextContent('No data yet');
expect(box).toHaveTextContent('loaded successfully');
expect(box.textContent).not.toMatch(/try again|retry|loading/i);
// Names WHAT is empty, so the tile is self-describing without authored copy.
expect(screen.getByTestId('chart-empty-source')).toHaveTextContent('crm_opportunity');
});

it('marks the empty state `status`, distinct from the failure box `alert`', async () => {
// The whole point of the card: the reader must be able to tell an empty
// result from a load failure. Asserted on the two roles rather than on
// copy, because that is the machine-readable half of the distinction.
const { unmount } = renderWith({ find: vi.fn().mockResolvedValue([]) });
expect(await screen.findByTestId('chart-empty-state')).toHaveAttribute('role', 'status');
unmount();

renderWith({ find: vi.fn().mockRejectedValue(new Error('Network request failed')) });
const failure = await screen.findByTestId('chart-error');
expect(failure).toHaveAttribute('role', 'alert');
// …and the empty branch must not have swallowed the error.
expect(screen.queryByTestId('chart-empty-state')).toBeNull();
});

it('does NOT fire over a populated result', async () => {
renderWith({
find: vi.fn().mockResolvedValue([
{ stage: 'Qualify', amount: 12 },
{ stage: 'Won', amount: 9 },
]),
});
// Wait for the fetch to settle before asserting the absence, so this cannot
// pass merely by running before the data arrives.
await waitFor(() => expect(screen.queryByTestId('chart-loading')).toBeNull());
expect(screen.queryByTestId('chart-empty-state')).toBeNull();
});

it('does NOT flash before the fetch resolves — loading still wins', async () => {
renderWith({ find: vi.fn(() => new Promise(() => {})) });
expect(await screen.findByTestId('chart-loading')).toBeTruthy();
expect(screen.queryByTestId('chart-empty-state')).toBeNull();
});

it('leaves an inline-data chart alone — it ran no query to report on', async () => {
// `data: []` is an authoring choice, not an empty query result, so the copy
// ("its query returned no records") would be false of it.
renderWith({ find: vi.fn().mockResolvedValue([]) }, { data: [] });
await waitFor(() => expect(screen.queryByTestId('chart-loading')).toBeNull());
expect(screen.queryByTestId('chart-empty-state')).toBeNull();
});
});
78 changes: 76 additions & 2 deletions packages/plugin-charts/src/ObjectChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import React, { useState, useEffect, useContext, useCallback, useMemo } from 're
import { useDataScope, SchemaRendererContext, SchemaRenderer, useDrillNavigation, useFilterScope, ElementDataSourceGate, type ElementDataSourceMapping } from '@object-ui/react';
import { ChartRenderer } from './ChartRenderer';
import { ComponentRegistry, humanizeLabel, extractRecords, computeDrillFilter, isDrillEnabled, resolveDrillTitle, resolveFilterPlaceholders, resolveContextTokens, shiftFilterByCompareTo, compareToTrendLabelKey, buildChartSeries, buildOptionColorMap, deriveDimensionLabelMaps, dimensionOptionTranslator, loadDimensionFieldMeta, relabelDimensions, localizeFieldOptions, elementDataSourceBlock, type DimensionFieldMeta, type CompareToConfig, type DrillEvent, type ChartResultField, type ChartSegmentClickEvent } from '@object-ui/core';
import { Sheet, SheetContent, SheetHeader, SheetTitle, Dialog, DialogContent, DialogHeader, DialogTitle, RefreshIndicator, Button, ChartSkeleton } from '@object-ui/components';
import { AlertCircle, ArrowUpRight } from 'lucide-react';
import { Sheet, SheetContent, SheetHeader, SheetTitle, Dialog, DialogContent, DialogHeader, DialogTitle, RefreshIndicator, Button, ChartSkeleton, DataEmptyState } from '@object-ui/components';
import { AlertCircle, ArrowUpRight, Inbox } from 'lucide-react';
import { useSafeFieldLabel, useSafeTranslate } from '@object-ui/i18n';
import type { DrillDownConfig } from '@object-ui/types';

Expand Down Expand Up @@ -919,6 +919,80 @@ export const ObjectChart = (props: any) => {
return <div className={"flex items-center justify-center text-muted-foreground text-sm p-4 " + (schema.className || '')} data-testid="chart-no-datasource">No data source available for &ldquo;{schema.objectName}&rdquo;</div>;
}

// Query succeeded and returned nothing → a self-describing empty state, NOT
// the bare frame this used to fall through to (objectui#7130).
//
// ## What the bare frame actually rendered — measured, not assumed
//
// The card was filed on the hypothesis that "a chart frame with axes is
// arguably self-describing": an empty table is a blank rectangle, but an
// empty chart still draws labelled axes telling the reader what WOULD have
// been plotted. Rendered in a real browser at 220c18d05, that is false.
// Recharts derives its ticks FROM the data, so with `data: []` there is no
// domain and no tick to label: the bar/line families emit an SVG containing
// two hairline axis rules and ZERO `<text>` nodes, and pie/donut emit an
// empty `<svg>` with no marks at all. Measured against a populated control
// in the same render, which emitted eight `<text>` nodes. So the frame is
// not self-describing — it is a blank tile beside a `chart-error` box that
// at least says something, which is precisely the hotcrm#1212 failure:
// nothing on screen says whether the chart failed or is simply young.
//
// ## Why this is not the KPI carve-out
//
// `DatasetWidget` deliberately exempts metric families — "a metric (single
// value) over an empty dataset is 0, not an empty state" — and names charts
// on the OTHER side of that line in the same comment: "Charts and tables
// keep the empty state (there is genuinely nothing to plot)." A KPI's `0` is
// a datum; a chart's blank frame is an absence. So a dataset-bound chart has
// rendered this state since #7124 and the object-bound one did not: same
// family, same empty result, two answers. This is the surface that ruling
// did not reach, not a new judgement.
//
// ## Why `DataEmptyState` and not `WidgetEmptyState`
//
// The seam #7124 built is `plugin-dashboard`-local and unexported (absent
// from that package's `index.tsx`), and `plugin-charts` does not depend on
// `plugin-dashboard`. Reaching it would mean promoting it to public API for
// a foreign plugin — the cross-surface abstraction objectui#7132 owns.
// `DataEmptyState` is instead the primitive that is ALREADY shared and
// already consumed by plugin-list / plugin-kanban / plugin-detail and by
// `WidgetEmptyState` itself, out of `@object-ui/components`, which this
// package already depends on. Nothing is promoted, no dependency edge is
// added, and when #7132 converges the defaults this call site collapses the
// same way the other four do.
//
// The copy is the keys #7124 landed in all ten packs — no new key, and no
// promise of recovery: it states that the load SUCCEEDED, which is the one
// fact the reader of a blank tile cannot otherwise get. `role="status"`
// against the `role="alert"` on the `chart-error` box above is the machine
// check that the two states are distinct.
//
// Gated on a QUERY-backed chart: a chart handed inline `data: []` by its
// author never ran a query, so "its query returned no records" would be
// false of it, and those charts render byte-for-byte as before.
const isQueryBacked = !!(schema.objectName || schema.dataset);
if (isQueryBacked && !boundData && !schema.data && finalData.length === 0) {
return (
<DataEmptyState
role="status"
data-testid="chart-empty-state"
className={"h-full w-full gap-2 p-4 [&>h3]:text-sm [&>h3]:font-medium [&>p]:text-xs " + (schema.className || '')}
icon={<Inbox className="h-5 w-5 text-muted-foreground/70" />}
iconWrapperClassName="flex size-9 items-center justify-center rounded-lg bg-muted"
title={tt('dashboard.empty.title', 'No data yet')}
description={tt(
'dashboard.empty.message',
'This widget loaded successfully and its query returned no records yet.',
)}
>
<p className="text-xs text-muted-foreground/80" data-testid="chart-empty-source">
<span>{tt('dashboard.empty.sourceLabel', 'Source:')}</span>{' '}
<span className="font-mono">{schema.dataset || schema.objectName}</span>
</p>
</DataEmptyState>
);
}

const internalChartClick = isDrillEnabled(drillDown)
? (ev: ChartSegmentClickEvent) => {
const labelCategory = ev.category;
Expand Down
Loading