Skip to content

refactor(studio): audit native labels, disclosures and ad-hoc alerts against the primitives - #2352

Merged
peppescg merged 6 commits into
mainfrom
refactor/studio-primitives-audit-2340
Oct 15, 2026
Merged

peppescg merged 6 commits into
mainfrom
refactor/studio-primitives-audit-2340

Conversation

@peppescg

Copy link
Copy Markdown
Contributor

Summary

Audits every native <label>, aria-expanded disclosure and ad-hoc role="alert" under apps/web/src/features/ against the primitives. Three disclosures move to a newly ported Collapsible primitive, the worktree picker's option cards move to Label, and three banners move to Alert. Names, roles, keyboard behaviour and pixels are unchanged in every captured state. The table under What changed gives the decision for each of the 32 call sites; 9 are in files that other open PRs change, so they are left for later.

Part of #2340
Part of #2201
Part of #2210

What changed

  • components/ui/collapsible.tsx (new): the stock shadcn Collapsible over @radix-ui/react-collapsible. The package was already in the lockfile through the Accordion; apps/web/package.json now names it directly, exact-pinned. collapsible.test.tsx covers a trigger showing and hiding the content, and a controlled open state.

  • Disclosures → Collapsible. The failed-turn card's Show details (chat/failed-turn-card.tsx), the reasoning summary (chat/reasoning-disclosure.tsx), and a learning suggestion's Details (learning/proposal-row.tsx) now use Collapsible, CollapsibleTrigger asChild and CollapsibleContent, instead of a button with hand-set aria-expanded and aria-controls over conditionally rendered content. They stay controlled, so the expand-details preference and the first-open detail read work as before. The suggestion card spaces its children with flex gap instead of space-y, so the closed content adds no margin.

  • Label. Each eligible worktree's card in chat/worktree-picker-dialog.tsx is a Label for its radio. font-normal and leading-[inherit] keep the card's weight and line height against Label's font-medium leading-none.

  • Banners → Alert (Alert with AlertDescription):

    • the runtime status line's failure banner (settings/settings-card.tsx). w-auto keeps it content-sized beside the Refresh button.
    • the learned-skills publication notice (knowledge/learned-skills.tsx). It keeps its warning tint and Dismiss button.
    • the learned-skill dialog's action error (same file). border-0 keeps it unframed, as before.

    The destructive banner uses the default variant with Studio's destructive tint: the Alert's destructive variant fades the description to 90%, which drops below 4.5:1 on white.

Every call site

<label>, aria-expanded= and role="alert" in non-test files under apps/web/src/features/ on main (1210721be): 32 sites.

