ui: confirming an alert dialog should close it #6

jules · 2026-07-14T05:00:16Z

jules commented

2026-07-14 05:00:16 +00:00

Confirming an alert dialog ran its action but never dismissed the dialog.

AlertDialogAction was a plain <Button> — only AlertDialogCancel wrapped the primitive's Close. Every call site already assumed confirming would close it.

Found in Trippy, where "Clear conversation" cleared the chat and left the modal sitting on screen. The same component is in every Crema app, so this is a sweep.

Delete-style dialogs hide the bug by accident — the row they delete unmounts out from under the open dialog and takes it with it. Any confirm whose target stays on screen (a rewrite, a regenerate, a clear) simply gets stuck.

Fixed at the component, so all call sites get it. The action still runs: Base UI merges its own close handler with the caller's onClick, so async work carries on behind the closed dialog and reports via toast as before.

Where the repo has a test setup, a regression test asserts both halves (it closes AND the action fires). It fails against the old component — verified.

🤖 Generated with Claude Code

Confirming an alert dialog ran its action but never dismissed the dialog. `AlertDialogAction` was a plain `<Button>` — only `AlertDialogCancel` wrapped the primitive's `Close`. Every call site already assumed confirming would close it. **Found in Trippy**, where "Clear conversation" cleared the chat and left the modal sitting on screen. The same component is in every Crema app, so this is a sweep. Delete-style dialogs hide the bug by accident — the row they delete unmounts out from under the open dialog and takes it with it. Any confirm whose target stays on screen (a rewrite, a regenerate, a clear) simply gets stuck. Fixed at the component, so all call sites get it. The action still runs: Base UI merges its own close handler with the caller's `onClick`, so async work carries on behind the closed dialog and reports via toast as before. Where the repo has a test setup, a regression test asserts both halves (it closes AND the action fires). It fails against the old component — verified. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jules added 7 commits 2026-07-14 05:00:16 +00:00

refactor: rename @crema/arcadia-client → @crema/arcadia-core-client ab116f8465

Disambiguates the Phoenix/auth client lib from lib-arcadia-agents-client.
Dir lib-arcadia-client → lib-arcadia-core-client; alias updated in
tsconfig paths, vite config, app.css @source, imports, CI and docs.

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

refactor: rename service references arcadia-app → arcadia-core 938143f3f5

The Phoenix auth/identity/tenancy backend repo is being renamed
arcadia-app → arcadia-core (its primary OTP app is already arcadia_core).
Updates prose, doc paths, and git.sky-ai.com repo URLs. Deliberately
leaves the Rust crate arcadia-app-client and host arcadia-app.internal
(handled separately), and the kept namespace (issuer/release "arcadia").

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

Admin UX overhaul: P0 fixes, error-UX foundations, nav cleanup, tenant detail page 7415b40240

From the 2026-07-14 UI/UX audit (17/40). Four phases:

P1 — Settings route crashed on Agents→Edit (Input/Textarea used but never
imported). Added imports + a shared route-level error boundary
(components/route-error.tsx) re-exported from all shell routes, so one
crashing panel degrades to an explained card with the nav intact instead of
replacing the whole app with a stack trace. Corrected the stale CLAUDE.md
claim that `npm run typecheck` crashes — it works, and would have caught the
missing import.

P2 — Error/empty/feedback foundations. Fixed useSession identity churn that
fired ~3x duplicate fetches per screen and self-inflicted 429s (referentially
stable snapshot). New lib/errors.ts (describeError → plain-language + the fix)
and components/data-state.tsx (DataState renders exactly one of
error/loading/empty/content, so a failed load never shows as "empty";
DialogError for in-dialog failures; 429 auto-retry). Rolled across all 15
list routes; every mutation now toasts. Surfaced+fixed two silent-failure
bugs (sso + buckets-CORS swallowed load errors; the latter could wipe rules
on save).

P3 — Nav IA + trust cleanup. Default-expanded rail; regrouped into 7 coherent
sections; collapsed the Apps/Plan/Entitlements stub triplication into one
honest /billing page; deleted fabricated seeded notifications and the dead
Help menu item; fixed the 403 copy (referenced a tenant switcher that doesn't
exist); removed orphan /assistant + /library routes; gated dev-seed login
hints behind DEV; renamed /activity → /audit-log with a redirect.

