Repository navigation
fix(app-shell): draw the shared EmptyDescription for the previews' empty-collection sentences - #8569
Merged
os-justin merged 2 commits intoSep 8, 2026
Conversation
…pty-collection sentences Sixteen sites across twelve files under `metadata-admin/previews/` each hand-rolled the same empty-collection state — a muted italic sentence in a plain `div` — and two of them (`AgentPreview`, `ToolPreview`) declared a file-local component literally named `Empty` that shadowed the shared family in the one place a maintainer would reach for it. They now render `EmptyDescription` from `@object-ui/components`, used alone outside any `Empty` container, the shape PR #8527 measured at 16px / 12px / start-aligned / italic against the row it replaced. Every site keeps its measured size class, so the per-site layout delta is zero: `text-xs` where the row sat in (or inherited from) a text-xs rail, `text-[10px]` for the run row's step log, `text-[11px]` for the translation category card, and `text-sm` for the screen body's sentence, which stands in for a description that renders at text-sm. Bare `italic` would have kept the family's `text-sm/relaxed`, so each size is spelled out. Not migrated, each for its own reason: the inline `span` carriers ("none", "not configured"), the amber warning-toned sentences, the dashed drop-zone placeholders, the palette search empties, the `PreviewEmptyState` / `PreviewMessage` full-canvas states, and the unknown-rule-type fallback whose chrome is the CEL block's own box. The pin navigates every site by a title its container owns, never by the empty state, so a caricature that renders the description unconditionally fails the populated assertions instead of killing the harness. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
…y a child position The caricature (EmptyDescription rendered unconditionally, populated collections included) was run against the first version of this pin: eight of the seventeen populated cases died on their marker lookup — "Unable to find an element" — before the refusing `toBeNull` assertion ever ran, because the harness took `children[1]` as the body slot and the caricature's inserted sibling shifted the rows one slot over. Every navigator now returns the container that owns the title, and both assertions are descendant queries on it; the empty state can never be that container, because the container is what holds the title. 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 12:20
os-justin
enabled auto-merge
September 8, 2026 12:20
This was referenced Sep 8, 2026
os-justin
deleted the
claude/issue-8526-previews-shared-empty-collection
branch
September 8, 2026 12:48
os-sales
added a commit
that referenced
this pull request
Sep 8, 2026
Brings the branch up to date after #8569, #8574 and the rest landed. No conflict: nothing on main touched packages/types/src/__tests__/zod-mirror-parity.test.ts since this branch's base f5cfbbd, so the objectui#8458 pin and the objectui#8248 amendment compose with main rather than contending with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZY49skxUBYyJcdnTcYPrE
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 #8526
Sixteen empty-collection sentences across twelve files under
packages/app-shell/src/views/metadata-admin/previews/each hand-rolled the same state — a muted italic sentence in a plaindiv— and two of those files (AgentPreview,ToolPreview) declared a file-local component literally namedEmpty, which shadowed the shared family in the one place a maintainer would reach for it. All sixteen now renderEmptyDescriptionfrom@object-ui/components, used alone, outside anyEmptycontainer — the shape PR #8527 (objectui#8520) landed and measured at 16px / 12px / start-aligned / italic against the row it replaced. The two localEmptycomponents are deleted. Nothing lands in@object-ui/components;@object-ui/componentsis alreadyworkspace:*in app-shell, so no manifest edge was added.Per the ruling on the card: the
Emptycontainer is not used anywhere (118.8px at the rail's 215px width), andEmptyValueis not used anywhere (itsaria-labelsays "No value", which is false about a collection).Census, re-derived
Pathspec
'packages/app-shell/src/views/metadata-admin/previews/*', run from the repo root. Lit control on the same pathspec:git grep -c PreviewShellfires on 24 files, so a zero from any probe below is an absence rather than a dead glob. Probes: (A)\bitalic\b, (B)border-dashed, (C)text-amber, (D)PreviewEmptyState, (E)function Empty\b, (F) theEmpty*family names, (G) length-conditional rendering (.length === 0,!x.length,Object.keys(x).length), (H) JSX text opening with No / none / empty, (I)tr('engine.*.(no…|empty)')keys. Every probe's matched lines were read in context, not counted.Reconciled by their differences, not their union: probe G (length shape) returned a false zero on
TranslationPreview:152, whose condition iscat.count === 0— probes A and H found it. Probe A (italic) returned a false zero on the two sites that went through the localEmpty(AgentPreview:136,ToolPreview:185) — probes E and H found them. Probe G is the one that surfaces the families probe A cannot see at all: the dashed drop zones, the amber warnings, the palette search empties.Effective type scale, read from each site's ancestors rather than from its own class list: text-xs at 13 of the 16 migrated sites (explicit at six; inherited from a
text-xscontainer at seven —ActionPreview:578,EmailTemplatePreview:93,FlowPreview:341,FlowRunsPanel:388,FlowSimulatorPanel:228,JobPreview:330,AgentPreview:161), 10px at one, 11px at one, text-sm at one. The card's "six different type scales" does not reproduce; the ruling's "uniform with a few genuine outliers" does. Instrument for the class merge:tailwind-merge@3.6.0(the versioncn()resolves), which evicts the family'stext-sm/relaxedfor any explicit size class — and keeps it for bareitalic. So every site spells its size out; bareitalicwould have been a 14px / 22.75px delta at the seven inheriting sites.Per-site delta
main)classNamenowAgentPreview:136Empty→div.text-xs.text-muted-foreground.italictext-xs italicAgentPreview:192"Defaults in use."div.text-muted-foreground.italic, inherits xstext-xs italicAgentPreview:256chip-listemptyHintdiv.text-xs.text-muted-foreground.italictext-xs italicAgentPreview:221localEmptyToolPreview:185Emptytext-xs italicToolPreview:296localEmptyDatasourcePreview:159div.text-xs.text-muted-foreground.italictext-xs italicSkillPreview:107text-xs italicActionPreview:581div.text-muted-foreground.italic, inherits xstext-xs italicEmailTemplatePreview:98codechild (the family's child rules targetaonly)text-xs italicFlowPreview:346text-xs italicFlowRunsPanel:349step logdiv.text-[10px].italic.text-muted-foregroundtext-[10px] italicFlowRunsPanel:406runs listdiv.italic.text-muted-foreground, inherits xstext-xs italicFlowSimulatorPanel:324scratch rowstext-xs italicFlowSimulatorPanel:383variable watchtext-xs italicJobPreview:348text-xs italicTranslationPreview:153div.text-[11px].text-muted-foreground.italictext-[11px] italicScreenPreview:77p.text-sm.italic.text-muted-foregroundtext-sm italicpmargins); elementp→divThe outliers, one sentence each
text-sm(ScreenPreview:77) — kept at text-sm, deliberately: this sentence stands in for the screen body, whose description renders at text-sm one line below (line 84), not for a text-xs rail row; averaging it down to xs would have been a visible delta with no sibling to justify it. The pin asserts the class stays.span"none" (ValidationPreview:353) — left as is; the ruling does not cover inline markers. A second inline carrier the ruling did not enumerate turned up:DatasourcePreview:225span.text-muted-foreground.italic"not configured" (a rail card whose object has no keys). Left for the same reason and reported.ValidationPreview:387,rounded border bg-background p-2.5) — establishing what the chrome does: it is theCelBlockbox (ValidationPreview:439,rounded border bg-background p-2.5 text-xs font-mono), so the sentence occupies the slot the rule body would have. It is also not an empty-collection state: it is thedefault:arm for an unknown ruletype("Unknown rule type … Showing common fields only."). Left untouched, chrome and all.Not migrated, by class
JobPreview:357,JobPreview:387,SkillPreview:100,ValidationPreview:282/335/378/408/436,BookPreview:254. Two of these are in the card's fifteen. They deliberately are not muted; migrating them means either dropping the amber (a visible delta the ruling did not authorize) or overriding the family's colour so the membership is nominal. A design call above this card.AppNavCanvas:330,FieldsListEditor:170,PageBlockCanvas:554/828,ObjectFormCanvas:516; bordered, centred editor targets, a different visual class (and two dashed conventions:border border-dashedvsborder-2 border-dashedinFieldStub).PageBlockCanvas:915,ObjectFormCanvas:1323,AddWidgetPicker:86,ViewColumnPanes:320(via its ownHint); centred, padded, not italic.PreviewEmptyState(BookPreview:191,DatasetPreview,ReportPreview) andPreviewMessagesites;ProblemsPanel:53(a success state with an icon).DashboardPreview:266"untitled",ValidationPreview:167"no message set",FieldStub:205(a stub input's placeholder; in the card's fifteen).EmptyValue's question, not this card's.FlowRunsPanel:402(serviceunavailable; its sibling branch at 406 is migrated),FlowSimulatorPanel:442(idle hint),BookPreview:250(explains that members are derived; not an absence).Tests
packages/app-shell/src/views/metadata-admin/previews/emptyCollection-8526.test.tsx— 33 cases: for each of the sixteen sites an empty leg (the family'sdata-slot="empty-description"is present under the titled container, carries the site's sentence, and noempty-valueis present) and a populated leg (the rows render and noempty-descriptionexists under that container — the caricature refuser), plus one whole-surface case on a fully populated agent. Every navigator reaches the container that owns a title (Sectionheading, rail label, panel header, category-card label), never the empty state and never a child position.Measured, not predicted — three legs, each restored by state (
git diff HEADempty over the 12 paths and all 12 blob hashes equal toHEAD:path), mutation proven on disk in both directions, trap onEXIT INT TERM:EmptyDescriptiontag left — the objectui#8520 precedent — and 2 localEmptydeclarations back): 16 failed / 17 passed; all 16 on "the shared EmptyDescription slot" expected truthy; 0 harness deaths.EmptyDescriptioninserted before every migrated conditional; on disk: 16 markers, 0 after restore): 33 failed / 0 passed, 0 harness deaths — 16 populated cases ontoBeNull, the whole-surface case ontoHaveLength(0), 16 empty cases on the sentence mismatch. The first run of this leg against the first version of the pin was the instrument that mattered: 8 of the 17 populated cases died on their marker lookup ("Unable to find an element") before the refusing assertion ran, because the harness tookchildren[1]as the body and the inserted sibling shifted the rows one slot over. The pin was rewritten to container-level navigation with descendant queries (second commit) and the leg re-run; the numbers above are from the re-run.ToolPreview's section title renamed): 2 failed / 31 passed, bothUnable to find an element with the text: Input Parameters— textually distinct from both modes above.Verification
turbo run build --filter='@object-ui/app-shell^...'— 28/28 tasks.pnpm --filter @object-ui/app-shell type-check(source leg and thetsconfig.test.jsonleg): exit 0 before and after.FlowNodeInspectortests (the one consumer ofScreenPreviewoutside the directory) and the two console tests naming the previews — 9 files / 297 tests.turbo ls --affectednames app-shell, console and two example packages; the two examples carry no test that reaches these modules.pnpm lintisturbo run lint, oneeslint .per package, and only app-shell changed. The CI-exact invocation for app-shell (eslint .inside the package,--format json) at61b113204: 1110 files, 0 errors, 2925 warnings (the known-debt stream; 24 of them in the twelve touched files, all pre-existing, none on a changed line; 0 in the pin). Invariance:eslint --print-configon a preview file shows noparserOptions.project/projectService, so this diff cannot move any untouched package's verdict.check:control-bytes,check-changeset-presence(12 source files of 1 released package, 1 changeset),check-changeset-no-major,check:vi-mock-specifiers,check:i18n-keys,check:phantom-deps.check-governed-queue-guard --teston the diff paths: NOT GOVERNED..changeset/previews-shared-empty-description-8526.md,@object-ui/app-shell: patch(skip-changesetis a phantom label in this repo and is not applied).Where the card and the ruling read differently from
mainEmptyDescription-alone precedent atAgentPreview:315landed in PR fix(app-shell): AgentPreview's empty rail collection announces itself in words #8527 (commit5eff8b161, objectui#8520). PR fix(components): make Empty's base classes override-friendly and drop its inert border-dashed #8552 (1570eac37, objectui#8525) is theEmptycontainer's override change. The ruling attributes the precedent to PR fix(components): make Empty's base classes override-friendly and drop its inert border-dashed #8552; git says otherwise. The target is the same either way.ValidationPreview:387) is not an empty-collection state — see above.PreviewEmptyState, one stub-input placeholder); nine of its fifteen are in the migrated set, and seven migrated sites were not in the card's list at all (AgentPreview:136,AgentPreview:192,FlowRunsPanel:406,FlowSimulatorPanel:383,JobPreview:348,TranslationPreview:153,ScreenPreview:77,ToolPreview:185via the localEmpty).🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code