Repository navigation
feat: owner-controls UX — confirmations, shared share state, manual radiogroup (D1b, stacked on #57) - #59
Conversation
…al radiogroup - One OwnerControlsProvider, seeded by page.tsx, holds the share list and live count for the Share dialog, the privacy switch and the Delete confirmation. The switch and Delete no longer re-read /shares on every open; the Share dialog's own re-read after a write is what they all show. A pure reducer drops out-of-order list answers and marks a confirmed revoke at once. - page.tsx runs the handoff signing and both owner queries with Promise.all. - Revoking a share link asks first, in a ConfirmDialog nested in the Share dialog; failure keeps it open with the error; focus returns to the Revoke button on cancel and to the row on success. - Privacy radiogroup uses manual activation: arrows/Home/End move focus only, Space/Enter commits. A polite "Saved" status is announced after the PATCH. - Busy labels on Create link, Revoke link and Delete. - The artifact iframe is titled with the artifact's title (share page keeps "Artifact"); e2e specs select it by data-testid instead of title. - The back link to the dashboard is a next/link. E2E specs updated for the extra confirm click on revoke: share-link, share-count-expiry, visibility-private-share-warning. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…alog - User table: Deactivate, Make admin, Make member and Delete confirm through one ConfirmDialog; only Reactivate commits on the press. Row actions are described as data in user-actions.ts (unit-tested). Requests now catch network failures, and a refusal keeps the dialog open with the server's reason instead of closing it and showing the error behind the popup. - Invite revoke asks first; failure stays in the dialog; focus lands on the row once its Revoke button is gone. - Busy labels on Create invite, Create category, Rename, Activate/Deactivate, Apply filters, Load more and Reactivate. - Admin nav and "Back to artifacts" use next/link. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- Provider key: a refused save shows the API envelope's message when it has one (e.g. the 422 VALIDATION_FAILED for a base URL on a blocked address), falling back to the generic line for anything else. Network failures are caught. Remove asks first and keeps failures in the dialog. - API tokens: Revoke asks first; failure stays in the dialog; focus lands on the row once its Revoke button is gone. Create failures are caught. - Busy labels on Save/Update key, Remove key, Create token, Revoke token. - Settings nav and "Back to artifacts" use next/link. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Both were removed in #57. design.md now says tokens are consumed as CSS custom properties from globals.css and CSS modules; docs/motion.md replaces the Sonner toast row with the in-place status-message rule. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…Done
The DELETE's 204 reaches Playwright before the page has handled it, so the
revoke ConfirmDialog is still open, then in its 220 ms exit transition, when
the spec reaches for Done. getByText('Done') matched its description
("...cannot be undone...") as well as the Share dialog's Done button.
Wait for share-revoke-dialog to be hidden, then press the Done button by role
and exact name, scoped to the share dialog.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The shell is a one-column grid with an implicit `auto` column, which is floored at the min-content width of its widest item. On /admin/users that is the data table, so the column grew to ~640px and the header and nav (which already wrap) stretched with it past the viewport. `minmax(0, 1fr)` pins the column to the viewport; tables keep scrolling inside their overflow-x: auto wrappers. The settings shell had the same latent pattern and only passed because it has no table; it gets the same one-line fix. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
datj9
left a comment
There was a problem hiding this comment.
🤖 Automated review (Claude Code)
Verdict: Looks good to merge after a look at the MEDIUM below. No blocking correctness or security defects in what this PR adds on top of #57.
Findings: 0 CRITICAL, 0 HIGH, 1 MEDIUM, 1 LOW
Checks (run locally on dcddc8e):
pnpm typecheck: passpnpm lint: passpnpm vitest run --project unit: 77 files, 1222 tests, all pass- Playwright: not run (no Docker)
Summary
- MEDIUM
app/a/[id]/privacy-switch.tsx:132: the private-downgrade warning now trusts a page-lifetimeliveCount, which can be too low (a link created from the CLI / another tab, or a failed post-create re-read). The downgrade then goes through without warning that a live link still opens. The docs inowner-controls.tsxsay that staleness only ever overcounts, which does not hold for links created elsewhere. - LOW
app/a/[id]/share-dialog.tsx:215(the same pattern in user-table, token, invite and key managers):onOpenChangeis passed straight through, so Esc or a backdrop click closes the confirmation while the request is in flight. If that request fails, the error goes into the closed dialog's slot and nobody sees it. The user thinks it worked.
Everything else I checked holds up: the reducer's request-id ordering, the manual-activation radio keys, the Promise.all in page.tsx, the envelope-message fallback (length cap, non-JSON), the user-action request bodies, and focus restoration via finalFocus after a successful revoke.
| if (next === 'private') { | ||
| void choosePrivateOrConfirm() | ||
| // Only a downgrade that leaves live links open needs asking about. | ||
| if (next === 'private' && liveShareLinks > 0) { |
There was a problem hiding this comment.
MEDIUM: stale-low liveCount skips the private-downgrade warning.
Before this PR, pressing Only me re-read /shares and so saw the current count. Now liveShareLinks is the page-seeded context value, which is only updated by this page's own Share dialog. It undercounts in two realistic cases:
- A link is created from the CLI (
enclave share create) or another tab after this page loaded. - A create here succeeds but the follow-up
refreshShares()fails. That failure is swallowed inowner-controls.tsx, so the count stays at its old value.
In either case liveShareLinks === 0, so choose('private') runs with no confirmation. The owner believes the artifact is locked down, but a share link still opens it. That is the exact case this confirmation exists for.
The comment in owner-controls.tsx:25-27 says staleness "errs toward warning ... never toward staying silent". That only holds for revokes, not creates.
Cheap fix: before the private downgrade only (a rare, deliberate action), call refreshShares() and read the result, or keep the old one-shot read. Also consider bumping liveCount optimistically after a successful create when the re-read fails.
| {/* Rendered inside the popup so base-ui treats it as a nested dialog: Esc closes only it. */} | ||
| <ConfirmDialog | ||
| open={isRevokeOpen} | ||
| onOpenChange={setIsRevokeOpen} |
There was a problem hiding this comment.
LOW: the confirmation can be dismissed mid-request, and then its failure is lost.
ConfirmDialog forwards every onOpenChange (Esc, backdrop, Keep it) even while busy. So Esc during the DELETE closes the dialog. If the DELETE then fails, setRevokeError(REVOKE_FAILED) writes into a closed dialog, and the next requestRevoke clears it. The user sees nothing and the link is still live.
The same pattern appears in user-table.tsx:202, token-manager.tsx, invite-manager.tsx and key-manager.tsx.
Suggested fix: ignore close requests while busy, for example onOpenChange={(open) => { if (!open && isBusy) return; setIsRevokeOpen(open) }}. Or make that the default inside ConfirmDialog when busy.
…rite The create/save/revoke handlers in the token, invite and provider-key managers now catch errors, and their follow-up list read ran inside that try. A network error on the re-read after a successful POST showed 'That did not work' next to a token/invite that had been created (or a key that had been saved, with the form already reset). The re-reads now swallow their own failures, like the user table and owner-controls do. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Review summary (independent review of the delta against refactor/ui-primitives-and-deps) Fixed in bab3bd5:
Left as is (PLAUSIBLE, low):
Checked and rejected:
Gates: typecheck, lint, unit (1222 passed) and build all pass. |
A successful Delete removed the row and the button that opened the confirmation, so base-ui returned focus to a detached node and it fell to <body>. The target is now picked before the request: the next row's first action, else the previous row's, else that row itself (your own row has no actions; rows get tabIndex=-1), else the table (tabIndex=-1). The row is also removed locally on success, so it goes even if the re-read fails. Other confirmed actions keep returning focus to their trigger, which stays. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Follow-up: a7a5169 fixes the user-table Delete focus finding. The target is picked before the request. It is the next row's first action, else the previous row's. If that neighbour is your own row, which has no actions, focus goes to the row itself (rows now have tabIndex=-1). If there is no other row, focus goes to the table, which now has tabIndex=-1 and aria-label="Accounts". On success the row is also removed locally, so it goes even if the re-read fails. Other confirmed actions still return focus to their trigger. There's no DOM test environment in the unit project, so no unit test. typecheck, lint, unit (1222) and build pass. I haven't touched cancel-while-busy; that fix is going into #57's ConfirmDialog. |
… into feat/owner-controls-ux
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Plan item D1b
feat/owner-controls-ux.Changes, by plan item
1. Confirmations via
ConfirmDialogapp/a/[id]/share-dialog.tsx): a ConfirmDialog nested inside the Share dialog, so Esc closes only the confirmation. Test ids:share-revoke-dialog,share-revoke-confirm.app/settings/tokens/token-manager.tsx), invite revoke (app/admin/invites/invite-manager.tsx), provider-key remove (app/settings/keys/key-manager.tsx).app/admin/users/user-table.tsx): Deactivate, Make admin, Make member and Delete all go through one table-level ConfirmDialog. Reactivate still commits on the press, because it only restores access. The wording and request bodies live inuser-actions.ts, which has unit tests. The deactivate confirmation says how many shared artifacts stay readable.2.
user-table.tsxerror handling: requests now catch network failures. A refusal, such as the server's "still owns artifacts" message, stays inside the open dialog. It used to close the dialog and show the error behind the popup.3. Privacy radiogroup (
privacy-switch.tsx+radio-keys.ts)role="status"region says "Saved" and clears after 2.5 s. It is always mounted and sits on the hint's line, so an empty region takes no space and saving doesn't shift the layout.4. Owner-controls provider (
owner-controls.tsx+ pureowner-controls-state.ts)page.tsxseeds one provider with the share list andliveCount. The Share dialog, the privacy switch and the Delete confirmation all read from it./sharesevery time they open. The only list read left is the Share dialog's own re-read after a create or revoke.liveCountis only ever taken from the server, because expiry is judged by Postgresnow().page.tsxruns the handoff signing and both owner queries in onePromise.all.5. Busy labels and iframe title
aria-disabled, notdisabled, so it keeps focus.titleis now the artifact's title.ArtifactFramedefaults to "Artifact" when no title is passed, so/s/[token], which this PR does not own, is unchanged. The iframe also getsdata-testid="artifact-frame".6.
next/link: admin nav, settings nav, both "Back to artifacts" links, and the artifact page's "← Artifacts" link. The download links indownload-menu.tsxstay plain<a>because they hit route handlers that return files.Extra items
app/settings/keys/failure-message.ts): when a save or remove fails, the key manager showserror.messagefrom the response envelope, for example fix: trusted-proxy client IP, sign-in hardening, SSRF policy for base URLs, OIDC invite verification #56's 422 VALIDATION_FAILED for a base URL that targets a refused address. It falls back to the generic line if the body isn't JSON, isn't an envelope, or has an empty or implausibly long message (over 300 characters). Unit-tested.design.md§ Exports no longer mentions Tailwind; it describes tokens consumed as CSS custom properties fromglobals.cssand CSS modules. The Sonner toast row indocs/motion.mdis replaced with the in-place status-message rule.Accessibility notes
tabIndex={-1}and is not in the tab order. For key removal, focus goes to the stored-key status region. Without this, focus would fall to<body>.content: '·' / ''), so it isn't announced.E2E specs changed
Not run locally (no Docker). Changes are kept minimal:
share-link.spec.ts: revoke now clicksshare-revoke, thenshare-revoke-confirm. The "pressing twice" test now checks that the busy confirm button keeps focus and that only one DELETE is sent, plus that opening the confirmation sends nothing.share-count-expiry.spec.ts: extra confirm click; comment updated.visibility-private-share-warning.spec.ts: extra confirm click. It now waits for the Share badge to drop its count before pressing Only me. The switch reads the shared state that the post-revoke re-read fills, and it no longer fetches its own count.iframe[title="Artifact"]toiframe[data-testid="artifact-frame"]inshare-link,trash-delete-restore,two-account-privacy,viewer-sandbox,visibility-public-seoandzz-direct-artifact-entry.users-and-invites, which uses no UI revoke or deactivate;tokens-api-push, which revokes throughrequest.delete;signin-change-password, whose nav link keeps its role and name.Tests
pnpm typecheck: passpnpm lint: passpnpm vitest run --project unit: 77 files, 1222 tests, all passing. New files:privacy-radio-keys,owner-controls-state,admin-user-actions,provider-key-failure-message.pnpm buildwith the.env.examplevalues: passRisks
owner-controls.tsx.useFormStatusdoesn't apply; D2 covers auth-form submit buttons.design.mdanddocs/motion.md(requested) and thetests/e2e/**specs listed above.🤖 Generated with Claude Code