Call site (apps/web/src/features/, line on main) Pattern Decision
chat/failed-turn-card.tsx:58 aria-expanded Changed → Collapsible. Show details / Hide details over the raw payload.
chat/reasoning-disclosure.tsx:65 aria-expanded Changed → Collapsible. Reasoning summary.
learning/proposal-row.tsx:274 aria-expanded Changed → Collapsible. A suggestion's Details; the first open still reads the detail once.
chat/worktree-picker-dialog.tsx:98 <label> Changed → Label, for each worktree's option card.
settings/settings-card.tsx:111 role="alert" Changed → Alert. The runtime status line's failure banner.
knowledge/learned-skills.tsx:161 role="alert" Changed → Alert. The publication notice with Dismiss.
knowledge/learned-skills.tsx:336 role="alert" Changed → Alert. The learned-skill dialog's action error.
chat/ask-args-view.tsx:57 aria-expanded Kept: a show-more toggle on a command that is always rendered. It changes how many lines show, not whether a region exists, so it isn't a disclosure. Already a Button primitive.
chat/delivery-note-card.tsx:75 aria-expanded Kept: the same show-more pattern, toggling line-clamp-8 on text that is always rendered. Already a Button.
chat/edit-diff.tsx:271 aria-expanded Kept: the same show-more pattern ("Show all N lines" / "Show first N lines") on a diff that is always rendered. Already a Button.
chat/authorization-review.tsx:233 role="alert" Kept: a one-sentence warning inside the authorization card, beside its Refresh button. Not a banner.
chat/session-details-dialog.tsx:201 role="alert" Kept: an inline error line in the flow (text plus, at most, a Retry button), not a banner. Alert would add a framed box where there was none.
chat/session-inspection.tsx:330 role="alert" Kept: an inline error line in the flow (text plus, at most, a Retry button), not a banner. Alert would add a framed box where there was none.
chat/soul-dialog.tsx:45 role="alert" Kept: an inline error line in the flow (text plus, at most, a Retry button), not a banner. Alert would add a framed box where there was none.
chat/transcript-dialog.tsx:64 role="alert" Kept: an inline error line in the flow (text plus, at most, a Retry button), not a banner. Alert would add a framed box where there was none.
chat/worktree-picker-dialog.tsx:66 role="alert" Kept: an inline error line in the flow (text plus, at most, a Retry button), not a banner. Alert would add a framed box where there was none.
learning/learning-settings-page.tsx:293 role="alert" Kept: an inline error line in the flow (text plus, at most, a Retry button), not a banner. Alert would add a framed box where there was none.
learning/learning-settings-page.tsx:303 role="alert" Kept: an inline error line in the flow (text plus, at most, a Retry button), not a banner. Alert would add a framed box where there was none.
learning/learning-settings-page.tsx:339 role="alert" Kept: an inline error line in the flow (text plus, at most, a Retry button), not a banner. Alert would add a framed box where there was none.
learning/learning-settings-page.tsx:457 role="alert" Kept: an inline error line in the flow (text plus, at most, a Retry button), not a banner. Alert would add a framed box where there was none.
learning/proposal-row.tsx:301 role="alert" Kept: an inline error line in the flow (text plus, at most, a Retry button), not a banner. Alert would add a framed box where there was none.
memory/memory-fact-detail.tsx:134 role="alert" Kept: an inline error that takes the value's place in the fact detail.
memory/facts-about-you.tsx:29 role="alert" Kept: the shared settings Note, the same plain line its loading and empty states use (role="status"). Only the role differs.
chat/chat-composer.tsx:720 role="alert" Skipped: file is in open PRs #2320 and #2331.
chat/chat-composer.tsx:725 role="alert" Skipped: file is in open PRs #2320 and #2331.
chat/chat-composer.tsx:950 <label> Skipped: file is in open PRs #2320 and #2331.
chat/tool-call-list.tsx:209 aria-expanded Skipped: file is in open PR #2321.
memory/consolidate-memory-card.tsx:333 role="alert" Skipped: file is in open PR #2334.
settings/interface-settings.tsx:181 <label> Skipped: file is in open PR #2320.
settings/interface-settings.tsx:208 <label> Skipped: file is in open PR #2320.
settings/settings-workspace.tsx:150 <label> Skipped: file is in open PRs #2330 and #2331.
settings/settings-workspace.tsx:569 <label> Skipped: file is in open PRs #2330 and #2331.

The skipped sites are left for after those PRs merge, so this PR doesn't close #2340.

Behaviour differences

None visible, and none to assistive technology. Radix now generates the aria-controls id and adds data-state to the trigger and the content. The closed content is still unmounted, as with the old conditional render.