P4 — Tenant detail page (routes/tenants.$id.tsx + components/tenant-detail/*),
closing the provision→configure gap. 8 tabs (Overview, Plan & quotas,
Branding, Localization, Email & SMS, Feature flags, IP rules, Inbound
webhooks), each verified saving to the real backend. Row name links to detail.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Phase 5: platform feature-flags CRUD, impersonation, billing catalogue af2c8d6663

Three new platform screens on top of the Phase 1-4 work.

Feature flags (/feature-flags) — platform-wide flag registry. New route +
lib/arcadia/feature-flags.ts, capability platform.feature_flags, nav under
Automation. List/create/edit/delete with a per-row default toggle; pairs with
the Phase-4 per-tenant override tab.

Impersonation — "Impersonate" action on active users. Entirely client-side
token swap in session.ts (beginImpersonation parks the operator's session +
API token and swaps to the impersonation token; endImpersonation restores it),
with a sticky "Viewing as <email> — Stop" banner in the shell driven by the
JWT's impersonated_by claim. Stop is client-side because the impersonation
token carries the target's roles and can't reach the admin-gated /stop
endpoint; impersonation is stateless JWT so restoring the parked token is
sufficient.

Billing (/billing) — replaced the coming-soon stub with the real plan
catalogue from GET /billing/plans (lib/arcadia/billing.ts). Per-tenant plan
assignment stays on the tenant detail page; Entitlements + Apps remain honestly
marked "Soon".

Verified in-browser with real backend; typecheck adds zero errors (36→36).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Phase 6: restructure Settings honestly a03fbd9e9b

Settings was assistant config mislabeled, plus a dev-scaffold Account panel
("Wire ~/lib/identity.ts…") shown to users. Now:

- Assistant (was "LLM") + Personas (was "Agents", labelled "stored in this
  browser") — honest names for what these actually configure.
- Tenant (new, platform.tenants-gated) — reuses the Phase-4 Branding +
  Localization tab components scoped to the operator's active tenant. This is
  the component reuse the phase order was built for.
- Appearance — real inline pickers (theme/font/surface/background) instead of
  "use the icons in the appbar" text.
- Account — real read-only identity from the session + an Edit-profile link,
  replacing the dev scaffold.

Sections filter by capability. Typecheck 36→36, 24-route sweep clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Phase 7: polish — overview health, reduced-motion, mobile, a11y, copy-ids 3fcdddefda

- Overview health hero derives from subsystem probes when the backend gives no
  overall status, so it never says "Unknown" next to an all-Healthy Subsystems
  card (the audit's most jarring contradiction). Home error copy now uses the
  Phase-2 describeError instead of raw backend strings.
- Global prefers-reduced-motion guard (transitions, loops, aurora field) +
  motion-reduce on the sidebar width transition.
- Mobile: content reserves a top row (pt-16 md:p-6) so the floating hamburger
  never overlaps the page H1.
- A11y: collapsed rail items get an explicit aria-label.
- Monitoring: disk "busiest mount" tile shows the % as the value and the long
  mount path as a truncated caption, so it no longer clips mid-word.
- New CopyId component (click-to-copy UUIDs), wired into Profile.

Typecheck 36→36, 24-route sweep clean. Remaining items (lib a11y labels,
table aria-sort, dev-only monitoring data) noted in the spec.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

ui: confirming an alert dialog should close it f5a994e1d8

AlertDialogAction was a plain <Button> — only AlertDialogCancel wrapped the
primitive's Close — so confirming ran the action but never dismissed the
dialog. Found in Trippy, where "Clear conversation" cleared the chat and left
the modal sitting on screen; the same component is in every Crema app.

Delete-style dialogs hide it by accident: the row they act on unmounts out from
under the open dialog and takes it with it. Any confirm that leaves its target
on screen (a rewrite, a regenerate, a clear) just gets stuck.

Fixed at the component, so every call site gets it. The action still runs —
Base UI merges its own close handler with the caller's onClick — and async work
carries on behind the closed dialog.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

This pull request can be merged automatically.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin fix/alert-dialog-action-closes:fix/alert-dialog-action-closes

git checkout fix/alert-dialog-action-closes

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: CremaUIStudio/arcadia-admin#6