Repository navigation
fix(app-shell): AgentPreview's empty rail collection announces itself in words - #8527
Merged
os-justin merged 1 commit intoSep 8, 2026
Merged
Conversation
… in words `KeyVals`'s zero-rows branch rendered a bare em dash in a plain `<div>` with no role, no label and no text alternative, so a screen-reader user reaching a rail block whose keys all resolved to `undefined` heard a naked punctuation mark. It now renders `EmptyDescription` from `@object-ui/components` with the text "No values set.". This is an empty COLLECTION, not an empty field value, so `EmptyValue` is the wrong member of that family: its docblock scopes it to a missing cell/field value and its `aria-label` resolves `detail.noValue` — "No value" — a false statement about a collection that simply has no members. The container `Empty` is not used either: measured in the browser at this rail's real 215px content width it is 118.8px tall against the 16px row it replaces, and its centred panel is built for a full canvas. `EmptyDescription` at `text-xs italic` is metrically identical to the row it replaces (16px, 12px, start-aligned, italic), so the layout delta is zero and only the announcement changes. The pin navigates by the rail block's own title, never by anything the empty state renders, so a caricature that erases the description fails an assertion instead of killing the harness. 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. Which half objected:
📦 Bundle Size Report
Size Limits
|
os-justin
marked this pull request as ready for review
September 8, 2026 06:18
os-justin
enabled auto-merge
September 8, 2026 06:18
os-justin
deleted the
claude/issue-8520-agentpreview-empty-collection-state
branch
September 8, 2026 07:03
os-justin
pushed a commit
that referenced
this pull request
Sep 8, 2026
…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
This was referenced Sep 8, 2026
Merged
akarma-synetal
pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Sep 9, 2026
…pty-collection sentences (objectstack-ai#8569) * fix(app-shell): draw the shared EmptyDescription for the previews' empty-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 objectstack-ai#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 * test(app-shell): navigate the 8526 pin by the titled container, not by 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
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 #8520
KeyVals's zero-rows branch inAgentPreview.tsxrendered a bare em dash in a plaindiv— no role, no label, no text alternative. It now rendersEmptyDescriptionfrom@object-ui/componentswith the text "No values set.".Which members, and why
Used:
EmptyDescription, alone, withclassName="text-xs italic".Not used:
EmptyValue(ruled out by the card). Also not used: theEmptycontainer.Dropping the container is the one judgement objectui#8520 left open, and it is a departure from the ruling's stated guess, so here is the measurement it was asked to rest on rather than an assertion.
The measurement
Chromium at
/opt/pw-browsers/chromium, the dev preview gallery (apps/consoleat?only=agent), viewport 1440x900. Every candidate rendered in the same page load, inside the realAgentPreviewside rail — rail box 240px, content column 215px — readinggetBoundingClientRectandgetComputedStyleoff each:div.text-muted-foreground.italicholding the em dashEmptyDescription+text-xs italicEmptyDescription, family defaultsEmptywrappingEmptyDescription— the ruling's predictiondashedat 0px widthclassName="px-3 py-8"— this repo's own sidebar precedentdashedat 0pxEmpty(see below)D is 7.4x the row it replaces, centred, inside a 215px rail whose other blocks are 16px-per-row definition lists. It is a full-canvas panel and the rail is not a canvas. E shows the obvious escape hatch does not work: an unprefixed
px-3 py-8replaces the basep-6but never touchesmd:p-12, so above 768px the padding comes straight back — filed as objectui#8525 along with the dashed border that renders at 0px width at every call site in the repo.B is metrically identical to A on all four axes. The layout delta of this PR is zero; only the announcement changes. That is worth stating plainly because the ruling predicted the opposite ("the visual delta here is deliberate and larger than the six sites in objectui#8504") — measured, there is no visual delta at all, which is a better outcome than the one that was being braced for.
Why not the container at a smaller size
Emptycan be beaten into 16px withgap-0 p-0 md:p-0 items-start text-left— measured, it works. It also neutralises six of the container's ten utilities and leavesrounded-lg border-dashed text-balancedoing nothing. Fighting a component that hard is the component telling you it is the wrong member.Why
EmptyDescriptionstanding aloneAll 28 existing
EmptyDescriptioncall sites in this repo nest it insideEmpty, so this is a new composition and reviewers should look at it. The case for it: it is a block-level text slot (data-slot="empty-description"), it carries the design-system hook so a future restyle reaches this site, and it states the condition in words — which is the whole of the defect. The case against is precedent, and it loses to a 7.4x height difference.What the sibling census found
The ruling asked for it before choosing. Probing
packages/app-shell/src/views/metadata-admin/previews/*with an element-agnostic grep for the family: zero of the 15 empty-collection states in that directory use it. All 15 hand-roll a muted sentence, across six type scales. Two of them —AgentPreview.tsx:221andToolPreview.tsx:296— declare a local component namedEmpty, which is why this PR importsEmptyDescriptionand notEmpty: the names collide in this file, with no compiler error waiting for whoever tries. Filed as objectui#8526.The local
Emptysits 166 lines above the defect and is used at line 136 for the same kind of state. The zero-rows branch was the file's own outlier, not a site that had no block-level empty state available.Test
AgentPreview.emptyCollection-8520.test.tsx, six cases. It navigates by the rail block's title ("Planning", owned byRailBlock) and never by anything the empty state renders — so a caricature that erases the description fails an assertion instead of killing the harness.Four ablation legs, each mutated on disk, proven by hash in both directions, restored by state:
EmptyValuewithclassName="italic"— the plausible wrong fixRailBlock— harness-killEmptywrappingEmptyDescription— the ruling's guessThe labelled discriminator was inert on first run and had to be repaired. The assertion written specifically to catch
EmptyValue— "is not labelled No value" — sat green through a measuredEmptyValuecaricature, becauseKeyValsreturns the empty state as the rail block's body element andElement.querySelectorAllnever matches the element it is called on. The axis that actually caught it was the generic one: the state announces letters, not punctuation. Repaired with a self-inclusive selector, re-run, and it now fires; the miss is recorded in the test's own comments so it does not get re-introduced.Declared scope: the surviving leg is not a bug in the pin. This test measures the accessibility contract — a readable, correctly-scoped, block-level announcement — and not visual weight. The container variant is a legitimate fix by that contract and is rejected on the browser measurement above, which is argument, not assertion. A future change that swaps in the container will pass this test; that is intended.
Checks run locally
pnpm exec vitest run packages/app-shell/src/views/metadata-admin/— 239 files, 2487 passed, 1 skippedpnpm --filter @object-ui/app-shell type-check— green, afterpnpm --filter '@object-ui/app-shell^...' build. That project chainstsc -p tsconfig.test.json, whoseincludeissrc/**/*.test.tsx, so the new pin is compiled and not merely run.pnpm exec eslint .— the full repo population, 4494 files, 0 errors. No narrowing, and no--no-inline-config(an objectstack convention that manufactures errors CI does not have here).no-irregular-whitespaceis on at error level via the preset; note itsskipStrings: true, so a separate scan of the diff for control bytes, zero-width and irregular whitespace was run and is clean.pnpm check:control-bytes— green.node scripts/check-changeset-presence.mjs— green, 1 changeset for 1 changed published source file.Reachability
No manifest edge.
@object-ui/componentsis already"workspace:*"underdependenciesinpackages/app-shell/package.json, and this file already importedcnfrom it.i18n
The string is a literal, matching this file —
AgentPreview.tsxhas not()call anywhere and states all of its copy in English ("No system prompt set yet.", "Defaults in use."). Introducing one key here would be the inconsistency, not the fix.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code