No regression

  • Pins first (cfd84de23), passing on main's code and on the head:

    • failed-turn-card.pinning.test.tsx (5): starts collapsed with the payload out of view; click toggles, renaming the button and revealing the controlled payload; Enter and Space toggle; the expand-details preference opens it.
    • reasoning-disclosure.pinning.test.tsx (5): the name with its line count, collapsed by default; click toggles the controlled text; the preference opens it.
    • learning-queue.test.tsx (+1): Details opens and closes from the keyboard and hides the sources when closed.
    • settings-card.pinning.test.tsx (2): a failed read is a single alert carrying the message; loading and notices stay polite status lines.
    • Existing tests already pin the worktree picker (worktree-picker.pinning.test.tsx) and both learned-skill alerts (knowledge-workspace.test.tsx).

    With main's versions of the six changed feature files, these 6 test files pass 43/43. On the head they pass 45/45 together with collapsible.test.tsx. No existing test was modified.

  • Mutation proof. Each mutation was applied alone and the focused tests rerun:

    Mutation Failing tests
    Failed turn: uncontrolled Collapsible 3
    Failed turn: trigger not wired to the Collapsible 4
    Reasoning: content force-mounted while closed 2
    Reasoning: expand-details preference ignored 1
    Suggestion: open change not handled (no detail read) 11
    Settings banner: not an alert 1
    Publication notice: not an alert 1
    Dialog error: not an alert 1
    Worktree card is a <span>, not a Label 2
    Worktree Label without htmlFor 0: equivalent. The Label wraps the radio, so wrapping alone associates them.
  • Pixels: 36 captures (9 states × desktop and 390px × light and dark), each compared with Before and with a second Before capture as the noise floor. All 36 are identical to Before. pixel-report.txt in the asset folder has the counts.

Screenshots

Before is origin/main (1210721be), After is this branch (89c808291), and Prototype is stack-08. Red boxes would mark pixels that differ between Before and After; there are none, because every capture is pixel-identical.

Failed turn, collapsed (identical. The prototype has no failed-turn card, so its column is the nearest transcript)

failed-turn-closed-desktop-light

Failed turn, collapsed: desktop dark and 390px, light and dark

failed-turn-closed-desktop-dark:

failed-turn-closed-desktop-dark

failed-turn-closed-mobile-light:

failed-turn-closed-mobile-light

failed-turn-closed-mobile-dark:

failed-turn-closed-mobile-dark

Failed turn and reasoning summary, both expanded (identical. Prototype column as above)

failed-turn-open-desktop-light

Failed turn and reasoning summary, both expanded: desktop dark and 390px, light and dark

failed-turn-open-desktop-dark:

failed-turn-open-desktop-dark

failed-turn-open-mobile-light:

failed-turn-open-mobile-light

failed-turn-open-mobile-dark:

failed-turn-open-mobile-dark

