fix(app-shell,plugin-dashboard,plugin-grid,plugin-chatbot,console): draw the shared EmptyValue for the six remaining hand-rolled placeholders - #8518
Merged
Conversation
…raw the shared EmptyValue for six hand-rolled placeholders Six sites built their own `<span>` holding a bare em dash, so an empty cell had no accessible name at all: no `data-slot`, no `aria-label`, and none of the shared component's `select-none` / `no-underline` / `pointer-events-none`. `EmptyValue` from `@object-ui/components` answers exactly this, and every one of the five packages already depends on that package. Two adjacent lines in the AI-approvals drawer (`proposed_by` / `decided_by`, bare `'—'` text nodes) are converted with them — a different source spelling of the same rendered defect, verified individually rather than swept up. `formatRelative`'s `: string` fallback is deliberately left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
…column 0 Column 0 is rendered through `defaultCell` itself, so the caricature ablation (EmptyValue everywhere) erased the very name the harness looked the row up by: all three cases went red on "the row for beta rendered" — the harness dying, not an assertion refusing the caricature. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
…were missing The caricature (EmptyValue drawn over every cell) was run against all six pins. Measured: every THE DEFECT case failed only on its CONTROL — its headline claim is equally true of a surface that has stopped rendering values — and each NON-REGRESSION case failed one assertion earlier, on value presence, so its own 'no placeholder' half never ran. Three files already had a case whose assertion fails BECAUSE a filled cell gained a placeholder. Two did not; they do now. Two dereferences that had been failing with a bare TypeError are guarded so their messages reach the summary, and the one case the caricature survives is relabelled a scope declaration rather than quoted as proof. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
…aricature directly Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
…cblock They were inserted to keep a glob from closing the block comment, and `no-irregular-whitespace` is an ERROR in apps/console — the only three lint errors on the branch. `check:control-bytes` is green either way: U+200B is not a control byte. The sentence names the pathspec in prose instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
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. 📦 Bundle Size Report
Size Limits
|
os-justin
marked this pull request as ready for review
September 8, 2026 05:12
os-justin
enabled auto-merge
September 8, 2026 05:12
os-justin
deleted the
claude/issue-8504-emptyvalue-remaining-carriers
branch
September 8, 2026 05:42
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 #8504
Six hand-rolled
<span>—</span>placeholders become the sharedEmptyValuefrom@object-ui/components, the way PR #8503 did for the five sites in objectui#8491. This closes the no accessible name half of the class; theDetailSection/HeaderHighlighthalf (which already carriesaria-label={t('detail.noValue', …)}) is a separate card and is untouched here.The six carriers
origin/mainpackages/app-shell/src/views/metadata-admin/ResourceListPage.tsx:636defaultCell→<EmptyValue />packages/app-shell/src/views/metadata-admin/AuditPanel.tsx:252<EmptyValue />packages/plugin-dashboard/src/RecordDetailDrawer.tsx:127<dd>→<EmptyValue />packages/plugin-grid/src/ImportWizard.tsx:1034<EmptyValue />packages/plugin-chatbot/src/AiPendingActionsInbox.tsx:193JsonBlock→<EmptyValue className="text-xs" />apps/console/src/pages/developer/PublicFormsPage.tsx:385<EmptyValue />Each was a plain span with no
data-slot, noaria-labeland none of the shared component'sselect-none/no-underline/pointer-events-none. Thepointer-events-nonehalf is not cosmetic in the metadata list:defaultCell's output for column 0 is rendered inside the row's<Link>, so the hand-rolled span inherited the link colour and stayed selectable — a missing value that looked clickable, the same affordance bug #8503 named in the grid's link cells.The adjacent pair, decided deliberately
AiPendingActionsInbox.tsx:507/:511({selected.proposed_by ?? '—'},{selected.decided_by ?? '—'}) are in. They are a different source spelling — a bare text node in a plain<div>, not a styled span — but the rendered defect is identical, they sit in the same drawer as:193, and the card verified them individually rather than leaving them in the un-triaged population. Leaving them would ship an unannounced em dash out of a PR whose purpose is to retire them from that file, and force a second PR onto the same four lines.Deliberately NOT swept in:
formatRelative(AiPendingActionsInbox.tsx:172) andformatImportJobTime(ImportWizard.tsx:1433), bothif (!x) return '—'inside a: string-declared helper. Converting those changes a return type and every call site — they belong to the fenced 41-member population under separate triage. Likewise untouched:ImportWizard's'— None —'/'— Map columns manually —'/'— Skip —'i18n labels andPublicFormsPage's<option>— Select a FormView —</option>, which are em dashes as label decoration, not placeholders.Reachability — checked, not assumed
All five packages already declare
@object-ui/componentsatworkspace:*. No manifest edge was added, and none was needed.apps/console(the one carrier inapps/, notpackages/) has it ondevDependencies, and 29 files underapps/console/srcalready import from it — this page among them, soEmptyValuejoined an import list that was already there.check:phantom-depsandcheck:unused-depsare both green on the result.Visual delta, per site — honestly
text-muted-foreground(full opacity) → sharedtext-muted-foreground/50. One deliberate step more muted, plus the three affordances and the accessible name.text-muted-foreground/60→/50. Same, one step smaller.JsonBlock(:193)text-xsis kept throughclassName— it stands where atext-xs <pre>would, with no neighbouring shared placeholder to match — so only the accessible name, the three affordances and the opacity change.:507/:511text-xs font-monofrom the parent<div>, which they still inherit.The glyph is an unchanged em dash at every site:
AuditPanel.lockState.test.tsx's pre-existingexpect(screen.getByText('—'))still passes untouched.Pins — 22 cases across 6 files, and what each one MEASURED
⭐ The caricature was run, not reasoned about:
EmptyValuedrawn unconditionally at all eight sites, filled values included. 21 of 22 red, 1 green.and the filled row does NOT,CONTROL: the locked sibling is still not a placeholder, …). Two files had none until this measurement; the cases were added for exactly that reason.NON-REGRESSIONcases fail one assertion earlier, on value presence (the value reaches the cell): the caricature also stops these surfaces printing values, so their ownno placeholderhalf is never reached. They do refuse it — just not through the half that names the defect.THE DEFECTcases fail ONLY on their CONTROL. Their headline claims — "the empty cell has an accessible name" — are equally true of a surface that has stopped rendering values, exactly as fix(plugin-grid,plugin-detail): draw the shared EmptyValue for missing cell values #8503 measured. The controls are not optional decoration.SCOPE DECLARATION — the placeholder is inert inside the row link. It readspointer-events-none/no-underline/select-noneoff the placeholder, which is true of an implementation that draws one over every cell. It is labelled a scope declaration in its own name and docblock, not quoted as proof. It goes red on the revert leg, so it does hold what the hand-rolled span lacked.Revert leg (hand-rolled spans restored on the committed implementation): 16 red / 6 green, every failure on a labelled headline assertion (
the empty cell draws the shared placeholder), everyNON-REGRESSION-shaped case green — the correct shape.A first caricature run failed the metadata-list pin's three cases on the harness (
the row for beta rendered) rather than on any assertion: the row lookup read column 0, whichdefaultCellalso renders. It now reads the row link'shref, and two dereferences that had been failing with a bareTypeError— so their messages never reached the summary — are guarded.Every assertion is scoped to one cell / one row / one labelled field, never the container (objectui#8495).
Both ablation legs restore by state:
git checkout HEAD -- <absolute path>(never the bare form, which reads the index), proven bygit hash-objectagainstgit rev-parse HEAD:<path>for all six files plus an emptygit diff HEAD, with atrap … EXIT INT TERM. Mutations are proven to have landed on disk by before/after occurrence counts, and an anchor miss aborts the leg rather than silently measuring nothing.Verification
pnpm exec vitest run <the six pins>→ 6 files, 22 tests, green.packages/plugin-dashboard/,packages/plugin-chatbot/andapps/console/src/pages/developer/, plusAuditPanel.lockState,AuditPanel.loadFailure,ImportWizard.i18n-parity,ImportWizard.interpolationandsavedMapping→ 155 files, 1457 tests, green.turbo run type-checkover all 7 affected packages → 42/42 tasks green.turbo run lintover the 5 edited packages → 0 errors (warnings are the pre-existing baseline).--no-inline-configdeliberately not used — that is an objectstack convention and manufactures errors CI does not have here.check:vi-mock-inherit,check:vi-mock-specifiers,check:control-bytes,check:phantom-deps,check:unused-deps,check:unreferenced-sources,check-changeset-presence→ all green.check-governed-queue-guard --testreports NOT GOVERNED.packages/app-shell/alone is 281 files) was started and abandoned after 31 minutes under container contention from a sibling agent. CI runs the whole farm. Only this session's own recorded PIDs were killed — never by process name.Note for review
ImportWizard.tsxgains one entry on its existing__testablesseam (SavedMappingSummary, the 22nd, same@internalcontract as the 21 before it). Reaching that component through the wizard means driving a spreadsheet parse and a RadixSelect, neither of which the component owns. What the seam does not cover: thatStepMappingstill routes to it — that line is untouched here.A real changeset is included (
minoron all five packages, matching #8503).skip-changesetis a phantom label in this repo and is deliberately not applied.Left in draft with no auto-merge, per dispatch.
🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code