From e13bab19ab5860b0e58db773b1e09d394502908d Mon Sep 17 00:00:00 2001 From: os-justin Date: Tue, 8 Sep 2026 02:57:05 +0000 Subject: [PATCH 1/2] fix(plugin-grid,plugin-detail): draw the shared EmptyValue for missing cell values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ObjectGrid and RelatedList each spelled their own empty-cell placeholder — a span classed `text-muted-foreground/50 text-xs italic` holding a bare em-dash. The shared `EmptyValue` carries a `data-slot`, an i18n-resolved `aria-label` and `select-none` / `no-underline` / `pointer-events-none`; the hand-rolled spans carried none of them, so an empty cell had no accessible name while its renderer-supplied neighbour did, and inside a link column it looked clickable. Four sites in ObjectGrid, not the three the originating census counted: the record-detail drawer carried the same placeholder in a `text-sm` spelling. The three cell sites adopt the bare shared component, which is a deliberate visual change — they now match the `EmptyValue` the no-renderer default branch already drew one column over. The drawer keeps its rendered text and typography through `glyph` and `className`, because `grid.empty` has exactly one call site in the workspace. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- ...491-8475-shared-empty-value-placeholder.md | 37 ++ packages/plugin-detail/src/RelatedList.tsx | 3 +- ...t.emptyPlaceholderAffordance-8475.test.tsx | 185 +++++++++ packages/plugin-grid/src/ObjectGrid.tsx | 8 +- ...d.emptyPlaceholderAffordance-8491.test.tsx | 379 ++++++++++++++++++ 5 files changed, 607 insertions(+), 5 deletions(-) create mode 100644 .changeset/8491-8475-shared-empty-value-placeholder.md create mode 100644 packages/plugin-detail/src/__tests__/RelatedList.emptyPlaceholderAffordance-8475.test.tsx create mode 100644 packages/plugin-grid/src/__tests__/ObjectGrid.emptyPlaceholderAffordance-8491.test.tsx diff --git a/.changeset/8491-8475-shared-empty-value-placeholder.md b/.changeset/8491-8475-shared-empty-value-placeholder.md new file mode 100644 index 0000000000..5dc9c691eb --- /dev/null +++ b/.changeset/8491-8475-shared-empty-value-placeholder.md @@ -0,0 +1,37 @@ +--- +'@object-ui/plugin-grid': minor +'@object-ui/plugin-detail': minor +--- + +`ObjectGrid` and `RelatedList` now draw the shared `EmptyValue` for a missing +cell value instead of each spelling its own placeholder (objectui#8491, +objectui#8475). + +**The accessibility defect.** Both components built a plain span holding a bare +em-dash, classed `text-muted-foreground/50 text-xs italic`. The shared +`EmptyValue` in `@object-ui/components` carries three things that span did not: +a `data-slot` of `empty-value`, an `aria-label` resolved through the i18n label +hook, and `select-none` / `no-underline` / `pointer-events-none`. So an empty +cell had **no accessible name at all** — a screen reader heard a naked +punctuation mark — while its renderer-supplied neighbour in the next column was +announced as "No value". In `RelatedList` the two outcomes were reachable in the +*same column*: an empty string took the hand-rolled branch, an unparseable +`datetime` reached `DateTimeCellRenderer` and got the real one. Inside a link +column the hand-rolled placeholder also inherited the link colour and was +selectable, so a missing value looked clickable and could be copied. + +**A deliberate visual change, not a no-op.** The three grid cell sites and the +related-list site drop `text-xs italic` and adopt the shared component's +typography. That is the point: `ObjectGrid`'s no-renderer default branch already +returned `EmptyValue`, so one table could show a 12px italic placeholder in one +column and the shared upright one in the next. They are now byte-identical. + +**A fourth site, and a purely additive change there.** The grid's record-detail +drawer had the same hand-rolled placeholder in a `text-sm` spelling, which the +originating census missed. It adopts `EmptyValue` too, but keeps its rendered +text and typography through `glyph` and `className` — the `grid.empty` string +has exactly one call site in the workspace and dropping it would strand a +translated value in ten locale packs. Its delta is the three affordances only. + +Filled cells are untouched in every path, including the grid's stacked card +layout below 768px. diff --git a/packages/plugin-detail/src/RelatedList.tsx b/packages/plugin-detail/src/RelatedList.tsx index 6352a94611..e08d9c6da6 100644 --- a/packages/plugin-detail/src/RelatedList.tsx +++ b/packages/plugin-detail/src/RelatedList.tsx @@ -24,6 +24,7 @@ import { AlertDialogHeader, AlertDialogTitle, cn, + EmptyValue, resolveIcon, useIsMobile, } from '@object-ui/components'; @@ -1004,7 +1005,7 @@ export const RelatedList: React.FC = ({ // whitespace-only string or an empty array through to a renderer that // paints nothing, in a column `pruneEmpty` had already judged empty. if (isValueEmpty(value)) { - return React.createElement('span', { className: 'text-muted-foreground/50 text-xs italic' }, '—'); + return React.createElement(EmptyValue); } return React.createElement(CellRenderer, { value, field: fieldMeta }); }; diff --git a/packages/plugin-detail/src/__tests__/RelatedList.emptyPlaceholderAffordance-8475.test.tsx b/packages/plugin-detail/src/__tests__/RelatedList.emptyPlaceholderAffordance-8475.test.tsx new file mode 100644 index 0000000000..1613e3ff72 --- /dev/null +++ b/packages/plugin-detail/src/__tests__/RelatedList.emptyPlaceholderAffordance-8475.test.tsx @@ -0,0 +1,185 @@ +/** + * 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. + */ + +/** + * `RelatedList`'s hand-rolled placeholder becomes the shared `EmptyValue` + * (objectui#8475). + * + * ## What was wrong, and why it showed up TWICE in one column + * + * `makeCell` built its own placeholder — `React.createElement('span', { + * className: 'text-muted-foreground/50 text-xs italic' }, '—')` — with no + * `data-slot`, no `aria-label`, and none of the shared component's + * `select-none` / `no-underline` / `pointer-events-none`. + * + * That branch runs FIRST and intercepts the values `isValueEmpty` recognises. + * Anything it passes through reaches a type-aware cell renderer that has its + * own empty branch and returns the real `EmptyValue`. Both outcomes are + * reachable in the SAME column: an empty string takes the hand-rolled branch, + * while an unparseable datetime is not "empty" to `isValueEmpty` at all and + * reaches `DateTimeCellRenderer`, which draws the shared one. + * + * ⚠️ objectui#8475's body named `DateCellRenderer` for this, quoting `if (date + * === null || isNaN(date.getTime())) return EmptyValue`. Measured: that line + * is `DateTimeCellRenderer`'s. `DateCellRenderer`'s only `EmptyValue` branch is + * `if (!value)`, which `isValueEmpty` has already intercepted, so an + * unparseable `date` renders a formatted span and never reaches the shared + * component. The card's conclusion holds; its attribution did not, which is + * why this case uses `datetime`. + * + * So one column could show two cells that read identically to a sighted user + * while only one of them was announced to a screen reader — and they were not + * even typographically identical, because the hand-rolled one added `text-xs + * italic` that the shared component does not have. `THE AGREEMENT` below is + * that exact pair, in one column, in one render. + * + * ## Which cases DISCRIMINATE, and which are scope declarations + * + * An implementation that renders `EmptyValue` **everywhere, including for + * filled cells**, passes `THE DEFECT` and passes `THE AGREEMENT` — "the cell + * has an accessible name" is true of a list that has stopped rendering values. + * The single case that REFUSES it is `NON-REGRESSION — a FILLED cell`, which + * asserts both that the value is present AND that no placeholder shares that + * cell. + * + * `THE AGREEMENT` is kept as a SCOPE DECLARATION about the visual half rather + * than shipped as if it discriminated: it is the only case pinning that the + * `text-xs italic` treatment is deliberately gone. + * + * ## The viewport is pinned on purpose (objectui#8399) + * + * `RelatedList` swaps the whole data-table for an `object-gallery` when + * `useIsMobile()` is true, and a gallery renders none of these cells. Without + * the pin every case below would assert against a surface that never ran the + * code under test. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, waitFor, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import * as React from 'react'; + +// The real data-table and its cell renderers must be registered — this pin +// reads what they DRAW, so `SchemaRenderer` is deliberately NOT mocked. +import '@object-ui/components'; +import { RelatedList } from '../RelatedList'; + +/** Desktop. See the docblock — the mobile branch renders no table at all. */ +beforeEach(() => { + Object.defineProperty(window, 'innerWidth', { configurable: true, value: 1280 }); +}); + +function makeDataSource(fields: Record, rows: any[]) { + return { + getObjectSchema: vi.fn(async () => ({ name: 'line', fields })), + find: vi.fn(async () => ({ data: rows, total: rows.length })), + }; +} + +async function mountGrid(fields: Record, rows: any[]) { + const { container } = render( + , + ); + await waitFor(() => expect(container.querySelector('table')).not.toBeNull()); + await waitFor(() => expect(container.textContent).toContain('Widget')); + + const headers = () => + Array.from(container.querySelectorAll('th')).map((th) => (th.textContent ?? '').trim()); + + /** The cell under `header` within ONE row — never a container-wide lookup. */ + const cell = (rowIndex: number, header: string): HTMLElement => { + const idx = headers().indexOf(header); + expect(idx, `the ${header} column is present — headers were ${JSON.stringify(headers())}`) + .toBeGreaterThanOrEqual(0); + const tr = container.querySelectorAll('tbody tr')[rowIndex]; + expect(tr, `row ${rowIndex} rendered`).toBeTruthy(); + const td = tr.querySelectorAll('td')[idx]; + expect(td, `row ${rowIndex} has a cell under ${header}`).toBeTruthy(); + return td as HTMLElement; + }; + return { headers, cell }; +} + +/** The shared placeholder inside ONE element, or null. */ +const emptyIn = (el: HTMLElement): HTMLElement | null => + el.querySelector('[data-slot="empty-value"]'); + +const TEXT_FIELDS = { + product: { type: 'text', label: 'Product' }, + note: { type: 'text', label: 'Note' }, +}; + +describe('RelatedList empty cells use the shared EmptyValue (objectui#8475)', () => { + it('THE DEFECT — an empty cell carries an accessible name', async () => { + const { cell } = await mountGrid(TEXT_FIELDS, [ + { id: '1', product: 'Widget', note: '' }, + { id: '2', product: 'Gadget', note: 'a real note' }, + ]); + const placeholder = emptyIn(cell(0, 'Note')); + + expect(placeholder, 'the empty cell draws the shared placeholder').not.toBeNull(); + expect(placeholder, 'and therefore has an accessible name').toHaveAttribute('aria-label'); + expect( + (placeholder as HTMLElement).getAttribute('aria-label'), + 'the name is a word, never a naked punctuation mark', + ).toBe('No value'); + // CONTROL — without this, a list that rendered no values at all passes above. + expect(within(cell(1, 'Note')).queryByText('a real note'), 'CONTROL: the sibling row rendered by value') + .not.toBeNull(); + }); + + it('NON-REGRESSION — a FILLED cell renders its value and NO placeholder', async () => { + const { cell } = await mountGrid(TEXT_FIELDS, [ + { id: '1', product: 'Widget', note: '' }, + { id: '2', product: 'Gadget', note: 'a real note' }, + ]); + const filled = cell(1, 'Note'); + + expect(within(filled).queryByText('a real note'), 'the value reaches the cell').not.toBeNull(); + // THE DISCRIMINATING HALF: red for an EmptyValue-everywhere implementation. + expect(emptyIn(filled), 'a filled cell carries NO placeholder').toBeNull(); + }); + + it('THE AGREEMENT — the two branches of ONE column now draw the identical placeholder', async () => { + // SCOPE DECLARATION about the visual half — see the docblock. This is the + // pair objectui#8475 described: row 0 takes `makeCell`'s own branch, row 1 + // is not empty to `isValueEmpty` and reaches `DateTimeCellRenderer`, whose + // own empty branch has always returned the shared component. + const { cell } = await mountGrid( + { product: { type: 'text', label: 'Product' }, when: { type: 'datetime', label: 'When' } }, + [ + { id: '1', product: 'Widget', when: '' }, + { id: '2', product: 'Gadget', when: 'not-a-date' }, + { id: '3', product: 'Gizmo', when: '2026-01-15T09:00:00Z' }, + ], + ); + const handRolledBranch = emptyIn(cell(0, 'When')); + const rendererBranch = emptyIn(cell(1, 'When')); + + expect(handRolledBranch, "the list's own branch drew a placeholder").not.toBeNull(); + expect(rendererBranch, "CONTROL: the renderer's branch drew one too").not.toBeNull(); + expect( + (handRolledBranch as HTMLElement).className, + 'two placeholders in ONE column are now typographically identical', + ).toBe((rendererBranch as HTMLElement).className); + expect( + (handRolledBranch as HTMLElement).className, + 'and the retired text-xs italic treatment is gone', + ).not.toContain('italic'); + // CONTROL — the column really does render real dates, so this is not a + // column that gave up: a parseable value still reaches the renderer. + expect(emptyIn(cell(2, 'When')), 'CONTROL: a parseable datetime draws NO placeholder').toBeNull(); + }); +}); diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 7d7213663e..31e93c11b3 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -2476,7 +2476,7 @@ export const ObjectGrid: React.FC = ({ cellRenderer = (value: any, row: any) => { const displayContent = CellRenderer ? - : (value != null && value !== '' ? String(value) : —); + : (value != null && value !== '' ? String(value) : ); return ( = ({ cellRenderer = (value: any, row: any) => { const displayContent = CellRenderer ? - : (value != null && value !== '' ? String(value) : —); + : (value != null && value !== '' ? String(value) : ); return ( = ({ objectName={schema.objectName} recordId={rowRecordId(row)} > - {value != null && value !== '' ? String(value) : —} + {value != null && value !== '' ? String(value) : } ); } else if (CellRenderer) { @@ -4359,7 +4359,7 @@ export const ObjectGrid: React.FC = ({ const renderFieldValue = (key: string, value: any): React.ReactNode => { if (value == null || value === '') { - return {t('grid.empty')}; + return ; } // Use objectSchema field type for type-aware rendering diff --git a/packages/plugin-grid/src/__tests__/ObjectGrid.emptyPlaceholderAffordance-8491.test.tsx b/packages/plugin-grid/src/__tests__/ObjectGrid.emptyPlaceholderAffordance-8491.test.tsx new file mode 100644 index 0000000000..b4a7850cb2 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/ObjectGrid.emptyPlaceholderAffordance-8491.test.tsx @@ -0,0 +1,379 @@ +/** + * 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. + */ + +/** + * `ObjectGrid`'s hand-rolled empty placeholders become the shared `EmptyValue` + * (objectui#8491). + * + * ## What was wrong + * + * Four sites in `ObjectGrid.tsx` drew their own muted placeholder — a plain + * span classed `text-muted-foreground/50 text-xs italic` holding a bare + * em-dash (three cell sites) and one classed `...text-sm italic` holding the + * `grid.empty` string (the record-detail drawer). None of them carried: + * + * - `data-slot="empty-value"`, which is how tests and tooling find + * placeholders; + * - an `aria-label`, so an empty cell had **no accessible name at all** + * while its renderer-supplied neighbour in the very next column did; + * - `select-none` / `no-underline` / `pointer-events-none`, so a missing + * value inside a LINK column looked clickable and got copied into a + * selection. + * + * The accessibility gap is what decides it. The typography is the second + * half: the same table showed a 12px italic placeholder in one column and the + * shared 14px upright one in the next, because the no-renderer default branch + * already returned `EmptyValue`. + * + * ## The card counted THREE cell sites. There are FOUR sites. + * + * The fourth is the record-detail drawer's own empty branch, which the card's + * census missed because it grepped for one exact class string and the drawer + * spells `text-sm`, not `text-xs`. It is pinned here by `THE DRAWER` cases. + * + * ## Why the drawer keeps its glyph and the cells do not + * + * The three cell sites adopt the bare shared component, so they agree with the + * `EmptyValue` the default branch already draws one column over — that + * agreement is the point, and it is a DELIBERATE visual change (the `text-xs + * italic` treatment goes away). + * + * The drawer instead keeps its rendered text through `glyph`, for a measured + * reason: `grid.empty` has exactly one call site in the workspace, and + * `i18n/src/__tests__/dead-key-batch-retired-4730.test.ts` names that call site + * as its evidence that the key is live. Swapping the drawer to a bare em-dash + * would silently strand a translated string in ten locale packs. So the + * drawer's delta is purely additive — same text, same typography, plus the + * three affordances — and `THE DRAWER — the rendered text is unchanged` is the + * case that holds that line. + * + * ## Which cases DISCRIMINATE, and which are scope declarations + * + * The lesson objectui#8474 and objectui#8481 each measured independently: the + * most quotable assertion in a pin is usually the one that cannot tell the fix + * from its worst caricature. Stated explicitly here rather than implied. + * + * An implementation that renders `EmptyValue` **everywhere, including for + * filled cells**, still passes every `THE DEFECT` case and both `AGREEMENT` + * cases below — "the cell has an accessible name" is true of a grid that has + * given up on values entirely. + * + * What REFUSES that caricature, and nothing else in this file does: + * + * - `NON-REGRESSION — a FILLED linked cell` + * - `NON-REGRESSION — a FILLED link+action cell` + * - `NON-REGRESSION — a FILLED primary cell` + * - `NON-REGRESSION — a FILLED card field` (mobile) + * - `THE DRAWER — a FILLED field` + * + * Each asserts BOTH that the value is present AND that no placeholder is in + * that same cell. The value half alone is not enough: an implementation that + * appends a placeholder next to every value would pass it. + * + * `AGREEMENT — the two branches draw the identical placeholder` is a SCOPE + * DECLARATION about typography, not an instrument against the caricature — it + * is green for `EmptyValue`-everywhere. It is kept because it is the only + * thing pinning the deliberate visual change, and it is labelled rather than + * shipped as if it proved more. + * + * ## Every DOM lookup is scoped to ONE row or ONE card + * + * Measured on PR #8495: a grid pin that asserted "no childless flex-wrap + * anywhere in the grid" FAILED against the correct implementation, because + * `ObjectGrid`'s toolbar renders a legitimately empty one. Nothing here reads + * the whole container; `emptyIn` / `valueIn` always take a single cell. + * + * ## The viewport is pinned in BOTH directions, and the mobile answer is a + * ## MEASURED correction + * + * `ObjectGrid` swaps the whole table for a stacked card layout below 768px, + * and that layout does call the very same `col.cell(val, row)` renderers this + * change edits — so it looks like a second read path for the placeholder. It + * is not. Its secondary-field loop drops empty values BEFORE the call ("hide + * empty values on mobile"), so an empty field is omitted from the card + * entirely, label and all, and no placeholder of either spelling has ever + * reached it. Measured by rendering, not read from source. + * + * What that leaves: the card layout is a real second read path for the + * POPULATED half, and both viewports are pinned explicitly so neither claim + * rests on whatever `window.innerWidth` a previous test happened to set. + */ +import React from 'react'; +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { ActionProvider } from '@object-ui/react'; +import { ObjectGrid } from '../ObjectGrid'; + +/** + * Deliberately NOT calling `registerAllFields()`. These four branches are the + * ones `ObjectGrid` takes when a column resolves NO cell renderer, which is + * exactly what `getCellRenderer` is never asked for when the column carries no + * resolvable type. The columns below are typeless and their names match none + * of the grid's inference patterns, so `CellRenderer` is null and the + * placeholder branch under test runs. + */ + +const DESKTOP = 1280; +const MOBILE = 480; // below the 768px card-view breakpoint + +function setViewport(width: number) { + Object.defineProperty(window, 'innerWidth', { configurable: true, value: width }); +} + +/** Desktop unless a case says otherwise — see the docblock. */ +beforeEach(() => setViewport(DESKTOP)); +afterEach(() => cleanup()); + +const ROWS = [ + { id: 'r1', title: 'Alpha', note: '', memo: '' }, + { id: 'r2', title: 'Beta', note: 'a real note', memo: 'a real memo' }, +]; + +function renderGrid(columns: unknown, extra: Record = {}) { + return render( + + + , + ); +} + +/** The `rowIndex`-th body row of the rendered table. */ +async function row(container: HTMLElement, rowIndex: number): Promise { + await waitFor(() => expect(container.querySelector('tbody tr')).not.toBeNull()); + const tr = container.querySelectorAll('tbody tr')[rowIndex]; + expect(tr, `row ${rowIndex} rendered`).toBeTruthy(); + return tr as HTMLElement; +} + +/** The cell under `header` within ONE row — never a container-wide lookup. */ +function cellIn(container: HTMLElement, tr: HTMLElement, header: string): HTMLElement { + const headers = Array.from(container.querySelectorAll('thead th')).map((th) => + (th.textContent ?? '').trim(), + ); + const idx = headers.indexOf(header); + expect(idx, `the ${header} column is present — headers were ${JSON.stringify(headers)}`) + .toBeGreaterThanOrEqual(0); + const td = tr.querySelectorAll('td')[idx]; + expect(td, `row has a cell under ${header}`).toBeTruthy(); + return td as HTMLElement; +} + +/** The shared placeholder inside ONE element, or null. */ +const emptyIn = (el: HTMLElement): HTMLElement | null => + el.querySelector('[data-slot="empty-value"]'); + +/** The ONE mobile card holding `title` — never a container-wide lookup. */ +function cardFor(title: string): HTMLElement { + const card = screen.getByText(title).closest('div[class*="rounded-lg"]'); + expect(card, `a card rendered for ${title}`).toBeTruthy(); + return card as HTMLElement; +} + +describe('ObjectGrid empty placeholders use the shared EmptyValue (objectui#8491)', () => { + it('THE DEFECT — an empty LINK cell carries an accessible name', async () => { + const { container } = renderGrid([ + { field: 'title', label: 'Title' }, + { field: 'note', label: 'Note', link: true }, + ]); + const placeholder = emptyIn(cellIn(container, await row(container, 0), 'Note')); + + expect(placeholder, 'the empty linked cell draws the shared placeholder').not.toBeNull(); + expect(placeholder, 'and therefore has an accessible name').toHaveAttribute('aria-label'); + expect( + (placeholder as HTMLElement).getAttribute('aria-label'), + 'the name is a word, never a naked punctuation mark', + ).toBe('No value'); + // CONTROL — without this, a grid that rendered no values at all passes above. + const filled = cellIn(container, await row(container, 1), 'Note'); + expect(within(filled).queryByText('a real note'), 'CONTROL: the sibling row rendered by value') + .not.toBeNull(); + }); + + it('NON-REGRESSION — a FILLED linked cell renders its value and NO placeholder', async () => { + const { container } = renderGrid([ + { field: 'title', label: 'Title' }, + { field: 'note', label: 'Note', link: true }, + ]); + const filled = cellIn(container, await row(container, 1), 'Note'); + + expect(within(filled).queryByText('a real note'), 'the value reaches the cell').not.toBeNull(); + // THE DISCRIMINATING HALF: red for an EmptyValue-everywhere implementation. + expect(emptyIn(filled), 'a filled cell carries NO placeholder').toBeNull(); + }); + + it('THE DEFECT — an empty LINK+ACTION cell carries an accessible name', async () => { + const { container } = renderGrid([ + { field: 'title', label: 'Title' }, + { field: 'note', label: 'Note', link: true, action: 'ping' }, + ]); + const placeholder = emptyIn(cellIn(container, await row(container, 0), 'Note')); + + expect(placeholder, 'the empty link+action cell draws the shared placeholder').not.toBeNull(); + expect(placeholder, 'and therefore has an accessible name').toHaveAttribute('aria-label'); + }); + + it('NON-REGRESSION — a FILLED link+action cell renders its value and NO placeholder', async () => { + const { container } = renderGrid([ + { field: 'title', label: 'Title' }, + { field: 'note', label: 'Note', link: true, action: 'ping' }, + ]); + const filled = cellIn(container, await row(container, 1), 'Note'); + + expect(within(filled).queryByText('a real note'), 'the value reaches the cell').not.toBeNull(); + expect(emptyIn(filled), 'a filled cell carries NO placeholder').toBeNull(); + }); + + it('THE DEFECT — the auto-linked PRIMARY cell of a bare string column list', async () => { + // The string-array column path: index 0 is the auto-linked primary field. + const { container } = renderGrid(['note', 'title']); + const placeholder = emptyIn(cellIn(container, await row(container, 0), 'Note')); + + expect(placeholder, 'the empty primary cell draws the shared placeholder').not.toBeNull(); + expect(placeholder, 'and therefore has an accessible name').toHaveAttribute('aria-label'); + }); + + it('NON-REGRESSION — a FILLED primary cell renders its value and NO placeholder', async () => { + const { container } = renderGrid(['note', 'title']); + const filled = cellIn(container, await row(container, 1), 'Note'); + + expect(within(filled).queryByText('a real note'), 'the value reaches the cell').not.toBeNull(); + expect(emptyIn(filled), 'a filled cell carries NO placeholder').toBeNull(); + }); + + it('AGREEMENT — the linked branch and the no-renderer default branch draw the IDENTICAL placeholder', async () => { + // SCOPE DECLARATION, not an instrument against the caricature — see the + // docblock. It is the only case pinning the deliberate visual change. + const { container } = renderGrid([ + { field: 'title', label: 'Title' }, + { field: 'note', label: 'Note', link: true }, + { field: 'memo', label: 'Memo' }, + ]); + const tr = await row(container, 0); + const linked = emptyIn(cellIn(container, tr, 'Note')); + const fallback = emptyIn(cellIn(container, tr, 'Memo')); + + expect(linked, 'the linked column drew a placeholder').not.toBeNull(); + expect(fallback, 'CONTROL: the default branch drew one too').not.toBeNull(); + expect( + (linked as HTMLElement).className, + 'two placeholders in ONE row are now typographically identical', + ).toBe((fallback as HTMLElement).className); + expect( + (linked as HTMLElement).className, + 'and the retired text-xs italic treatment is gone from both', + ).not.toContain('italic'); + }); + + it('MOBILE CARD VIEW — the card layout OMITS an empty field rather than drawing a placeholder', async () => { + // SCOPE DECLARATION, and a MEASURED correction to the brief's premise: the + // card layout below 768px is NOT a second read path for these three cell + // sites. Its secondary-field loop drops empty values before it would call + // `col.cell(val, row)` ("hide empty values on mobile"), so no placeholder + // of either spelling has ever reached a card. Green before AND after this + // change; it is here so a later edit to that loop cannot quietly start + // drawing one without a reader noticing. + setViewport(MOBILE); + const { container } = renderGrid([ + { field: 'title', label: 'Title' }, + { field: 'note', label: 'Note', link: true }, + ]); + await waitFor(() => expect(screen.queryByText('Alpha')).not.toBeNull()); + // No table below the breakpoint — this is a genuinely different renderer. + expect(container.querySelector('tbody tr'), 'the card layout renders no table rows').toBeNull(); + + const emptyCard = cardFor('Alpha'); + expect(emptyCard, 'the empty record rendered as a card').toBeTruthy(); + expect(within(emptyCard).queryByText('Note'), 'the empty field is omitted, label and all').toBeNull(); + expect(emptyIn(emptyCard), 'and no placeholder is drawn in its place').toBeNull(); + // CONTROL — without this, a run that rendered no cards at all passes above. + const filledCard = cardFor('Beta'); + expect(within(filledCard).queryByText('Note'), 'CONTROL: a populated field DOES get a label').not.toBeNull(); + }); + + it('NON-REGRESSION — a FILLED card field renders its value and NO placeholder', async () => { + // The card layout reaches the edited `col.cell` renderers for POPULATED + // values, so this half is a real second read path. + setViewport(MOBILE); + renderGrid([ + { field: 'title', label: 'Title' }, + { field: 'note', label: 'Note', link: true }, + ]); + await waitFor(() => expect(screen.queryByText('Beta')).not.toBeNull()); + + const card = cardFor('Beta'); + expect(card, 'the populated record rendered as a card').toBeTruthy(); + expect(within(card).queryByText('a real note'), 'the value reaches the card').not.toBeNull(); + expect(emptyIn(card), 'a fully populated card carries NO placeholder').toBeNull(); + }); + + it('THE DRAWER — the fourth site: an empty detail field carries an accessible name', async () => { + const { container } = renderGrid( + [ + { field: 'title', label: 'Title' }, + { field: 'note', label: 'Note' }, + ], + { navigation: { mode: 'drawer' } }, + ); + fireEvent.click(await screen.findByText('Alpha')); + await waitFor(() => expect(screen.getByRole('dialog')).toBeInTheDocument()); + const dialog = screen.getByRole('dialog'); + const placeholder = emptyIn(dialog); + + expect(placeholder, 'the drawer field draws the shared placeholder').not.toBeNull(); + expect(placeholder, 'and therefore has an accessible name').toHaveAttribute('aria-label'); + // CONTROL — the drawer really did render this record, not an empty shell. + expect(within(dialog).queryByText('Alpha'), 'CONTROL: the drawer rendered by value').not.toBeNull(); + void container; + }); + + it('THE DRAWER — the rendered TEXT is unchanged, the change is purely additive', async () => { + // This is what forbids swapping the drawer to a bare em-dash: `grid.empty` + // would lose its only call site. See the docblock. + renderGrid( + [ + { field: 'title', label: 'Title' }, + { field: 'note', label: 'Note' }, + ], + { navigation: { mode: 'drawer' } }, + ); + fireEvent.click(await screen.findByText('Alpha')); + await waitFor(() => expect(screen.getByRole('dialog')).toBeInTheDocument()); + const placeholder = emptyIn(screen.getByRole('dialog')) as HTMLElement; + + expect(placeholder, 'the drawer drew a placeholder').not.toBeNull(); + expect(placeholder.textContent, "the localized 'Empty' text survives verbatim").toBe('Empty'); + expect(placeholder.className, 'and so does the drawer typography').toContain('text-sm'); + expect(placeholder.className, 'and so does the drawer typography').toContain('italic'); + }); + + it('THE DRAWER — a FILLED field renders its value and NO placeholder', async () => { + renderGrid( + [ + { field: 'title', label: 'Title' }, + { field: 'note', label: 'Note' }, + ], + { navigation: { mode: 'drawer' } }, + ); + fireEvent.click(await screen.findByText('Beta')); + await waitFor(() => expect(screen.getByRole('dialog')).toBeInTheDocument()); + const dialog = screen.getByRole('dialog'); + + expect(within(dialog).queryByText('a real note'), 'the value reaches the drawer').not.toBeNull(); + // THE DISCRIMINATING HALF for the drawer. + expect(emptyIn(dialog), 'a fully populated record draws NO placeholder').toBeNull(); + }); +}); From 6b840cb6934ac4617699a7845f4f786baa7553a8 Mon Sep 17 00:00:00 2001 From: os-justin Date: Tue, 8 Sep 2026 03:08:55 +0000 Subject: [PATCH 2/2] test(plugin-grid,plugin-detail): state the MEASURED discrimination, not the predicted one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The caricature (EmptyValue returned unconditionally, filled cells included) was run rather than reasoned about. It reddens 8 of the 12 grid cases, not the "every THE DEFECT case stays green" the docblock had predicted — three of them redden through their value-bearing controls rather than their headline assertion, and exactly four stay green. Both docblocks now name those four as scope declarations and say which assertion refuses the caricature. Two further corrections written into the files: - The mobile card layout is NOT a second read path for these cell sites. It calls the same col.cell renderers, but drops empty values before the call, so an empty field is omitted from the card entirely and no placeholder has ever reached it. Measured by rendering. - objectui#8475 attributed the reachable EmptyValue branch to DateCellRenderer; that line is DateTimeCellRenderer's. The card's conclusion holds, so the agreement case uses datetime. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- ...t.emptyPlaceholderAffordance-8475.test.tsx | 26 ++++++----- ...d.emptyPlaceholderAffordance-8491.test.tsx | 43 ++++++++++++------- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/packages/plugin-detail/src/__tests__/RelatedList.emptyPlaceholderAffordance-8475.test.tsx b/packages/plugin-detail/src/__tests__/RelatedList.emptyPlaceholderAffordance-8475.test.tsx index 1613e3ff72..0f8f2676ce 100644 --- a/packages/plugin-detail/src/__tests__/RelatedList.emptyPlaceholderAffordance-8475.test.tsx +++ b/packages/plugin-detail/src/__tests__/RelatedList.emptyPlaceholderAffordance-8475.test.tsx @@ -38,18 +38,24 @@ * italic` that the shared component does not have. `THE AGREEMENT` below is * that exact pair, in one column, in one render. * - * ## Which cases DISCRIMINATE, and which are scope declarations + * ## Which cases DISCRIMINATE — MEASURED, not predicted * - * An implementation that renders `EmptyValue` **everywhere, including for - * filled cells**, passes `THE DEFECT` and passes `THE AGREEMENT` — "the cell - * has an accessible name" is true of a list that has stopped rendering values. - * The single case that REFUSES it is `NON-REGRESSION — a FILLED cell`, which - * asserts both that the value is present AND that no placeholder shares that - * cell. + * The caricature was RUN, not reasoned about: `EmptyValue` returned + * unconditionally from `makeCell`, filled cells included. All three cases go + * red — but only ONE of them through its headline assertion. * - * `THE AGREEMENT` is kept as a SCOPE DECLARATION about the visual half rather - * than shipped as if it discriminated: it is the only case pinning that the - * `text-xs italic` treatment is deliberately gone. + * - `NON-REGRESSION — a FILLED cell` refuses it directly: it asserts both + * that the value is present AND that no placeholder shares that cell. + * - `THE DEFECT` and `THE AGREEMENT` go red only because the harness waits + * for a real value ("Widget") to reach the table and it never arrives. + * That is what the control is for, and it is worth distinguishing: their + * headline assertions — "the empty cell has an accessible name", "the two + * branches draw the same thing" — are both TRUE of a list that has given + * up on values entirely. + * + * So `THE AGREEMENT` is a SCOPE DECLARATION about the visual half, kept + * because it is the only case pinning that the `text-xs italic` treatment is + * deliberately gone, and labelled rather than quoted as proof of the fix. * * ## The viewport is pinned on purpose (objectui#8399) * diff --git a/packages/plugin-grid/src/__tests__/ObjectGrid.emptyPlaceholderAffordance-8491.test.tsx b/packages/plugin-grid/src/__tests__/ObjectGrid.emptyPlaceholderAffordance-8491.test.tsx index b4a7850cb2..6e327ba54e 100644 --- a/packages/plugin-grid/src/__tests__/ObjectGrid.emptyPlaceholderAffordance-8491.test.tsx +++ b/packages/plugin-grid/src/__tests__/ObjectGrid.emptyPlaceholderAffordance-8491.test.tsx @@ -52,34 +52,45 @@ * three affordances — and `THE DRAWER — the rendered text is unchanged` is the * case that holds that line. * - * ## Which cases DISCRIMINATE, and which are scope declarations + * ## Which cases DISCRIMINATE — MEASURED, not predicted * * The lesson objectui#8474 and objectui#8481 each measured independently: the * most quotable assertion in a pin is usually the one that cannot tell the fix - * from its worst caricature. Stated explicitly here rather than implied. + * from its worst caricature. So the caricature was RUN, not reasoned about — + * `EmptyValue` rendered unconditionally at all four sites, filled cells + * included. Result: 8 of these 12 cases red, 4 GREEN. * - * An implementation that renders `EmptyValue` **everywhere, including for - * filled cells**, still passes every `THE DEFECT` case and both `AGREEMENT` - * cases below — "the cell has an accessible name" is true of a grid that has - * given up on values entirely. + * The four that a give-up-on-values implementation still passes, labelled here + * rather than shipped as if they proved something: * - * What REFUSES that caricature, and nothing else in this file does: + * - `THE DEFECT — an empty LINK+ACTION cell` + * - `THE DEFECT — the auto-linked PRIMARY cell` + * - `AGREEMENT — the linked branch and the no-renderer default branch` + * - `MOBILE CARD VIEW — the card layout OMITS an empty field` + * + * The first two are the vivid ones. "The cell has an accessible name" is true + * of a grid that has stopped rendering values at all. The third is a scope + * declaration about typography — it is the only case pinning the deliberate + * visual change, which is why it is kept. The fourth is a scope declaration + * about a path this change does not touch. + * + * What REFUSES the caricature, by asserting BOTH that the value is present AND + * that no placeholder shares its cell: * * - `NON-REGRESSION — a FILLED linked cell` * - `NON-REGRESSION — a FILLED link+action cell` * - `NON-REGRESSION — a FILLED primary cell` - * - `NON-REGRESSION — a FILLED card field` (mobile) + * - `NON-REGRESSION — a FILLED card field` * - `THE DRAWER — a FILLED field` * - * Each asserts BOTH that the value is present AND that no placeholder is in - * that same cell. The value half alone is not enough: an implementation that - * appends a placeholder next to every value would pass it. + * `THE DEFECT — an empty LINK cell` and both empty-drawer cases also go red + * under the caricature, but through their value-bearing CONTROLS rather than + * their headline assertion. That is what the controls are for, and it is worth + * distinguishing: without them those three would have joined the green four. * - * `AGREEMENT — the two branches draw the identical placeholder` is a SCOPE - * DECLARATION about typography, not an instrument against the caricature — it - * is green for `EmptyValue`-everywhere. It is kept because it is the only - * thing pinning the deliberate visual change, and it is labelled rather than - * shipped as if it proved more. + * `THE DRAWER — the rendered TEXT is unchanged` is the one instrument for the + * glyph decision: it is the ONLY case that reddens when the drawer adopts the + * bare shared component and strands `grid.empty`. * * ## Every DOM lookup is scoped to ONE row or ONE card *