Worktree picker (identical. The prototype's picker has the same bordered option rows)

worktree-picker-desktop-light

Worktree picker: desktop dark and 390px, light and dark

worktree-picker-desktop-dark:

worktree-picker-desktop-dark

worktree-picker-mobile-light:

worktree-picker-mobile-light

worktree-picker-mobile-dark:

worktree-picker-mobile-dark

Worktree picker, a worktree chosen (identical)

worktree-picker-chosen-desktop-light

Worktree picker, a worktree chosen: desktop dark and 390px, light and dark

worktree-picker-chosen-desktop-dark:

worktree-picker-chosen-desktop-dark

worktree-picker-chosen-mobile-light:

worktree-picker-chosen-mobile-light

worktree-picker-chosen-mobile-dark:

worktree-picker-chosen-mobile-dark

Learning suggestion, Details closed (identical. Matches the prototype's suggestion card)

learning-closed-desktop-light

Learning suggestion, Details closed: desktop dark and 390px, light and dark

learning-closed-desktop-dark:

learning-closed-desktop-dark

learning-closed-mobile-light:

learning-closed-mobile-light

learning-closed-mobile-dark:

learning-closed-mobile-dark

Learning suggestion, Details open (identical. Matches the prototype's open Details)

learning-open-desktop-light

Learning suggestion, Details open: desktop dark and 390px, light and dark

learning-open-desktop-dark:

learning-open-desktop-dark

learning-open-mobile-light:

learning-open-mobile-light

learning-open-mobile-dark:

learning-open-mobile-dark

Storage settings, failed read banner (identical. The prototype has no failure state here, so its column shows the healthy page)

storage-error-desktop-light

Storage settings, failed read banner: desktop dark and 390px, light and dark

storage-error-desktop-dark:

storage-error-desktop-dark

storage-error-mobile-light:

storage-error-mobile-light

storage-error-mobile-dark:

storage-error-mobile-dark

Learned skills, publication notice (identical. The prototype has no publication notice)

learned-notice-desktop-light

Learned skills, publication notice: desktop dark and 390px, light and dark

learned-notice-desktop-dark:

learned-notice-desktop-dark

learned-notice-mobile-light:

learned-notice-mobile-light

learned-notice-mobile-dark:

learned-notice-mobile-dark

Learned skill dialog, action error (identical. The prototype's dialog has no action error or action buttons)

learned-error-desktop-light

Learned skill dialog, action error: desktop dark and 390px, light and dark

learned-error-desktop-dark:

learned-error-desktop-dark

learned-error-mobile-light:

learned-error-mobile-light

learned-error-mobile-dark:

learned-error-mobile-dark

Gates

On the head rebased onto origin/main (1210721be):

  • task studio:check: passed. Web 1503 tests in 175 files, server 210 tests in 26 files. The first run had one failure in an unrelated file, auth/auth-gate-account.test.tsx, under load. It passed 5 times in a row on its own, and the full rerun passed.
  • task studio:browser: passed. 124 passed, 30 skipped.

🤖 Generated with Claude Code

peppescg and others added 6 commits October 15, 2026 10:37
…anges

Pins the failed-turn and reasoning disclosures (names, aria-expanded, the
controlled region, click and Enter/Space, the expand-details preference),
keyboard toggling of a learning suggestion's Details, and the runtime status
line's alert and status announcements, against the unchanged code.

Refs #2340

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Adds the stock shadcn Collapsible over @radix-ui/react-collapsible, already
in the lockfile through the Accordion, for single show/hide disclosures.

Refs #2340

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The failed-turn card's Show details, the reasoning summary, and a learning
suggestion's Details now use Collapsible, CollapsibleTrigger and
CollapsibleContent instead of a button with hand-set aria-expanded and
aria-controls over conditionally rendered content. Names, styles, the
expand-details preference and the first-open detail read are unchanged.

The suggestion card spaces its children with flex gap instead of space-y,
so the collapsed content's hidden element adds no margin.

Refs #2340

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ption cards

Each eligible worktree's card is now a Label for its radio. font-normal keeps
the card's text weight, since Label defaults to font-medium.

Refs #2340

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…imitive

The settings runtime status line's failure banner, the learned-skills
publication notice, and the learned-skill dialog's action error are now
Alert with AlertDescription. They keep their tints, and the destructive
banner keeps full-strength destructive text: the Alert's destructive variant
fades the description to 90%, which drops below 4.5:1 on white, so it uses
the default variant with Studio's destructive tint instead.

Refs #2340

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Screenshots showed three layout shifts from the primitives' defaults:
- Label's leading-none shortened each worktree card; leading-[inherit]
  restores the dialog's line height.
- Alert's w-full stretched the storage error beside its Refresh button and
  pushed the button to a new line; w-auto sizes it like the paragraph it
  replaced, full width in block flow and content-sized in a flex row.
- AlertDescription's relaxed paragraph leading made the publication notice
  taller, and the Alert border framed a dialog error that had none; the
  notice inherits its line height again and the dialog error drops the
  border.

Refs #2340

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@kantord kantord left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, the per-call-site decision table made the audit easy to follow, and the Collapsible, Label and Alert swaps preserve roles, names and the controlled-open behaviour. Approving, with a few non-blocking nits:

  • The worktree picker Label overrides font-normal and leading-[inherit] but not the primitive's text-sm. I checked the children and visible text looks unaffected, but I could not render it, so "pixels unchanged" for that site is unverified on my side.
  • Label's default font-medium leading-none is now undone at several call sites across this PR and #2359 (font-normal on three, plus leading-[inherit] here). If it keeps recurring, the Label default may be the thing to revisit.
  • The worktree picker Label and the two learned-skills Alerts have no new test.

Fully or partially written by an AI agent.

@peppescg
peppescg merged commit b1e8c77 into main Oct 15, 2026
31 checks passed
@peppescg
peppescg deleted the refactor/studio-primitives-audit-2340 branch October 15, 2026 17:01
@toolhive-release-app toolhive-release-app Bot mentioned this pull request Oct 20, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Studio: audit native labels, disclosures and ad-hoc alerts against the primitives

2 participants