From 7bfcb8cd84f39e42e36b3fa3636e2d4cda3b8053 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 06:02:48 +0000 Subject: [PATCH 1/2] fix(plugin-detail): DetailSection and HeaderHighlight draw the shared EmptyValue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both files spelled their own `—` for a missing value and resolved their own accessible name from `detail.noValue`. `@object-ui/components`' `EmptyValue` resolves EXACTLY that key, with the same "No value" English fallback, through the provider-safe `useObjectTranslation` — so the accessible name is byte-identical before and after in every locale, and the placeholder gains `data-slot="empty-value"`, `no-underline` and the shared `select-none`. Two per-site decisions, argued rather than averaged: - Both retire their own `text-muted-foreground/60 text-sm` and take the shared `/50`. Not a neutral difference: a type-aware cell renderer already draws the same shared component at /50 in the very next row or chip (an unparseable `datetime` is the reachable case), so one section could show two dashes in two greys. - `DetailSection` keeps its `title` and keeps it ALIVE. The title arrived in the same commit as the aria-label and is the sighted-mouse counterpart of it; two landed pins read it as the only instrument telling this placeholder apart from a cell renderer's. It rides through `...props`, but the shared component is `pointer-events-none` and a title on a non-hoverable element never renders a tooltip, so this one site restores `pointer-events-auto`. `HeaderHighlight` has no title and takes the shared non-interactive default unchanged; it keeps only `block`, which is layout, not typography. `detailPage.emptinessAuthority-8394`'s strip instrument is re-derived: it read `[aria-label="No value"]:not([data-slot="empty-value"])`, i.e. it navigated by exactly the thing this change moves, so it failed over a correct surface. It now anchors on the field LABEL and asserts WHICH chip draws the affordance. `EmptyValue`'s docblock stops calling its glyph default an "en-dash"; it is and always was U+2014. Comment only — measured code-identical after comment strip. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- ...-detail-placeholders-shared-empty-value.md | 30 ++ packages/components/src/custom/empty.tsx | 9 +- packages/plugin-detail/src/DetailSection.tsx | 33 +- .../plugin-detail/src/HeaderHighlight.tsx | 27 +- ...etailPage.emptinessAuthority-8394.test.tsx | 51 ++- ...laceholders.sharedEmptyValue-8506.test.tsx | 377 ++++++++++++++++++ ...holders.sharedEmptyValueI18n-8506.test.tsx | 164 ++++++++ 7 files changed, 658 insertions(+), 33 deletions(-) create mode 100644 .changeset/8506-detail-placeholders-shared-empty-value.md create mode 100644 packages/plugin-detail/src/__tests__/detailPlaceholders.sharedEmptyValue-8506.test.tsx create mode 100644 packages/plugin-detail/src/__tests__/detailPlaceholders.sharedEmptyValueI18n-8506.test.tsx diff --git a/.changeset/8506-detail-placeholders-shared-empty-value.md b/.changeset/8506-detail-placeholders-shared-empty-value.md new file mode 100644 index 0000000000..76dc212a55 --- /dev/null +++ b/.changeset/8506-detail-placeholders-shared-empty-value.md @@ -0,0 +1,30 @@ +--- +'@object-ui/plugin-detail': patch +'@object-ui/components': patch +--- + +fix(plugin-detail): the record page's two hand-rolled empty placeholders become the shared `EmptyValue` + +`DetailSection` (the details body grid) and `HeaderHighlight` (the ADR-0085 +highlights strip) each spelled their own `—` for a missing value and +resolved their own accessible name from `detail.noValue`. Both now render +`@object-ui/components`' `EmptyValue`, which resolves exactly that key with the +same `"No value"` English fallback through a provider-safe hook — so the +accessible name is byte-identical in every locale, and the placeholder gains the +shared `data-slot="empty-value"`, `no-underline` and `select-none` treatment. + +Two deliberate visual changes, decided per site: + +- Both retire their own `text-muted-foreground/60 text-sm` treatment and take the + shared `text-muted-foreground/50`. That was not a neutral difference: a + type-aware cell renderer already draws the same shared component at `/50` in + the very next row or chip (an unparseable `datetime` is the reachable case), so + one section could show two dashes in two greys. +- `DetailSection` keeps its `title` hover affordance, and keeps it working: the + shared component is `pointer-events-none`, on which a `title` never renders a + tooltip, so that one site restores `pointer-events-auto`. `HeaderHighlight` has + no `title` and takes the shared non-interactive default unchanged. + +`EmptyValue`'s docblock also stops calling its `glyph` default an "en-dash"; it +is and always was U+2014, an em-dash. Comment only — no behaviour change in +`@object-ui/components`. diff --git a/packages/components/src/custom/empty.tsx b/packages/components/src/custom/empty.tsx index 8c2f741d2a..e450238067 100644 --- a/packages/components/src/custom/empty.tsx +++ b/packages/components/src/custom/empty.tsx @@ -116,9 +116,12 @@ function EmptyContent({ className, ...props }: React.ComponentProps<"div">) { * EmptyValue — universal inline placeholder for missing cell/field values. * * Use this anywhere a renderer would otherwise show "-" or "—" for a null, - * undefined or empty value. It renders a muted, non-interactive en-dash that - * does not inherit link/button colors from surrounding ancestors, so a missing - * value never looks clickable. + * undefined or empty value. It renders a muted, non-interactive EM-dash (the + * `glyph` default below is U+2014, and every call site depends on that width) + * that does not inherit link/button colors from surrounding ancestors, so a + * missing value never looks clickable. This sentence said "en-dash" until + * objectui#8506: the word was wrong, never the code — do not "fix" the glyph + * to match a stale docblock. */ function EmptyValue({ className, diff --git a/packages/plugin-detail/src/DetailSection.tsx b/packages/plugin-detail/src/DetailSection.tsx index 02c003676f..0e28e7a9db 100644 --- a/packages/plugin-detail/src/DetailSection.tsx +++ b/packages/plugin-detail/src/DetailSection.tsx @@ -17,6 +17,7 @@ import { CollapsibleTrigger, CollapsibleContent, Button, + EmptyValue, Tooltip, TooltipContent, TooltipProvider, @@ -336,14 +337,34 @@ export const DetailSection: React.FC = ({ } const isEmpty = !hasCellValue(value); if (isEmpty) { + // The SHARED placeholder (objectui#8506). The span that stood here + // spelled its own em-dash and resolved its own `aria-label` from + // `detail.noValue`; `EmptyValue` resolves EXACTLY that key, with the + // same `"No value"` English fallback, through a provider-safe hook — so + // the accessible name survives byte-for-byte in every locale and the + // duplicate resolution goes away. Two deliberate props: + // + // - `title` — the sighted-mouse counterpart of the accessible name, + // added alongside it in the same commit. It rides through + // `EmptyValue`'s `...props`. `pointer-events-auto` is what keeps it + // a real tooltip: the shared component sets `pointer-events-none`, + // which stops the placeholder being a hit target, and a `title` on + // an element that can never be hovered is a dead attribute — the + // hover would fall through to this row's own + // `title={t('detail.editInlineHint')}` instead. It is also the only + // instrument that tells THIS placeholder apart from the one a cell + // renderer draws one row over (`DetailSection.emptinessAuthority-8376`, + // `record-details.emptySectionDefault`), which read it by title. + // - no `className` typography — the retired `text-muted-foreground/60 + // text-sm` is a deliberate change, not an oversight: it made this + // row's dash a different grey and a different size from the dash + // `DateTimeCellRenderer` draws for an unparseable value in the very + // next row of the same section. They now draw identically. return ( - - — - + /> ); } // Use type-aware cell renderer; respect format hints (e.g. diff --git a/packages/plugin-detail/src/HeaderHighlight.tsx b/packages/plugin-detail/src/HeaderHighlight.tsx index 8273834c92..88a2d6565f 100644 --- a/packages/plugin-detail/src/HeaderHighlight.tsx +++ b/packages/plugin-detail/src/HeaderHighlight.tsx @@ -10,6 +10,7 @@ import * as React from 'react'; import { cn, Button, + EmptyValue, Tooltip, TooltipContent, TooltipProvider, @@ -249,12 +250,26 @@ export const HeaderHighlight: React.FC = ({ ) ) : isEmpty ? ( - - — - + // The SHARED placeholder (objectui#8506). This span + // spelled its own em-dash and resolved its own + // `aria-label` from `detail.noValue`; `EmptyValue` + // resolves EXACTLY that key with the same `"No value"` + // English fallback, so the accessible name survives + // byte-for-byte in every locale. Unlike the body grid's + // placeholder this one carries NO `title`, so nothing + // here needs `pointer-events` back — the strip's chip + // keeps its own `onDoubleClick`, which fires from the + // chip, not from the dash. + // + // `block` is layout, not typography: the filled branch + // below is a `block` span inside the same + // `min-w-0 flex-1` box, and an inline dash would sit on + // a different baseline. The retired + // `text-sm text-muted-foreground/60` IS a deliberate + // typography change — it made the strip's own dash a + // different grey from the one a cell renderer draws for + // a chip two columns over. + ) : ( {label}` followed by the value slot, so + * `chipOf(label)` is the chip box and the affordance is read INSIDE it. That + * also strengthens the assertion from "how many" to "which chip", the way the + * summary chips below are already read through the `aria-label=": + * "` each `Badge` carries. */ import { describe, it, expect, beforeAll, afterEach } from 'vitest'; @@ -90,9 +98,16 @@ afterEach(cleanup); * CI summary would carry none of the reason. */ const shown = (text: string) => screen.queryByText(text) !== null; -/** `HeaderHighlight`'s OWN em-dash affordance — see the docblock. */ -const stripAffordances = (c: HTMLElement) => - c.querySelectorAll('[aria-label="No value"]:not([data-slot="empty-value"])'); +/** + * The chip box a field's LABEL sits in — the strip renders the label as the + * chip's first child. Anchored on the label, never on the placeholder: see the + * docblock. + */ +const chipOf = (label: string) => screen.getByText(label).parentElement as HTMLElement; + +/** WHICH of `labels`' chips draw the em-dash affordance — see the docblock. */ +const chipsDrawingAffordance = (labels: string[]) => + labels.filter((l) => chipOf(l).querySelector('[data-slot="empty-value"]') !== null); /** The summary chip for `field`, read by the `aria-label` the Badge carries. */ const chipFor = (c: HTMLElement, field: string) => @@ -146,9 +161,9 @@ describe('HeaderHighlight — the ADR-0085 strip trims (#8394)', () => { expect(shown('Notes'), 'CONTROL: the whitespace-only chip is on screen at all').toBe(true); expect( - stripAffordances(container), + chipsDrawingAffordance(['Industry', 'Notes', 'Amount']), 'exactly the whitespace-only chip draws the strip\'s `No value` em-dash', - ).toHaveLength(1); + ).toEqual(['Notes']); expect( container.textContent, 'the raw whitespace must not reach the DOM as a rendered value', @@ -180,15 +195,15 @@ describe('HeaderHighlight — the ADR-0085 strip trims (#8394)', () => { expect(shown('Alpha'), 'a multiselect array renders its options').toBe(true); expect(shown('Beta'), 'a multiselect array renders its options').toBe(true); expect( - stripAffordances(container), + chipsDrawingAffordance(['Office Location', 'Tags', 'Industry']), 'NO chip here is empty — an object value is a value on this surface', - ).toHaveLength(0); + ).toEqual([]); }); it('NON-REGRESSION — an emptiness test that answers EMPTY for everything is refused (`0` and a string)', () => { // Every whitespace case above is ALSO satisfied by deleting the feature. // This one is not: `0` is the value a careless `!value` rewrite loses. - const { container } = render( + render( { expect(shown('Manufacturing'), 'a plain string is a value').toBe(true); expect(shown('0'), '`0` is a value, not a blank').toBe(true); expect( - stripAffordances(container), + chipsDrawingAffordance(['Industry', 'Amount']), 'no chip draws the em-dash — nothing here is empty', - ).toHaveLength(0); + ).toEqual([]); }); }); diff --git a/packages/plugin-detail/src/__tests__/detailPlaceholders.sharedEmptyValue-8506.test.tsx b/packages/plugin-detail/src/__tests__/detailPlaceholders.sharedEmptyValue-8506.test.tsx new file mode 100644 index 0000000000..da738cf6b2 --- /dev/null +++ b/packages/plugin-detail/src/__tests__/detailPlaceholders.sharedEmptyValue-8506.test.tsx @@ -0,0 +1,377 @@ +/** + * 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. + */ + +/** + * `DetailSection` and `HeaderHighlight` draw the SHARED `EmptyValue` + * (objectui#8506). + * + * ## What changed, and what did NOT + * + * Both files hand-rolled a `—` that resolved its own accessible + * name from `detail.noValue`. `@object-ui/components`' `EmptyValue` resolves + * EXACTLY that key, with the same `"No value"` English fallback, through the + * provider-safe `useObjectTranslation` — so the accessible name is byte-identical + * before and after in every locale. The card that filed this feared the opposite + * ("a translated label traded for an untranslated one"); the sibling file + * `detailPlaceholders.sharedEmptyValueI18n-8506.test.tsx` measures the locale + * half in `zh` and `de`, and the PROVIDER-LESS case below measures the fallback + * half here, in a file that mounts no provider at all (see that file's docblock + * for why the two legs cannot share one file). + * + * ## The two per-site decisions this file pins + * + * 1. ⭐ **Typography: both sites take the shared treatment, deliberately.** Each + * retired a `text-muted-foreground/60 text-sm` spelling of its own. That was + * not a neutral difference: a type-aware cell renderer draws the SAME shared + * `EmptyValue` at `/50` for a value `hasCellValue` waves through but the + * renderer cannot draw — an unparseable `datetime` is the reachable example + * (measured on objectui#8503, whose card had attributed the branch to + * `DateCellRenderer`; it is `DateTimeCellRenderer`'s). So one section could + * show two dashes in two greys, one row apart. `THE AGREEMENT` is that exact + * pair, in one render. + * + * 2. ⭐ **`DetailSection` keeps its `title`, and keeps it ALIVE.** The `title` + * arrived in the same commit as the `aria-label` (`7ca7203ec`) — it is the + * sighted-mouse counterpart of the accessible name — and two landed pins read + * it as the only instrument that tells this placeholder apart from a cell + * renderer's (`DetailSection.emptinessAuthority-8376`, + * `record-details.emptySectionDefault`). It rides through `EmptyValue`'s + * `...props`. But the shared component sets `pointer-events-none`, and a + * `title` on an element that can never be hovered is a dead attribute: the + * hover falls through to the row's own `title={t('detail.editInlineHint')}`. + * Hence `pointer-events-auto` at that site only — `HeaderHighlight` has no + * `title`, so it takes `pointer-events-none` as-is. `THE TOOLTIP IS ALIVE` + * pins that, and it is the case the PLAUSIBLE WRONG FIX below reddens. + * + * ## Which cases DISCRIMINATE — MEASURED, not predicted + * + * Three ablations were RUN against this file, each proved on disk in both + * directions and restored by state. The results, not the predictions: + * + * - **THE CARICATURE** — `EmptyValue` returned unconditionally from both sites, + * filled values included. ⭐ The FIRST run reddened 7 of 8 cases and **not one + * of them through its headline assertion**: every failure came out of a + * CONTROL ("CONTROL: the filled text row rendered"), because a surface that + * draws nothing but placeholders satisfies "the empty row has an accessible + * name", "the title survives" and "the two branches agree" perfectly well — + * and the 8th, the provider-less fallback, survived outright. That reading is + * why this file is shaped the way it is: the two NON-REGRESSION cases now + * state `drawingAffordance([...])` FIRST, ahead of their own controls, + * because it is the only assertion here that refuses the caricature on its + * own terms; and the provider-less case gained a value control and a label + * saying what it does not decide. RE-MEASURED after that change: 8 of 8, with + * the two `drawingAffordance` cases failing on their HEADLINE and naming the + * rows that wrongly drew a dash (`expected ['industry','notes','amount'] to + * deeply equal ['notes']`). The other six still fail through a control, which + * is what a control is for; they are scope declarations on this axis. + * - **THE PLAUSIBLE WRONG FIX** — the swap made, but each site's own + * `className` dropped (`pointer-events-auto`, `block`). This is the leg that + * matters, because it is what a mechanical swap actually produces and it is + * invisible to every assertion that only reads the accessible name. Exactly + * three cases refuse it, all three through their HEADLINE: `THE TOOLTIP IS + * ALIVE`, `THE AGREEMENT`, `THE LAYOUT CLASS SURVIVES`. + * - **THE HARNESS KILL** — the field LABEL removed from both surfaces. Every + * lookup here is anchored on that label, never on the placeholder, precisely + * so a mutation cannot erase what the harness navigates by (objectui#8504). + * All 8 cases redden, and LOUDLY — `Unable to find an element with the text: + * industry` — so the anchor is load-bearing rather than decorative, and no + * case here can pass over a surface that stopped rendering. + * + * ## Reading the DOM + * + * ⚠️ `queryByText` throws on MULTIPLE matches as well as on ambiguity, and the + * failure surfaces as a `waitFor`/assertion pointing at the component rather + * than at the query. Every lookup below is either scoped to one row/chip first + * or counted with `queryAllByText`. + */ + +import { describe, it, expect, beforeAll, afterEach } from 'vitest'; +import { render, screen, cleanup, within } from '@testing-library/react'; +import * as React from 'react'; +import { DetailSection } from '../DetailSection'; +import { HeaderHighlight } from '../HeaderHighlight'; +import type { DetailViewSection } from '@object-ui/types'; + +/** + * Desktop. `DetailSection` renders an entirely different iOS-style row on + * mobile, and `HeaderHighlight`'s chip widths switch too — pinned rather than + * inherited from happy-dom's default so nothing below is green only because of + * an unpinned viewport (objectui#8399). + */ +beforeAll(() => { + Object.defineProperty(window, 'innerWidth', { configurable: true, value: 1280 }); +}); + +afterEach(cleanup); + +const objectSchema = { + fields: { + industry: { type: 'text', label: 'Industry' }, + notes: { type: 'text', label: 'Notes' }, + amount: { type: 'number', label: 'Amount' }, + when: { type: 'datetime', label: 'When' }, + }, +}; + +/** + * The row/chip a field's LABEL sits in. + * + * ⭐ THE ANCHOR. `DetailSection` renders `
{LABEL}
` as the first child + * of the row box; `HeaderHighlight` renders `{LABEL}` as the first + * child of the chip box. Neither is touched by the placeholder swap, by the + * caricature, or by the wrong fix — which is the whole point: an anchor the + * mutation can erase makes the harness die first and the pin measure nothing + * while reading as a strong refusal. + */ +const boxOf = (label: string) => screen.getByText(label).parentElement as HTMLElement; + +/** The shared placeholder inside ONE row/chip, or null. */ +const emptyIn = (el: HTMLElement): HTMLElement | null => + el.querySelector('[data-slot="empty-value"]'); + +/** + * WHICH of `labels`' rows/chips draw the placeholder — never merely how many. + * Anchored on the label like everything else here, so the caricature cannot + * satisfy it by drawing placeholders and nothing else. + */ +const drawingAffordance = (labels: string[]) => + labels.filter((l) => emptyIn(boxOf(l)) !== null); + +/** Class tokens of one element, as a set. */ +const classesOf = (el: HTMLElement) => new Set(el.className.split(/\s+/).filter(Boolean)); + +/** Tokens in exactly one of the two sets. */ +const symmetricDifference = (a: Set, b: Set) => + [...new Set([...a, ...b])].filter((t) => a.has(t) !== b.has(t)).sort(); + +const sectionOf = (fields: string[]): DetailViewSection => + ({ title: 'Details', fields: fields.map((name) => ({ name })) }) as DetailViewSection; + +const renderSection = (fields: string[], data: Record) => + render(); + +const renderStrip = (fields: string[], data: Record) => + render( + ({ + name, + label: (objectSchema.fields as Record)[name].label, + })) as any + } + data={data} + objectSchema={objectSchema} + />, + ); + +describe('DetailSection — the body grid draws the shared EmptyValue (#8506)', () => { + it('THE DEFECT — the empty row draws the SHARED placeholder, and keeps its accessible name', () => { + renderSection(['industry', 'notes', 'amount'], { + industry: 'Manufacturing', + notes: '', + amount: 42, + }); + + // CONTROLS — the section rendered, and the sibling rows rendered BY VALUE. + // Without these, a section that gave up on values entirely passes below. + expect(screen.queryAllByText('Details').length, 'CONTROL: the section heading rendered') + .toBeGreaterThan(0); + expect(within(boxOf('industry')).queryByText('Manufacturing'), 'CONTROL: the filled text row') + .not.toBeNull(); + expect(within(boxOf('amount')).queryByText('42'), 'CONTROL: the filled number row').not.toBeNull(); + + const placeholder = emptyIn(boxOf('notes')); + expect(placeholder, 'the empty row draws the shared component, not a hand-rolled span') + .not.toBeNull(); + expect( + placeholder!.getAttribute('aria-label'), + 'the accessible name survives the swap — a word, never a naked punctuation mark', + ).toBe('No value'); + expect(placeholder!.textContent, 'and it is still the EM-dash the sites always drew').toBe('—'); + }); + + it('⭐ THE TOOLTIP IS ALIVE — the title survives AND can still be hovered', () => { + renderSection(['industry', 'notes'], { industry: 'Manufacturing', notes: '' }); + expect(within(boxOf('industry')).queryByText('Manufacturing'), 'CONTROL: the filled row') + .not.toBeNull(); + + const placeholder = emptyIn(boxOf('notes'))!; + expect(placeholder, 'CONTROL: the placeholder is on screen').not.toBeNull(); + expect( + placeholder.getAttribute('title'), + 'the deliberate hover affordance rides through `...props`', + ).toBe('No value'); + // THE DISCRIMINATING HALF. `EmptyValue` ships `pointer-events-none`; a + // `title` on a non-hoverable element never renders a tooltip, so keeping + // the attribute without keeping the pointer events keeps a DEAD attribute + // that two landed pins would go on reading as if it were alive. + const classes = classesOf(placeholder); + expect(classes.has('pointer-events-auto'), 'the site restores pointer events for its title') + .toBe(true); + expect(classes.has('pointer-events-none'), 'and `cn` collapsed the shared default away') + .toBe(false); + }); + + it('⭐ THE AGREEMENT — the section\'s own branch and a cell renderer\'s draw the same placeholder', () => { + // Row 0 takes `DetailSection`'s own `!hasCellValue` branch. Row 1 is NOT + // empty to `hasCellValue` and reaches `DateTimeCellRenderer`, whose own + // empty branch has always returned the shared component. Before this + // change those two dashes were different greys, one row apart. + renderSection(['notes', 'when', 'industry'], { + notes: '', + when: 'not-a-date', + industry: 'Manufacturing', + }); + + const ownBranch = emptyIn(boxOf('notes')); + const rendererBranch = emptyIn(boxOf('when')); + expect(ownBranch, "the section's own branch drew a placeholder").not.toBeNull(); + expect(rendererBranch, "CONTROL: the renderer's branch drew one too").not.toBeNull(); + // CONTROL — the section still renders real values, so this is not a + // section that gave up: a filled row is right there. + expect(within(boxOf('industry')).queryByText('Manufacturing'), 'CONTROL: a filled row') + .not.toBeNull(); + + expect( + classesOf(ownBranch!).has('text-muted-foreground/50'), + 'the section now paints the SHARED grey', + ).toBe(true); + expect( + ownBranch!.className, + 'and the retired `text-muted-foreground/60 text-sm` treatment is gone', + ).not.toContain('/60'); + expect( + symmetricDifference(classesOf(ownBranch!), classesOf(rendererBranch!)), + 'the ONLY class the two branches disagree on is the non-visual pointer-events pair', + ).toEqual(['pointer-events-auto', 'pointer-events-none']); + }); + + it('⭐ NON-REGRESSION — exactly the EMPTY row draws the placeholder (the caricature refusal)', () => { + renderSection(['industry', 'notes', 'amount'], { + industry: 'Manufacturing', + notes: '', + amount: 0, + }); + + // STATED FIRST, deliberately, and the ordering was MEASURED rather than + // chosen. With the controls ahead of it, the caricature reddened every case + // in this file THROUGH a control and not one headline assertion ever ran — + // so the CI summary for a placeholder defect read "CONTROL: the filled text + // row rendered". This is the assertion that should name it, so it goes + // first; `0` is also the value a careless `!value` rewrite loses. + expect( + drawingAffordance(['industry', 'notes', 'amount']), + 'exactly the empty row draws the placeholder — a filled row draws none', + ).toEqual(['notes']); + + // CONTROLS — after it, so a section that rendered nothing is still refused. + expect(within(boxOf('industry')).queryByText('Manufacturing'), 'CONTROL: the value reaches the filled row') + .not.toBeNull(); + expect(within(boxOf('amount')).queryByText('0'), 'CONTROL: `0` is a value, not a blank').not.toBeNull(); + }); +}); + +describe('HeaderHighlight — the ADR-0085 strip draws the shared EmptyValue (#8506)', () => { + it('THE DEFECT — the empty chip draws the SHARED placeholder, and keeps its accessible name', () => { + renderStrip(['industry', 'notes', 'amount'], { + industry: 'Manufacturing', + notes: ' ', + amount: 42, + }); + + // CONTROLS — the strip rendered its neighbours BY VALUE. + expect(within(boxOf('Industry')).queryByText('Manufacturing'), 'CONTROL: the filled text chip') + .not.toBeNull(); + expect(within(boxOf('Amount')).queryByText('42'), 'CONTROL: the filled number chip').not.toBeNull(); + + const placeholder = emptyIn(boxOf('Notes')); + expect(placeholder, 'the empty chip draws the shared component').not.toBeNull(); + expect( + placeholder!.getAttribute('aria-label'), + 'the accessible name survives the swap', + ).toBe('No value'); + expect(placeholder!.textContent, 'still the EM-dash').toBe('—'); + // The strip carries no `title` of its own and never did, so it takes the + // shared non-interactive default unmodified. + expect(placeholder!.getAttribute('title'), 'the strip has no tooltip to preserve').toBeNull(); + }); + + it('⭐ THE LAYOUT CLASS SURVIVES — `block` is layout, not typography', () => { + renderStrip(['industry', 'notes'], { industry: 'Manufacturing', notes: ' ' }); + expect(within(boxOf('Industry')).queryByText('Manufacturing'), 'CONTROL: the filled chip') + .not.toBeNull(); + + const placeholder = emptyIn(boxOf('Notes'))!; + expect(placeholder, 'CONTROL: the placeholder is on screen').not.toBeNull(); + // THE DISCRIMINATING HALF for a swap that drops the site's own className: + // the filled branch beside it is a `block` span in the same `min-w-0 + // flex-1` box, and an inline dash sits on a different baseline. + expect(classesOf(placeholder).has('block'), 'the chip keeps its block display').toBe(true); + expect( + classesOf(placeholder).has('text-muted-foreground/50'), + 'and takes the SHARED grey — the retired /60 is a deliberate change', + ).toBe(true); + expect(placeholder.className, 'the retired treatment is gone').not.toContain('/60'); + }); + + it('⭐ NON-REGRESSION — exactly the EMPTY chip draws the placeholder (the caricature refusal)', () => { + renderStrip(['industry', 'notes', 'amount'], { + industry: 'Manufacturing', + notes: ' ', + amount: 0, + }); + + // First for the same measured reason as its twin above. + expect( + drawingAffordance(['Industry', 'Notes', 'Amount']), + 'exactly the empty chip draws the placeholder — a filled chip draws none', + ).toEqual(['Notes']); + + // CONTROLS — after it. + expect(within(boxOf('Industry')).queryByText('Manufacturing'), 'CONTROL: the value reaches the filled chip') + .not.toBeNull(); + expect(within(boxOf('Amount')).queryByText('0'), 'CONTROL: `0` is a value, not a blank').not.toBeNull(); + }); +}); + +describe('The provider-less path — no I18nProvider is mounted in this FILE (#8506)', () => { + /** + * The half an `en` locale can never decide, stated from the other side. + * + * The old spelling read `t('detail.noValue', { defaultValue: 'No value' })` + * through `useDetailTranslation`, whose `DETAIL_DEFAULT_TRANSLATIONS` map + * carries the row. `EmptyValue` reads the bare key through + * `useObjectTranslation` and maps the raw-key answer to `"No value"` itself. + * Two different mechanisms, one required outcome: never a raw key on screen. + */ + it('BOTH sites announce the English fallback, never the raw key', () => { + // ⚠️ SCOPE DECLARATION on the caricature axis, MEASURED: this case was the + // ONE survivor of `EmptyValue`-everywhere, and it should be — "the name is + // English, not a raw key" is perfectly true of a surface that draws nothing + // but placeholders. It is kept for the fallback half only. The control + // below was added afterwards so it is at least not vacuous, and the + // harness-kill leg reddens it loudly. + renderSection(['industry', 'notes'], { industry: 'Manufacturing', notes: '' }); + expect(within(boxOf('industry')).queryByText('Manufacturing'), 'CONTROL: the section rendered a real value') + .not.toBeNull(); + const bodyName = emptyIn(boxOf('notes'))!.getAttribute('aria-label'); + cleanup(); + + renderStrip(['industry', 'notes'], { industry: 'Manufacturing', notes: ' ' }); + expect(within(boxOf('Industry')).queryByText('Manufacturing'), 'CONTROL: the strip rendered a real value') + .not.toBeNull(); + const stripName = emptyIn(boxOf('Notes'))!.getAttribute('aria-label'); + + expect(bodyName, 'the body grid falls back to English').toBe('No value'); + expect(stripName, 'and so does the strip').toBe('No value'); + // Stated separately because it is the failure this could actually have: + // a provider-less host rendering the i18n key into the accessible name. + expect(bodyName, 'never the raw key').not.toBe('detail.noValue'); + expect(stripName, 'never the raw key').not.toBe('detail.noValue'); + }); +}); diff --git a/packages/plugin-detail/src/__tests__/detailPlaceholders.sharedEmptyValueI18n-8506.test.tsx b/packages/plugin-detail/src/__tests__/detailPlaceholders.sharedEmptyValueI18n-8506.test.tsx new file mode 100644 index 0000000000..bad2221327 --- /dev/null +++ b/packages/plugin-detail/src/__tests__/detailPlaceholders.sharedEmptyValueI18n-8506.test.tsx @@ -0,0 +1,164 @@ +/** + * 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. + */ + +/** + * The accessible name of the shared placeholder is TRANSLATED at both detail + * sites (objectui#8506). + * + * ## Why an `en` test cannot decide this + * + * objectui#8506 was filed as a decision — "adopting the shared `EmptyValue` + * would trade a translated label for an untranslated one" — and the card names + * the reason no English test could settle it: `detail.noValue`'s `en` value is + * `"No value"`, byte-identical to the literal the old inline + * `t('detail.noValue', { defaultValue: 'No value' })` would have fallen back to. + * An `en` assertion is green whether the name comes from the pack or from a + * hardcoded string. That trap is written up on objectui#7173, in one of these + * very packages. + * + * So the measurement is made where the two answers differ: + * + * - **`zh`** — `无`. A non-Latin pack, where a value byte-identical to `en` + * would be decidable evidence of an untranslated string. + * - **`de`** — `Kein Wert`. A Latin pack, where identity CAN be genuine, so + * the assertion names the German words rather than merely "not English". + * + * Expectations are written as LITERALS, never read back out of the pack: a test + * that resolves its expectation through the table under test passes against any + * table, including an empty one. + * + * ## Why the provider-less leg is a different FILE + * + * `createI18n` registers its instance as react-i18next's module-global default, + * and the registration survives unmount and `cleanup()`. A "no provider" + * assertion placed HERE would silently resolve against whichever language ran + * last. That leg lives in `detailPlaceholders.sharedEmptyValue-8506.test.tsx`, + * which mounts no provider at all. (Same split, same reason, as + * `config-panel-footer-i18n-4750` / `-no-provider-4750`.) + * + * ## ⚠️ What this file does NOT decide — measured, not assumed + * + * Under the caricature run against the sibling file (`EmptyValue` returned + * unconditionally from both sites, filled values included) all three cases here + * go red — but every one of them through its CONTROL ("CONTROL: the filled row + * rendered"), never through its headline. That is correct and worth writing + * down: `aria-label === '无'` is perfectly true of a surface that has given up + * on values entirely. These cases are a SCOPE DECLARATION about the locale axis; + * the case that refuses an `EmptyValue`-everywhere implementation on its own + * terms is `drawingAffordance([...])` in the sibling file. + * + * ## What the two sites resolve THROUGH, and why that is worth pinning + * + * They no longer resolve it themselves. `EmptyValue` calls + * `useObjectTranslation` and reads the bare `detail.noValue`; `DetailSection`'s + * surviving `title` still calls `useDetailTranslation`, which is + * `createSafeTranslation(DETAIL_DEFAULT_TRANSLATIONS, 'detail.back')`. Two + * different hooks over one key: `THE TWO PATHS AGREE` asserts they land on the + * same bytes, which is the property that makes the surviving `title` safe. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { render, screen, cleanup, within } from '@testing-library/react'; +import * as React from 'react'; +import { I18nProvider } from '@object-ui/i18n'; +import { DetailSection } from '../DetailSection'; +import { HeaderHighlight } from '../HeaderHighlight'; +import type { DetailViewSection } from '@object-ui/types'; + +afterEach(cleanup); + +const objectSchema = { + fields: { + industry: { type: 'text', label: 'Industry' }, + notes: { type: 'text', label: 'Notes' }, + }, +}; + +/** See the sibling file: the anchor is the field LABEL, never the placeholder. */ +const boxOf = (label: string) => screen.getByText(label).parentElement as HTMLElement; + +const emptyIn = (el: HTMLElement): HTMLElement | null => + el.querySelector('[data-slot="empty-value"]'); + +const inLocale = (language: string, ui: React.ReactElement) => + render( + + {ui} + , + ); + +const section = { title: 'Details', fields: [{ name: 'industry' }, { name: 'notes' }] } as DetailViewSection; + +const body = ( + +); + +const strip = ( + +); + +describe('The shared placeholder announces in the session language (#8506)', () => { + it('zh — both sites announce `无`, not `No value`', () => { + inLocale('zh', body); + // CONTROL — the surface rendered a real value, so an absent name below + // could not be explained by a section that drew nothing. + expect(within(boxOf('industry')).queryByText('Manufacturing'), 'CONTROL: the filled row rendered') + .not.toBeNull(); + const bodyName = emptyIn(boxOf('notes'))!.getAttribute('aria-label'); + cleanup(); + + inLocale('zh', strip); + expect(within(boxOf('Industry')).queryByText('Manufacturing'), 'CONTROL: the filled chip rendered') + .not.toBeNull(); + const stripName = emptyIn(boxOf('Notes'))!.getAttribute('aria-label'); + + expect(bodyName, 'the body grid announces the zh pack value').toBe('无'); + expect(stripName, 'and so does the strip').toBe('无'); + // Stated apart because it is the exact regression the card feared: an + // English name reaching a zh screen-reader session. + expect(bodyName, 'never the English literal').not.toBe('No value'); + expect(stripName, 'never the English literal').not.toBe('No value'); + }); + + it('de — both sites announce `Kein Wert` (a Latin pack, named explicitly)', () => { + inLocale('de', body); + expect(within(boxOf('industry')).queryByText('Manufacturing'), 'CONTROL: the filled row rendered') + .not.toBeNull(); + const bodyName = emptyIn(boxOf('notes'))!.getAttribute('aria-label'); + cleanup(); + + inLocale('de', strip); + expect(within(boxOf('Industry')).queryByText('Manufacturing'), 'CONTROL: the filled chip rendered') + .not.toBeNull(); + const stripName = emptyIn(boxOf('Notes'))!.getAttribute('aria-label'); + + expect(bodyName, 'the body grid announces the de pack value').toBe('Kein Wert'); + expect(stripName, 'and so does the strip').toBe('Kein Wert'); + }); + + it('⭐ THE TWO PATHS AGREE — `EmptyValue`\'s hook and `useDetailTranslation` land on the same bytes', () => { + // `DetailSection` keeps a `title`, and it is resolved by a DIFFERENT hook + // from the one that now resolves the accessible name. If those two ever + // disagreed, one control would announce `无` and hover `No value`. + inLocale('zh', body); + expect(within(boxOf('industry')).queryByText('Manufacturing'), 'CONTROL: the filled row rendered') + .not.toBeNull(); + + const placeholder = emptyIn(boxOf('notes'))!; + expect(placeholder, 'CONTROL: the placeholder is on screen').not.toBeNull(); + expect(placeholder.getAttribute('title'), 'the tooltip is translated too').toBe('无'); + expect( + placeholder.getAttribute('title'), + 'and it is the SAME string the accessible name resolved to', + ).toBe(placeholder.getAttribute('aria-label')); + }); +}); From cfe8bdfc5e24044b6390374e28690b696759abf1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 06:17:43 +0000 Subject: [PATCH 2/2] docs(components): EmptyValue's `pointer-events-none` makes a passed-through `title` a dead attribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the trap the objectui#8506 adoption found, at the shared component rather than only at the one call site that hit it: `title` rides through `...props` and stays in the DOM, so `getAttribute('title')` is green over a tooltip that can never render. Comment only — measured code-identical after comment strip (2335 chars either side, with a control confirming the strip does not hide a real change). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- packages/components/src/custom/empty.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/components/src/custom/empty.tsx b/packages/components/src/custom/empty.tsx index e450238067..f1e2fa89a3 100644 --- a/packages/components/src/custom/empty.tsx +++ b/packages/components/src/custom/empty.tsx @@ -122,6 +122,15 @@ function EmptyContent({ className, ...props }: React.ComponentProps<"div">) { * missing value never looks clickable. This sentence said "en-dash" until * objectui#8506: the word was wrong, never the code — do not "fix" the glyph * to match a stale docblock. + * + * ⚠️ A caller that passes a `title` through `...props` must also pass + * `pointer-events-auto` in `className`. `pointer-events-none` below stops this + * span being a hit target, so a `title` on it never renders a tooltip — the + * hover falls through to whichever ancestor has one. The attribute stays in the + * DOM either way, which is exactly why nothing catches it: a test that reads + * `getAttribute('title')` is green over a tooltip that can never appear + * (objectui#8506, where `DetailSection` kept such a `title` and two landed pins + * navigate by it). */ function EmptyValue({ className,