From ff43f39f7522fb94cd791397f209effe06940daf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 05:45:31 +0000 Subject: [PATCH] fix(components,plugin-list): let DataEmptyState declare role="status", so an empty result is not shaped like a failed one `DataLoadingState` has always been `role="status"` and `DataErrorState` `role="alert"`. `DataEmptyState`, in the same file, declared nothing -- so an empty box and a failed box were the same node shape to a screen reader and to any structural test. Measured before changing anything, by rendering every surface and reading its empty box: the bare primitive, plugin-list's empty list, plugin-list's load-error panel, plugin-detail's two activity timelines and the `ui:empty` schema renderer all carried NO role, while plugin-dashboard's WidgetEmptyState (#7063), plugin-kanban's empty board and plugin-charts' ObjectChart (PR #7139) each carried `role="status"` typed out at their own call site. So the surfaces were not legitimately differing. Every surface that wanted the property had hand-copied the same line, and the ones that had not yet done so were silently missing it -- one platform default, paid for per package, which is the tax objectstack#13848 rules against. It is a DEFAULT, not a fixed attribute: `role` is spread from props, so a call site keeps the last word. That is what makes this inert for the two already ruled surfaces -- both pass `role="status"` explicitly and receive the identical attribute either way. Neither ruled surface's behaviour changes, and the metric/KPI carve-out is untouched because it gates whether an empty state is rendered at all, upstream of this component. The measurement also turned up a real defect. plugin-list renders its load FAILURE through `DataEmptyState`, borrowing it for layout, so "You don't have access" and "Nothing here yet" were indistinguishable. That panel now declares `role="alert"` -- which fixes the pre-existing collision and stops the new default from announcing an outage as a routine status. Pinned in data-empty-state-role-7132.test.tsx and ListView.emptyVsErrorRole-7132.test.tsx: the default, the override that keeps the ruled surfaces inert, the sibling contrast, and the two list branches by exact role value. objectui#7132. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM --- .changeset/7132-empty-state-role-default.md | 50 ++++++++++ .../data-empty-state-role-7132.test.tsx | 71 ++++++++++++++ .../components/src/custom/view-states.tsx | 26 +++++ packages/plugin-list/src/ListView.tsx | 8 ++ .../ListView.emptyVsErrorRole-7132.test.tsx | 98 +++++++++++++++++++ 5 files changed, 253 insertions(+) create mode 100644 .changeset/7132-empty-state-role-default.md create mode 100644 packages/components/src/__tests__/data-empty-state-role-7132.test.tsx create mode 100644 packages/plugin-list/src/__tests__/ListView.emptyVsErrorRole-7132.test.tsx diff --git a/.changeset/7132-empty-state-role-default.md b/.changeset/7132-empty-state-role-default.md new file mode 100644 index 0000000000..4b6cce93ac --- /dev/null +++ b/.changeset/7132-empty-state-role-default.md @@ -0,0 +1,50 @@ +--- +'@object-ui/components': minor +'@object-ui/plugin-list': patch +--- + +`DataEmptyState` now declares `role="status"` by default, so an empty result is +distinguishable from a failed one on every surface that renders it +(objectui#7132). + +This is the convergence half of the two rulings that landed as objectui#7063 and +objectui#7064, both resting on objectstack#13848: uniform behaviour belongs to +the platform, and per-surface compensation is the per-app tax being ruled +against. Those two fixed their own surfaces deliberately and locally; this card +measured whether the shared primitive should carry the property. It did not. + +**Measured, not assumed.** All the surfaces were rendered and their empty boxes +read directly: + +| surface | `role` before | +|---|---| +| `DataEmptyState` bare default | *none* | +| `plugin-list` empty list | *none* | +| `plugin-list` load-error panel | *none* | +| `plugin-detail` activity timelines | *none* | +| `ui:empty` schema renderer | *none* | +| `plugin-dashboard` `WidgetEmptyState` (#7063) | `status`, typed at the call site | +| `plugin-kanban` empty board | `status`, typed at the call site | + +The sibling states in the same file had always declared themselves — +`DataLoadingState` is `role="status"`, `DataErrorState` is `role="alert"` — and +the empty state alone declared nothing. So the surfaces were not legitimately +differing: the ones that wanted the property had each hand-typed the same line, +and the ones that had not yet done so were silently missing it. That is one +platform default, copied by hand, at package level. + +**It is a default, not a fixed attribute** — `role` is spread from props, so a +call site keeps the last word. That is what makes this inert for the two ruled +surfaces: both already pass `role="status"` explicitly and receive the identical +attribute with or without it. Neither surface's behaviour changes. + +**One real defect fell out of the measurement.** `plugin-list` renders its load +FAILURE through `DataEmptyState`, borrowing it for layout — so a 403 saying "You +don't have access" and a young object saying "Nothing here yet" were the same +node shape, with no role on either. That panel now declares `role="alert"`, +which both fixes the pre-existing indistinguishability and stops the new default +from announcing an outage as a routine status. + +Metric/KPI widgets are untouched: their carve-out (`rows.length === 0 && +!isMetric`) gates whether an empty state is rendered *at all*, upstream of this +component, so a KPI still reads `0` rather than "no data". diff --git a/packages/components/src/__tests__/data-empty-state-role-7132.test.tsx b/packages/components/src/__tests__/data-empty-state-role-7132.test.tsx new file mode 100644 index 0000000000..9d4984d30b --- /dev/null +++ b/packages/components/src/__tests__/data-empty-state-role-7132.test.tsx @@ -0,0 +1,71 @@ +/** + * 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#7132 — the empty state must declare what it is. + * + * `DataLoadingState` has always been `role="status"` and `DataErrorState` + * `role="alert"`; `DataEmptyState` alone declared nothing, so "this list is + * young" and "this list failed to load" were the same node shape. Both the + * hotcrm#1212 (#7063) and hotcrm#1247 (#7064) rulings name *distinguishable + * from a load failure* as the first property an empty state owes, and four + * separate call sites had each hand-typed `role="status"` to get it. + * + * SUITE DIRECTION, predicted before running: the DEFAULT arm is red against + * `origin/main` and green after. The OVERRIDE arms and the two SIBLING arms are + * green in both worlds — they are the negative controls proving the default is + * a default (a call site keeps the last word, which is why the two already-ruled + * surfaces are inert under this change) and that the contrast it is measured + * against is real. + */ + +import { describe, it, expect } from 'vitest'; +import { render } from '@testing-library/react'; +import { DataEmptyState, DataErrorState, DataLoadingState } from '../custom/view-states'; + +const emptyBox = (c: HTMLElement) => c.querySelector('[data-slot="data-empty-state"]'); + +describe('DataEmptyState — role default (#7132)', () => { + it('DEFAULT: declares role="status" with no call-site prop', () => { + const { container } = render(); + const box = emptyBox(container); + expect(box).not.toBeNull(); + expect(box!.getAttribute('role')).toBe('status'); + }); + + it('DEFAULT survives the props a real call site passes alongside it', () => { + const { container } = render( + , + ); + expect(emptyBox(container)!.getAttribute('role')).toBe('status'); + // The default must not have displaced the rest of the render. + expect(container.textContent).toContain('Nothing here yet'); + }); + + it('OVERRIDE: a call site passing role="alert" keeps it (the load-error borrow)', () => { + const { container } = render(); + expect(emptyBox(container)!.getAttribute('role')).toBe('alert'); + }); + + it('OVERRIDE: the already-ruled surfaces pass role="status" explicitly and are unchanged', () => { + // plugin-dashboard's WidgetEmptyState (#7063) and plugin-kanban both spell + // this out. They must receive the identical attribute with or without the + // default, which is what makes #7132 inert for them. + const { container } = render(); + const box = emptyBox(container)!; + expect(box.getAttribute('role')).toBe('status'); + expect(box.getAttribute('aria-live')).toBe('polite'); + }); + + it('SIBLING CONTRAST: the error state is an alert and the loading state a status', () => { + const { container: err } = render(); + expect(err.querySelector('[data-slot="data-error-state"]')!.getAttribute('role')).toBe('alert'); + const { container: load } = render(); + expect(load.querySelector('[data-slot="data-loading-state"]')!.getAttribute('role')).toBe('status'); + }); +}); diff --git a/packages/components/src/custom/view-states.tsx b/packages/components/src/custom/view-states.tsx index 118f2834d8..294d845a60 100644 --- a/packages/components/src/custom/view-states.tsx +++ b/packages/components/src/custom/view-states.tsx @@ -83,6 +83,31 @@ interface DataEmptyStateProps extends React.ComponentProps<"div"> { action?: React.ReactNode } +/** + * `role` defaults to `"status"` (objectui#7132). + * + * The sibling states in this file each declare what they are — `DataLoadingState` + * is `role="status"`, `DataErrorState` is `role="alert"` — and the empty state + * alone declared nothing, so an empty box and a failed box were the same node + * shape to a screen reader and to any structural test. That is the exact + * property both the hotcrm#1212 (objectui#7063) and hotcrm#1247 (objectui#7064) + * rulings named first: an empty state must be *distinguishable from a load + * failure at a glance*. + * + * With no default, every surface that wanted the property had to type it at its + * own call site, and four independently did — `plugin-kanban`'s empty board, + * `plugin-dashboard`'s `WidgetEmptyState`, and `plugin-charts`' `ObjectChart` — + * while `plugin-list`, `plugin-detail`'s two timelines and the `ui:empty` + * renderer silently did not. Four hand-copies of one line is the per-app tax + * objectstack#13848 rules against, paid at the package level. + * + * It is a DEFAULT, not a fixed attribute: `role` is spread from `props` below, + * so a call site keeps the last word. That is what makes this change inert for + * the two ruled surfaces — both already pass `role="status"` explicitly and + * receive the identical attribute either way — and it is what lets a call site + * rendering something that is NOT empty say so (`plugin-list`'s load-error + * panel borrows this component and declares `role="alert"`). + */ function DataEmptyState({ className, icon, @@ -102,6 +127,7 @@ function DataEmptyState({ return (
(({ empty state on slow networks. */} {loadError && data.length === 0 ? ( Promise) { + const ds = { + find: vi.fn().mockImplementation(find), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + }; + return render( + + + , + ); +} + +async function panel(container: HTMLElement, testId: string): Promise { + await waitFor(() => { + expect(container.querySelector(`[data-testid="${testId}"]`)).not.toBeNull(); + }); + return container.querySelector(`[data-testid="${testId}"]`) as HTMLElement; +} + +describe('ListView — an empty list is not a failed list (#7132)', () => { + it('EMPTY: the empty state is announced as role="status"', async () => { + const { container } = renderWith(() => Promise.resolve([])); + const box = await panel(container, 'empty-state'); + expect(box.getAttribute('role')).toBe('status'); + // Guard against the arm passing over a collapsed render: the empty copy + // must actually be present in the box being measured. + expect(box.textContent).toMatch(/nothing here yet/i); + }); + + it('ERROR: the load-failure panel is announced as role="alert"', async () => { + const { container } = renderWith(() => + Promise.reject(Object.assign(new Error('Forbidden'), { httpStatus: 403 })), + ); + const box = await panel(container, 'list-error-state'); + expect(box.getAttribute('role')).toBe('alert'); + expect(box.getAttribute('data-error-kind')).toBe('forbidden'); + }); + + it('the two branches carry DIFFERENT roles, by exact value', async () => { + const { container: emptyC } = renderWith(() => Promise.resolve([])); + const emptyRole = (await panel(emptyC, 'empty-state')).getAttribute('role'); + const { container: errC } = renderWith(() => + Promise.reject(Object.assign(new Error('Forbidden'), { httpStatus: 403 })), + ); + const errorRole = (await panel(errC, 'list-error-state')).getAttribute('role'); + // Asserted by exact value, not by inequality: on `origin/main` the roles + // were `null` and `null`, but a partial fix leaving the empty branch at + // `null` would still satisfy `null !== 'alert'` and pass a mere-difference + // assertion. Both values must be named for this arm to be able to fail. + expect(emptyRole).toBe('status'); + expect(errorRole).toBe('alert'); + }); +});