fix(plugin-detail): DetailSection and HeaderHighlight draw the shared EmptyValue - #8528
Merged
os-justin merged 2 commits intoSep 8, 2026
Merged
Conversation
… EmptyValue Both files spelled their own `<span>—</span>` 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
…hrough `title` a dead attribute
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
os-justin
marked this pull request as ready for review
September 8, 2026 06:20
os-justin
enabled auto-merge
September 8, 2026 06:20
Contributor
❌ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. Which half objected:
📦 Bundle Size Report
Size Limits
|
os-justin
deleted the
claude/issue-8506-detail-placeholder-shared-emptyvalue
branch
September 8, 2026 07:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8506
Placeholder note: this body spells element names in words (a "span element") rather than in angle brackets, because GitHub's body sanitizer eats tag-shaped fragments even inside backticks and fenced blocks.
What this does
packages/plugin-detail/src/DetailSection.tsx:342(the details body grid) andpackages/plugin-detail/src/HeaderHighlight.tsx:254(the ADR-0085 highlights strip) each hand-rolled a span element holding a bare em-dash and resolved their ownaria-labelfromdetail.noValue. Both now render@object-ui/components' sharedEmptyValue, which resolves exactly that key with the same"No value"English fallback through the provider-safeuseObjectTranslation.The card's central premise is falsified — verified here, not inherited
The card was filed as a decision: "adopting the shared
EmptyValuewould trade a translated accessible name for an untranslated one", and asked whether@object-ui/componentshad to learn a caller-supplied translated name first. It does not. Verified onorigin/mainrather than taken on trust:EmptyValuereadst("detail.noValue")and maps a raw-key answer to"No value"itself. Same key, same fallback, different hook.enpacks, becausedetail.noValue'senvalue is byte-identical to the literal it replaces and English is green either way (the objectui#7173 trap, in this very package):zhis无,deisKein Wert. Both sites announce those, and the provider-less path announcesNo value, never a raw key.glyph's default and both call sites' literal are U+2014, measured by codepoint.So nothing needed to land in
@object-ui/componentsfirst, andpremise_still_validis false for the card's stated reason to hesitate.Per-site judgement, decided rather than averaged
Both sites take the shared typography. Each retired its own
text-muted-foreground/60 text-smand now paints the sharedtext-muted-foreground/50. That was not a neutral difference: a type-aware cell renderer already draws the same shared component at/50in the very next row or chip, for a valuehasCellValuewaves through but the renderer cannot draw. An unparseabledatetimeis the reachable case (measured on PR #8503, whose card had attributed that branch toDateCellRendererwhen it isDateTimeCellRenderer's). So one section could show two dashes in two different greys, one row apart.THE AGREEMENTpins that pair in one render.Archaeology, since "deliberate" was worth checking rather than asserting:
7ca7203ec(2026-05-22) movedDetailSectionfrom/50 text-xs italicto/60 text-smand added both thearia-labeland thetitle, three weeks afterEmptyValueshipped at/50(4f33c60d9, 2026-05-01);471c5d38d(2026-07-16) copied that spelling intoHeaderHighlight. Nothing in either commit shows awareness of the shared component, so the/60reads as continued hand-rolling rather than as a decision against/50.DetailSectionkeeps itstitle, and keeps it ALIVE — this is the one carve-out. Thetitlearrived in the same commit as thearia-label: it is the sighted-mouse counterpart of the accessible name. It also happens to be the only instrument two landed pins use to tell this placeholder apart from a cell renderer's (DetailSection.emptinessAuthority-8376,record-details.emptySectionDefault, both byqueryAllByTitle('No value')). It rides throughEmptyValue's...propsuntouched — but the shared component setspointer-events-none, and atitleon an element that is not a hit target never renders a tooltip: the hover falls through to the row's owntitle={t('detail.editInlineHint')}. Keeping the attribute without keeping the pointer events would leave a dead attribute that two landed pins go on reading as if it were alive. So that one site mergespointer-events-auto, whichcncollapses the shared default away for.HeaderHighlighttakes the shared non-interactive default unchanged. It carries notitleand never did, so there is nothing to keep hoverable. It keeps onlyblock, which is layout rather than typography: the filled branch beside it is a block span in the samemin-w-0 flex-1box, and an inline dash sits on a different baseline.The landed pin whose instrument had to be re-derived
detailPage.emptinessAuthority-8394.test.tsxread the strip's affordance as[aria-label="No value"]:not([data-slot="empty-value"])— that is, it navigated by exactly the thing this change moves. Once the strip adopted the shared component the selector matched nothing and the case failed over a perfectly correct surface. It now anchors on the field label, which no placeholder change can move, and asserts which chip draws the affordance rather than how many. Its docblock records why, so the next reader does not re-introduce the coupling.Ablations — run, not predicted
Three legs. Each mutation was proved on disk in both directions (injected marker present, removed text absent) before the instrument ran, each script carried
trap ... EXIT INT TERMwith absolute paths, and each restore was verified by state (git diff HEADempty andgit hash-objectequal togit rev-parse HEAD:PATHfor both files), never by exit code.EmptyValuereturned unconditionally from both sites, filled values includedclassNamedroppedRead the modes, not the count. The first caricature run reddened 7 of 8 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 drawing nothing but placeholders satisfies "the empty row has an accessible name", "the title survives" and "the two branches agree" perfectly well. The eighth, the provider-less fallback case, survived outright. That reading changed the file rather than the write-up: the two NON-REGRESSION cases now state
drawingAffordance([...])first, ahead of their own controls, and the provider-less case gained a value control plus a label saying what it does not decide. Re-measured after that change: 8 of 8, with the twodrawingAffordancecases failing on their headline and naming the offenders —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 labelled scope declarations on that axis, as are all three cases in the i18n file (theiraria-label === '无'headline is true of a page that has given up on values).The wrong-fix leg is the one that earns the per-site argument above: it is what a mechanical swap actually produces, it is invisible to every assertion that only reads the accessible name, and exactly the three cases that encode the two decisions refuse it —
the site restores pointer events for its title,the ONLY class the two branches disagree on is the non-visual pointer-events pair,the chip keeps its block display.The harness-kill leg exists because a harness that navigates by what the mutation erases dies first and then measures nothing while reading as a strong refusal. Every lookup here is anchored on the field label; removing it reddens all 8 with
TestingLibraryElementError: Unable to find an element with the text: industry, i.e. loudly and unmistakably, never as a quiet pass.Testing
pnpm exec vitest run packages/plugin-detail/— 143 files, 1281 tests, all passing (full package, at the source state of this PR).detailPage.emptinessAuthority-8394,DetailSection.emptinessAuthority-8376,record-details.emptySectionDefault,record-details.hideEmptyRetired-7129,RelatedList.emptyPlaceholderAffordance-8475.pnpm --filter @object-ui/plugin-detail --filter @object-ui/components run type-check— bothDone; each package's script istsc --noEmit && tsc -p tsconfig.test.json, so the new test files are inside the measurement rather than excluded from it.no-explicit-anywarnings). Plus an explicit irregular-whitespace sweep with a planted-ZWSP control, sincecheck:control-bytespassing does not cover that class.check:control-bytes,changeset:check,check:i18n-keys,check-changeset-presence.mjs— all green.node scripts/check-governed-queue-guard.mjs --testover the seven paths: NOT GOVERNED.Declared narrowing.
turbo ls --affectednames 31 packages, because the diff touches@object-ui/components. That package's change is a docblock only — proved mechanically, not asserted: stripping comments from both theHEADblob and the working file leaves byte-identical text (2335 chars each), with a control confirming the strip does not hide a real code change.EmptyValue's exported signature, classes and behaviour are untouched, and no test outsideplugin-detailreads either changed surface (grepped with lit controls). So its full suite and the other 29 packages were left to CI; a sibling agent shares this container and the shared verify lock was held for roughly six minutes in total.Changeset
.changeset/8506-detail-placeholders-shared-empty-value.md,patchfor both packages.skip-changesetis a phantom label in this repository, so a real changeset was written;majoris forbidden here and is not used.Session for this implementation, in prose so it survives a body edit:
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code