From c614e57db3494c2a6c59770f46883264d9ec0d43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Mon, 12 Oct 2026 21:37:12 +0200 Subject: [PATCH 01/19] Implement new shared confirm controller and block Also refactors tests to feature tests --- .changeset/mosaic-confirm-controller.md | 2 + .../__tests__/confirm.controller.test.ts | 127 +++++ .../src/blocks/confirm/confirm-shell.tsx | 150 ++++++ .../src/blocks/confirm/confirm.controller.ts | 148 ++++++ packages/mosaic/src/blocks/confirm/index.ts | 3 + .../__tests__/confirmation.controller.test.ts | 137 ----- .../__tests__/confirmation.feature.test.tsx | 483 ++++++++++++++++++ .../__tests__/confirmation.test.tsx | 277 ---------- .../confirmation/confirmation.controller.ts | 78 --- .../src/blocks/confirmation/confirmation.tsx | 291 ++--------- .../mosaic/src/blocks/confirmation/index.ts | 8 +- .../__tests__/destructive.controller.test.ts | 291 ----------- .../__tests__/destructive.feature.test.tsx | 381 +++++++------- .../__tests__/destructive.test.tsx | 378 -------------- .../destructive/destructive.controller.ts | 86 ---- .../src/blocks/destructive/destructive.tsx | 265 +++------- .../mosaic/src/blocks/destructive/index.ts | 7 +- .../src/utils/__tests__/errors.test.tsx | 25 +- packages/mosaic/src/utils/errors.ts | 22 +- 19 files changed, 1274 insertions(+), 1885 deletions(-) create mode 100644 .changeset/mosaic-confirm-controller.md create mode 100644 packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts create mode 100644 packages/mosaic/src/blocks/confirm/confirm-shell.tsx create mode 100644 packages/mosaic/src/blocks/confirm/confirm.controller.ts create mode 100644 packages/mosaic/src/blocks/confirm/index.ts delete mode 100644 packages/mosaic/src/blocks/confirmation/__tests__/confirmation.controller.test.ts create mode 100644 packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx delete mode 100644 packages/mosaic/src/blocks/confirmation/__tests__/confirmation.test.tsx delete mode 100644 packages/mosaic/src/blocks/confirmation/confirmation.controller.ts delete mode 100644 packages/mosaic/src/blocks/destructive/__tests__/destructive.controller.test.ts delete mode 100644 packages/mosaic/src/blocks/destructive/__tests__/destructive.test.tsx delete mode 100644 packages/mosaic/src/blocks/destructive/destructive.controller.ts diff --git a/.changeset/mosaic-confirm-controller.md b/.changeset/mosaic-confirm-controller.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-confirm-controller.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts new file mode 100644 index 00000000000..ea4c558c737 --- /dev/null +++ b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts @@ -0,0 +1,127 @@ +import { ClerkRuntimeError } from '@clerk/shared/error'; +import { act, renderHook, waitFor } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { deferred } from '../../../__tests__/async'; +import { createActor } from '../../../machine/create-actor'; +import { SaveError } from '../../../utils/errors'; +import { confirmMachine, useConfirmController } from '../confirm.controller'; + +const blocked = new SaveError({ global: { code: 'action_blocked', message: 'Raw server sentence.' } }); +const cancelled = new ClerkRuntimeError('cancelled', { code: 'reverification_cancelled' }); +afterEach(() => { + vi.restoreAllMocks(); +}); + +function start() { + const actor = createActor(confirmMachine).start(); + actor.send({ type: 'OPEN' }); + return actor; +} + +describe('confirmMachine', () => { + it('opens into confirming and cancels back to idle', () => { + const actor = start(); + expect(actor.getSnapshot().value).toBe('confirming'); + + actor.send({ type: 'CANCEL' }); + + expect(actor.getSnapshot().value).toBe('idle'); + }); + + it('runs the confirmed action and returns to idle when it lands', async () => { + const run = vi.fn(() => Promise.resolve()); + const actor = start(); + + actor.send({ type: 'CONFIRM', run }); + expect(actor.getSnapshot().value).toBe('pending'); + + await vi.waitFor(() => expect(actor.getSnapshot().value).toBe('idle')); + expect(run).toHaveBeenCalledOnce(); + expect(actor.getSnapshot().status).toBe('active'); + }); + + it('abandons the run when cancelled while pending, ignoring how it settles', async () => { + const run = deferred(); + const actor = start(); + actor.send({ type: 'CONFIRM', run: () => run.promise }); + + actor.send({ type: 'CANCEL' }); + expect(actor.getSnapshot().value).toBe('idle'); + + run.reject(blocked); + await run.promise.catch(() => undefined); + expect(actor.getSnapshot().value).toBe('idle'); + expect(actor.getSnapshot().context.error).toBeUndefined(); + }); + + it('lands back on confirming with the reason when the action fails', async () => { + const actor = start(); + actor.send({ type: 'CONFIRM', run: () => Promise.reject(blocked) }); + + await vi.waitFor(() => expect(actor.getSnapshot().value).toBe('confirming')); + expect(actor.getSnapshot().context.error).toEqual(blocked.formError.global); + }); + + it('keeps the cause of an unexpected error, never its message', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const cause = new Error('Cannot read properties of undefined'); + const actor = start(); + actor.send({ type: 'CONFIRM', run: () => Promise.reject(cause) }); + + await vi.waitFor(() => expect(actor.getSnapshot().value).toBe('confirming')); + expect(actor.getSnapshot().context.error).toEqual({ cause }); + }); + + it('closes without a reason when the action was cancelled by reverification', async () => { + const actor = start(); + actor.send({ type: 'CONFIRM', run: () => Promise.reject(cancelled) }); + + await vi.waitFor(() => expect(actor.getSnapshot().value).toBe('idle')); + expect(actor.getSnapshot().context.error).toBeUndefined(); + }); + + it('drops the error when cancelled, so the next open starts clean', async () => { + const actor = start(); + actor.send({ type: 'CONFIRM', run: () => Promise.reject(blocked) }); + await vi.waitFor(() => expect(actor.getSnapshot().context.error).toBeDefined()); + + actor.send({ type: 'CANCEL' }); + + expect(actor.getSnapshot().value).toBe('idle'); + expect(actor.getSnapshot().context.error).toBeUndefined(); + }); +}); + +describe('useConfirmController', () => { + it('ignores open while no block is mounted on the controller, and keeps one handle across renders', () => { + const { result, rerender } = renderHook(() => useConfirmController({ onConfirm: () => Promise.resolve() })); + const { handle } = result.current; + + act(() => result.current.open()); + rerender(); + + expect(result.current.isOpen).toBe(false); + expect(result.current.handle).toBe(handle); + }); + + it('ignores confirm while closed and while a run is pending', async () => { + const pending = deferred(); + const onConfirm = vi.fn(() => pending.promise); + const { result } = renderHook(() => useConfirmController({ onConfirm })); + + act(() => result.current.confirm()); + expect(onConfirm).not.toHaveBeenCalled(); + + act(() => result.current.onOpenChange(true)); + act(() => result.current.confirm()); + act(() => result.current.confirm()); + expect(onConfirm).toHaveBeenCalledOnce(); + + await act(async () => { + pending.resolve(); + await pending.promise; + }); + await waitFor(() => expect(result.current.isOpen).toBe(false)); + }); +}); diff --git a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx new file mode 100644 index 00000000000..2699d02df9e --- /dev/null +++ b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx @@ -0,0 +1,150 @@ +import type { ReactNode } from 'react'; + +import { Card } from '../../components/card'; +import type { DialogFocusTarget, DialogTriggerProps } from '../../components/dialog'; +import { Dialog } from '../../components/dialog'; +import { Flow } from '../../components/flow'; +import { Reverification } from '../../features/reverification'; +import { type FromPayload, resolveFromPayload as resolve } from '../../utils/resolve-from-payload'; +import type { ConfirmController } from './confirm.controller'; + +export interface ConfirmBlockProps { + controller: ConfirmController; + /** Element that opens the dialog, for a dialog that sits next to its button. A trigger carries no payload, so only a payload-less controller takes one */ + trigger?: [Payload] extends [void] ? DialogTriggerProps['render'] : never; + /** + * Where focus returns when the dialog closes. Default: the trigger + */ + finalFocus?: DialogFocusTarget; + /** Dialog heading, or a function of the payload */ + title: FromPayload; + /** What the action does and why it warrants a second look, or a function of the payload. Takes markup. */ + description: FromPayload; + /** Text of the confirming button, or a function of the payload */ + actionLabel: FromPayload; + /** Text of the cancel button (default: "Cancel") */ + cancelLabel?: string; +} + +/** What the confirm step renders from: the copy resolved for the payload and the controller's state. */ +export interface ConfirmStepProps { + payload: Payload; + title: string; + description: ReactNode; + actionLabel: string; + cancelLabel: string; + isPending: boolean; + errorMessage: string | undefined; + confirm: () => void; +} + +interface ConfirmShellProps extends ConfirmBlockProps { + initialFocus?: DialogFocusTarget; + children: (step: ConfirmStepProps) => ReactNode; +} + +interface ConfirmCardProps extends Omit< + ConfirmShellProps, + 'trigger' | 'initialFocus' | 'finalFocus' +> { + payload: Payload; + cancelLabel: string; +} + +function ConfirmCard({ + controller, + payload, + title, + description, + actionLabel, + cancelLabel, + children, +}: ConfirmCardProps) { + const { reverification, step } = controller; + const confirmStep = children({ + payload, + title: resolve(title, payload), + description: resolve(description, payload), + actionLabel: resolve(actionLabel, payload), + cancelLabel, + isPending: controller.isPending, + errorMessage: controller.errorMessage, + confirm: () => controller.confirm(payload), + }); + + return ( + + {reverification ? ( + + {() => ( + <> + {confirmStep} + + controller.onOpenChange(false)} + /> + + + )} + + ) : ( + confirmStep + )} + + ); +} + +/** + * The dialog around a confirm block: the root on the controller's handle, the popup and card, and + * the verify step a reverified action swaps in. The block renders the confirm step through + * `children`. + */ +export function ConfirmShell({ + controller, + trigger, + initialFocus, + finalFocus, + cancelLabel = 'Cancel', + ...card +}: ConfirmShellProps) { + return ( + + {({ payload }) => ( + <> + {trigger ? : null} + + + + + )} + + ); +} diff --git a/packages/mosaic/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts new file mode 100644 index 00000000000..be90f95ce05 --- /dev/null +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -0,0 +1,148 @@ +import { isReverificationCancelledError } from '@clerk/shared/error'; +import { useMemo } from 'react'; + +import type { DialogHandle } from '../../components/dialog'; +import { Dialog } from '../../components/dialog'; +import type { ReverificationController } from '../../features/reverification'; +import type { ErrorDescription } from '../../localization'; +import { useErrorText } from '../../localization'; +import { setup } from '../../machine/setup'; +import type { ErrorInvokeEvent } from '../../machine/types'; +import { useMachine } from '../../machine/use-machine'; +import { toLocalizableError } from '../../utils/errors'; + +export interface ConfirmContext { + run: () => Promise; + error: ErrorDescription | undefined; +} + +export type ConfirmEvent = { type: 'OPEN' } | { type: 'CONFIRM'; run: () => Promise } | { type: 'CANCEL' }; + +const { createMachine, assign, fromPromise } = setup(); + +function notSeated(): Promise { + return Promise.reject(new Error('confirm run is not seated')); +} + +export const confirmMachine = createMachine({ + id: 'confirm', + initial: 'idle', + context: { + run: notSeated, + error: undefined, + }, + states: { + idle: { + on: { + OPEN: { target: 'confirming', actions: assign(() => ({ error: undefined })) }, + }, + }, + confirming: { + on: { + CONFIRM: { target: 'pending', actions: assign((_, event) => ({ run: event.run, error: undefined })) }, + CANCEL: { target: 'idle', actions: assign(() => ({ error: undefined })) }, + }, + }, + pending: { + on: { + CANCEL: { target: 'idle', actions: assign(() => ({ error: undefined })) }, + }, + invoke: fromPromise(context => context.run(), { + onDone: { target: 'idle', actions: assign(() => ({ error: undefined })) }, + onError: [ + { + guard: (_: ConfirmContext, event: ErrorInvokeEvent) => isReverificationCancelledError(event.error), + target: 'idle', + }, + { + target: 'confirming', + actions: assign((_, event) => ({ error: toLocalizableError(event.error) })), + }, + ], + }), + }, + }, +}); + +export type ConfirmStep = 'confirm' | 'verify'; + +export interface ConfirmController { + /** The underlying handle that manages the dialog. Normally not needed, but exposed if you want to pass it to a detached `Dialog.Trigger`. */ + handle: DialogHandle; + isOpen: boolean; + isPending: boolean; + errorMessage: string | undefined; + /** Which card the dialog shows: the confirm step, or the reverification the action was interrupted by. */ + step: ConfirmStep; + reverification: ReverificationController | undefined; + /** Opens the dialog about `payload` */ + open: (payload: Payload) => void; + onOpenChange: (open: boolean) => void; + /** Runs the action for `payload`. The dialog closes when it resolves and shows why when it rejects. */ + confirm: (payload: Payload) => void; +} + +export interface ConfirmControllerOptions { + /** The action the dialog confirms. Resolve to close; reject to keep the dialog open showing why. */ + onConfirm: (payload: Payload) => Promise | void; + /** The flow `onConfirm` runs under, when the action may need step-up verification. */ + reverification?: ReverificationController; + /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ + errorFallback?: string; +} + +/** + * Owns a confirm dialog's lifecycle: open, pending, error, closed. One per action. Create it in the + * wrapper or controller layer, hand it to a `Confirmation` or `Destructive` block as `controller`, + * and open it from anywhere with `open(payload)`; the component derives its copy from the payload and + * calls back `confirm(payload)` when the user answers. + * + * With `reverification`, the dialog switches to the verify step while the action is interrupted, + * closes silently when the user cancels the challenge, and refuses to close while the action is + * past the point of cancelling. + */ +export function useConfirmController({ + onConfirm, + reverification, + errorFallback, +}: ConfirmControllerOptions): ConfirmController { + const handle = useMemo(() => Dialog.createHandle(), []); + const [snapshot, send] = useMachine(confirmMachine); + const errorText = useErrorText(); + const { error } = snapshot.context; + const isPending = snapshot.value === 'pending'; + const canClose = !isPending || Boolean(reverification?.onCancel); + + return { + handle, + isOpen: snapshot.value !== 'idle', + isPending, + errorMessage: error ? errorText(error, errorFallback) : undefined, + step: isPending && reverification?.visible ? 'verify' : 'confirm', + reverification, + open: payload => handle.open(payload), + onOpenChange: open => { + if (open) { + send({ type: 'OPEN' }); + return; + } + if (!canClose) { + return; + } + reverification?.onCancel?.(); + send({ type: 'CANCEL' }); + }, + confirm: payload => { + if (snapshot.value !== 'confirming') { + return; + } + reverification?.reset(); + send({ + type: 'CONFIRM', + run: async () => { + await onConfirm(payload); + }, + }); + }, + }; +} diff --git a/packages/mosaic/src/blocks/confirm/index.ts b/packages/mosaic/src/blocks/confirm/index.ts new file mode 100644 index 00000000000..0db115f4651 --- /dev/null +++ b/packages/mosaic/src/blocks/confirm/index.ts @@ -0,0 +1,3 @@ +export { useConfirmController } from './confirm.controller'; +export type { ConfirmController, ConfirmControllerOptions, ConfirmStep } from './confirm.controller'; +export type { ConfirmBlockProps } from './confirm-shell'; diff --git a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.controller.test.ts b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.controller.test.ts deleted file mode 100644 index b8aec6ccf65..00000000000 --- a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.controller.test.ts +++ /dev/null @@ -1,137 +0,0 @@ -import { createDeferredPromise } from '@clerk/shared/utils'; -import { act, renderHook, waitFor } from '@testing-library/react'; -import { afterEach, describe, expect, it, vi } from 'vitest'; - -import { createActor } from '../../../machine/create-actor'; -import { SaveError } from '../../../utils/errors'; -import { confirmationMachine, useConfirmationController } from '../confirmation.controller'; - -const blocked = new SaveError({ global: { code: 'action_blocked', message: 'Raw server sentence.' } }); - -afterEach(() => { - vi.restoreAllMocks(); -}); - -function start() { - const actor = createActor(confirmationMachine).start(); - actor.send({ type: 'OPEN' }); - return actor; -} - -describe('confirmationMachine', () => { - it('opens into confirming and cancels back to idle', () => { - const actor = start(); - expect(actor.getSnapshot().value).toBe('confirming'); - - actor.send({ type: 'CANCEL' }); - - expect(actor.getSnapshot().value).toBe('idle'); - }); - - it('runs the confirmed action and returns to idle when it lands', async () => { - const run = vi.fn(() => Promise.resolve()); - const actor = start(); - - actor.send({ type: 'CONFIRM', run }); - expect(actor.getSnapshot().value).toBe('pending'); - - await vi.waitFor(() => expect(actor.getSnapshot().value).toBe('idle')); - expect(run).toHaveBeenCalledOnce(); - expect(actor.getSnapshot().status).toBe('active'); - }); - - it('holds the dialog open while the action is pending', () => { - const actor = start(); - actor.send({ type: 'CONFIRM', run: () => new Promise(() => {}) }); - - actor.send({ type: 'CANCEL' }); - - expect(actor.getSnapshot().value).toBe('pending'); - }); - - it('lands back on confirming with the reason when the action fails', async () => { - const actor = start(); - actor.send({ type: 'CONFIRM', run: () => Promise.reject(blocked) }); - - await vi.waitFor(() => expect(actor.getSnapshot().value).toBe('confirming')); - expect(actor.getSnapshot().context.error).toEqual(blocked.formError.global); - }); - - it('keeps the cause of an unexpected error, never its message', async () => { - vi.spyOn(console, 'error').mockImplementation(() => {}); - const cause = new Error('Cannot read properties of undefined'); - const actor = start(); - actor.send({ type: 'CONFIRM', run: () => Promise.reject(cause) }); - - await vi.waitFor(() => expect(actor.getSnapshot().value).toBe('confirming')); - expect(actor.getSnapshot().context.error).toEqual({ cause }); - }); - - it('drops the error when cancelled, so the next open starts clean', async () => { - const actor = start(); - actor.send({ type: 'CONFIRM', run: () => Promise.reject(blocked) }); - await vi.waitFor(() => expect(actor.getSnapshot().context.error).toBeDefined()); - - actor.send({ type: 'CANCEL' }); - - expect(actor.getSnapshot().value).toBe('idle'); - expect(actor.getSnapshot().context.error).toBeUndefined(); - }); -}); - -describe('useConfirmationController', () => { - it('holds the dialog open across confirming and pending, then closes on success', async () => { - const { result } = renderHook(() => useConfirmationController()); - expect(result.current.isOpen).toBe(false); - - act(() => result.current.onOpenChange(true)); - expect(result.current.isOpen).toBe(true); - expect(result.current.isConfirming).toBe(false); - - act(() => result.current.onConfirm(() => Promise.resolve())); - expect(result.current.isOpen).toBe(true); - expect(result.current.isConfirming).toBe(true); - - await waitFor(() => expect(result.current.isOpen).toBe(false)); - }); - - it('surfaces a failure as the localized copy for its code and stays open', async () => { - const { result } = renderHook(() => useConfirmationController()); - act(() => result.current.onOpenChange(true)); - - act(() => result.current.onConfirm(() => Promise.reject(blocked))); - - await waitFor(() => expect(result.current.errorMessage).toMatch(/contact support/)); - expect(result.current.isOpen).toBe(true); - expect(result.current.isConfirming).toBe(false); - }); - - it('surfaces the fallback it was given for an unexpected error', async () => { - vi.spyOn(console, 'error').mockImplementation(() => {}); - const { result } = renderHook(() => useConfirmationController({ errorFallback: 'Unable to remove this member.' })); - act(() => result.current.onOpenChange(true)); - - act(() => result.current.onConfirm(() => Promise.reject(new Error('internal')))); - - await waitFor(() => expect(result.current.errorMessage).toBe('Unable to remove this member.')); - }); - - it('clears the previous error while a retry is pending', async () => { - const { result } = renderHook(() => useConfirmationController()); - act(() => result.current.onOpenChange(true)); - act(() => result.current.onConfirm(() => Promise.reject(blocked))); - await waitFor(() => expect(result.current.errorMessage).toMatch(/contact support/)); - - const pending = createDeferredPromise(); - try { - act(() => result.current.onConfirm(() => pending.promise)); - - expect(result.current.isOpen).toBe(true); - expect(result.current.isConfirming).toBe(true); - expect(result.current.errorMessage).toBeUndefined(); - } finally { - await act(() => pending.resolve()); - } - await waitFor(() => expect(result.current.isOpen).toBe(false)); - }); -}); diff --git a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx new file mode 100644 index 00000000000..688193e57d4 --- /dev/null +++ b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx @@ -0,0 +1,483 @@ +import { reverificationError } from '@clerk/shared/authorization-errors'; +import { act, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import type { ReactNode } from 'react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { deferred } from '../../../__tests__/async'; +import { clerkApiError } from '../../../__tests__/clerk-errors'; +import { openDialog } from '../../../__tests__/feature/dialog'; +import { holdRequests, serveFapi } from '../../../__tests__/feature/fake-fapi'; +import { fapiClient, fapiSession, fapiUser } from '../../../__tests__/feature/fapi'; +import { renderWithClerk } from '../../../__tests__/feature/render'; +import { Button } from '../../../components/button'; +import { Menu } from '../../../components/menu'; +import { useReverificationFlow } from '../../../features/reverification'; +import type { MosaicLocalization } from '../../../localization'; +import type { ConfirmController, ConfirmControllerOptions } from '../../confirm'; +import { useConfirmController } from '../../confirm'; +import { Confirmation } from '../confirmation'; + +type User = ReturnType; + +const session = fapiSession({ id: 'sess_1', user: fapiUser({ id: 'user_1' }) }); +const VERIFY = '/v1/client/sessions/sess_1/verify'; +const TITLE = 'Remove connected account'; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function Host({ + options, + children, +}: { + options: ConfirmControllerOptions; + children: (controller: ConfirmController) => ReactNode; +}) { + const controller = useConfirmController(options); + return <>{children(controller)}; +} + +async function renderAccount( + options: ConfirmControllerOptions, + { localization, description }: { localization?: MosaicLocalization; description?: ReactNode } = {}, +) { + const ref: { current: ConfirmController | undefined } = { current: undefined }; + serveFapi({ client: fapiClient([session]) }); + await renderWithClerk( + options={options}> + {controller => { + ref.current = controller; + return ( + Remove Google} + title={TITLE} + description={description ?? 'Google will be removed from this account.'} + actionLabel='Remove' + /> + ); + }} + , + undefined, + localization, + ); + const user = userEvent.setup(); + const trigger = () => screen.getByRole('button', { name: 'Remove Google' }); + const open = () => openDialog(user, trigger(), { name: TITLE }); + const closed = () => waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + return { user, ref, trigger, open, closed }; +} + +const confirmButton = () => screen.getByRole('button', { name: 'Remove' }); + +describe('Confirmation', () => { + it('opens from its trigger, confirms, closes once the action lands, and hands focus back', async () => { + const run = deferred(); + const onConfirm = vi.fn(() => run.promise); + const { user, trigger, open, closed } = await renderAccount({ onConfirm }); + expect(screen.queryByRole('dialog')).toBeNull(); + + const dialog = await open(); + expect(dialog).toHaveAccessibleDescription('Google will be removed from this account.'); + + await user.click(confirmButton()); + expect(onConfirm).toHaveBeenCalledOnce(); + expect(confirmButton()).toHaveAttribute('aria-busy', 'true'); + + run.resolve(); + await closed(); + expect(trigger()).toHaveFocus(); + }); + + it('renders markup in the description', async () => { + const { open } = await renderAccount( + { onConfirm: () => Promise.resolve() }, + { + description: ( + <> + preston@clerk.dev will be removed from this account. + + ), + }, + ); + + const dialog = await open(); + + expect(dialog).toHaveAccessibleDescription('preston@clerk.dev will be removed from this account.'); + expect(screen.getByText('preston@clerk.dev').tagName).toBe('STRONG'); + }); + + it.each([ + ['Cancel', (user: User) => user.click(screen.getByRole('button', { name: 'Cancel' }))], + ['Escape', (user: User) => user.keyboard('{Escape}')], + ['the corner dismiss', (user: User) => user.click(screen.getByRole('button', { name: 'Close' }))], + ['an outside press', (user: User) => user.click(document.body)], + ])('cancels from %s without running the action', async (_, dismiss) => { + const onConfirm = vi.fn(() => Promise.resolve()); + const { user, open, closed } = await renderAccount({ onConfirm }); + await open(); + + await dismiss(user); + + await closed(); + expect(onConfirm).not.toHaveBeenCalled(); + }); + + it('holds the dialog and ignores every way out while the action is pending', async () => { + const run = deferred(); + const onConfirm = vi.fn(() => run.promise); + const { user, ref, open, closed } = await renderAccount({ onConfirm }); + await open(); + await user.click(confirmButton()); + expect(confirmButton()).toHaveAttribute('aria-busy', 'true'); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + await user.keyboard('{Escape}'); + await user.click(document.body); + act(() => ref.current?.onOpenChange(false)); + + expect(screen.getByRole('dialog')).toBeInTheDocument(); + expect(onConfirm).toHaveBeenCalledOnce(); + + run.resolve(); + await closed(); + }); + + it('keeps the dialog open and explains a Clerk failure, then lets the retry succeed', async () => { + const onConfirm = vi + .fn<() => Promise>() + .mockRejectedValueOnce(clerkApiError('connected_account_required', 'Google is your only way to sign in.')) + .mockResolvedValueOnce(undefined); + const { user, open, closed } = await renderAccount({ onConfirm }); + await open(); + + await user.click(confirmButton()); + + await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Google is your only way to sign in.')); + expect(screen.getByRole('dialog')).toBeInTheDocument(); + expect(confirmButton()).not.toHaveAttribute('aria-busy', 'true'); + + await user.click(confirmButton()); + await closed(); + expect(onConfirm).toHaveBeenCalledTimes(2); + }); + + it('shows the localized copy for the code a refusal carries', async () => { + const { user, open } = await renderAccount( + { onConfirm: () => Promise.reject(clerkApiError('action_blocked', 'Raw server sentence.')) }, + { localization: { overrides: { 'errors.action_blocked': 'Acción bloqueada.' } } }, + ); + await open(); + + await user.click(confirmButton()); + + await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Acción bloqueada.')); + }); + + it('shows the fallback for an error Clerk cannot describe', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const { user, open } = await renderAccount({ + onConfirm: () => Promise.reject(new TypeError('Cannot read properties of undefined')), + errorFallback: 'Unable to remove this account.', + }); + await open(); + + await user.click(confirmButton()); + + await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Unable to remove this account.')); + }); + + it('shows the generic error when no fallback is given', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const { user, open } = await renderAccount({ onConfirm: () => Promise.reject(new Error('internal')) }); + await open(); + + await user.click(confirmButton()); + + await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Something went wrong. Please try again.')); + }); + + it('starts the next open clean after a failure', async () => { + const { user, open, closed } = await renderAccount({ + onConfirm: () => Promise.reject(clerkApiError('action_blocked', 'nope')), + }); + await open(); + await user.click(confirmButton()); + await waitFor(() => expect(screen.getByRole('alert')).not.toBeEmptyDOMElement()); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + await closed(); + await open(); + + expect(screen.getByRole('alert').textContent).toBe(''); + }); +}); + +interface Member { + id: string; + name: string; +} + +describe('Confirmation shared by rows', () => { + it('opens from any row with that row, derives its copy, confirms with it, and hands focus back', async () => { + const members: Member[] = [ + { id: 'mem_1', name: 'Preston' }, + { id: 'mem_2', name: 'Jane' }, + ]; + const onConfirm = vi.fn((_: Member) => Promise.resolve()); + serveFapi({ client: fapiClient([session]) }); + await renderWithClerk( + options={{ onConfirm }}> + {controller => ( + <> + {members.map(member => ( + + Actions for {member.name}} /> + + controller.open(member)} + > + Remove + + + + ))} + `Remove ${member.name}?`} + description={member => ( + <> + {member.name} will be removed from the organization. + + )} + actionLabel='Remove member' + /> + + )} + , + ); + const user = userEvent.setup(); + expect(screen.queryByRole('dialog')).toBeNull(); + + const firstTrigger = screen.getByRole('button', { name: 'Actions for Preston' }); + await user.click(firstTrigger); + await user.click(await screen.findByRole('menuitem', { name: 'Remove' })); + expect(await screen.findByRole('dialog')).toHaveAccessibleName('Remove Preston?'); + await user.click(screen.getByRole('button', { name: 'Cancel' })); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + await waitFor(() => expect(firstTrigger).toHaveFocus()); + + const secondTrigger = screen.getByRole('button', { name: 'Actions for Jane' }); + await user.click(secondTrigger); + await user.click(await screen.findByRole('menuitem', { name: 'Remove' })); + const dialog = await screen.findByRole('dialog'); + expect(dialog).toHaveAccessibleName('Remove Jane?'); + expect(dialog).toHaveAccessibleDescription('Jane will be removed from the organization.'); + expect(screen.getByText('Jane').tagName).toBe('STRONG'); + await user.click(screen.getByRole('button', { name: 'Remove member' })); + expect(onConfirm).toHaveBeenCalledExactlyOnceWith(members[1]); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(secondTrigger).toHaveFocus(); + }); +}); + +function ReverifiedHost({ + action, + cleanupMs, + settleMs, + beforeRunMs, +}: { + action: () => Promise; + cleanupMs?: number; + settleMs?: number; + beforeRunMs?: number; +}) { + const [run, reverification] = useReverificationFlow(action); + const controller = useConfirmController({ + onConfirm: async () => { + if (beforeRunMs !== undefined) { + await new Promise(resolve => setTimeout(resolve, beforeRunMs)); + } + try { + const result = await run(); + if (settleMs !== undefined) { + await new Promise(resolve => setTimeout(resolve, settleMs)); + } + return result; + } catch (error) { + if (cleanupMs !== undefined) { + await new Promise(resolve => setTimeout(resolve, cleanupMs)); + } + throw error; + } + }, + reverification, + }); + + return ( + <> + {controller.isOpen ? 'open' : 'closed'} + Remove Google} + title={TITLE} + description='This action asks for verification first.' + actionLabel='Remove' + /> + + ); +} + +const passwordVerification = { + client: fapiClient([session]), + verification: { secrets: { password: 'hunter2' }, firstFactors: [{ strategy: 'password' as const }] }, +}; + +async function renderReverified(props: Parameters[0]) { + await renderWithClerk(); + return { user: userEvent.setup() }; +} + +async function confirmIntoVerification(user: User) { + await openDialog(user, screen.getByRole('button', { name: 'Remove Google' }), { name: TITLE }); + await user.click(confirmButton()); +} + +async function untilConfirmStepIsGone() { + await waitFor(() => expect(screen.queryByText(TITLE)).toBeNull()); +} + +function watchForConfirmStep() { + const log: string[] = []; + const inspect = (records: MutationRecord[]) => { + for (const record of records) { + for (const node of record.addedNodes) { + if (node.textContent?.includes(TITLE)) { + log.push(`added: ${(node as Element).outerHTML?.slice(0, 120) ?? node.textContent}`); + } + } + } + }; + const observer = new MutationObserver(inspect); + observer.observe(document.body, { childList: true, subtree: true }); + + return () => { + inspect(observer.takeRecords()); + observer.disconnect(); + return log; + }; +} + +const interruptedOnce = () => vi.fn().mockResolvedValueOnce(reverificationError()).mockResolvedValue({ done: true }); + +describe('Confirmation with reverification', () => { + it('keeps the confirm step pending while the challenge starts, then swaps in the factor card', async () => { + serveFapi(passwordVerification); + const { user } = await renderReverified({ action: interruptedOnce() }); + const start = holdRequests('post', VERIFY); + + await confirmIntoVerification(user); + + expect(screen.getByText(TITLE)).toBeVisible(); + expect(confirmButton()).toHaveAttribute('aria-busy', 'true'); + expect(screen.queryByLabelText('Password')).toBeNull(); + + start.release(); + expect(await screen.findByLabelText('Password')).toBeVisible(); + expect(screen.getAllByRole('dialog')).toHaveLength(1); + await untilConfirmStepIsGone(); + }); + + it('closes from the error card without going back to the confirm step', async () => { + serveFapi({ client: fapiClient([session]) }); + const { user } = await renderReverified({ action: interruptedOnce(), cleanupMs: 400 }); + holdRequests('post', VERIFY).fail('some_new_error', 'Nope'); + + await confirmIntoVerification(user); + + expect(await screen.findByText('Something went wrong')).toBeVisible(); + await untilConfirmStepIsGone(); + const stopWatching = watchForConfirmStep(); + const buttons = screen.getAllByRole('button', { name: 'Close' }); + await user.click(buttons[buttons.length - 1]); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(stopWatching()).toEqual([]); + expect(screen.getByLabelText('Outcome')).toHaveTextContent('closed'); + }); + + for (const cleanupMs of [undefined, 400]) { + it(`closes from the factor card without going back to the confirm step (host cleans up after ${cleanupMs ?? 0}ms)`, async () => { + serveFapi(passwordVerification); + const { user } = await renderReverified({ action: interruptedOnce(), cleanupMs }); + await confirmIntoVerification(user); + + expect(await screen.findByLabelText('Password')).toBeVisible(); + await untilConfirmStepIsGone(); + const stopWatching = watchForConfirmStep(); + await user.click(screen.getAllByRole('button', { name: 'Close' })[0]); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(stopWatching()).toEqual([]); + expect(screen.getByLabelText('Outcome')).toHaveTextContent('closed'); + }); + } + + for (const settleMs of [undefined, 400]) { + it(`does not go back to the confirm step after a successful verification (host settles after ${settleMs ?? 0}ms)`, async () => { + serveFapi(passwordVerification); + const { user } = await renderReverified({ action: interruptedOnce(), settleMs }); + await confirmIntoVerification(user); + + const field = await screen.findByLabelText('Password'); + await untilConfirmStepIsGone(); + const stopWatching = watchForConfirmStep(); + await user.type(field, 'hunter2{Enter}'); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull(), { timeout: 3000 }); + expect(stopWatching()).toEqual([]); + expect(screen.getByLabelText('Outcome')).toHaveTextContent('closed'); + }); + } + + it('cannot be closed while the verified action is retrying', async () => { + const retry = deferred<{ done: true }>(); + const action = vi.fn().mockResolvedValueOnce(reverificationError()).mockReturnValueOnce(retry.promise); + serveFapi(passwordVerification); + const { user } = await renderReverified({ action }); + await confirmIntoVerification(user); + const field = await screen.findByLabelText('Password'); + + await user.type(field, 'hunter2{Enter}'); + await waitFor(() => expect(action).toHaveBeenCalledTimes(2)); + await user.keyboard('{Escape}'); + await user.click(screen.getAllByRole('button', { name: 'Close' })[0]); + expect(screen.getByRole('dialog')).toBeInTheDocument(); + expect(screen.getByLabelText('Outcome')).toHaveTextContent('open'); + + retry.resolve({ done: true }); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + }); + + it('does not show the previous attempt again when the host starts the next run after a delay', async () => { + const action = vi + .fn() + .mockResolvedValueOnce(reverificationError()) + .mockResolvedValueOnce(reverificationError()) + .mockResolvedValue({ done: true }); + serveFapi(passwordVerification); + const { user } = await renderReverified({ action, beforeRunMs: 400 }); + await confirmIntoVerification(user); + expect(await screen.findByLabelText('Password')).toBeVisible(); + await user.click(screen.getAllByRole('button', { name: 'Close' })[0]); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + + await openDialog(user, screen.getByRole('button', { name: 'Remove Google' }), { name: TITLE }); + const stopWatching = watchForConfirmStep(); + await user.click(confirmButton()); + await new Promise(resolve => setTimeout(resolve, 150)); + expect(screen.queryByLabelText('Password')).toBeNull(); + expect(await screen.findByLabelText('Password', undefined, { timeout: 3000 })).toBeVisible(); + expect(stopWatching()).toEqual([]); + }); +}); diff --git a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.test.tsx b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.test.tsx deleted file mode 100644 index 5c6d0c520ed..00000000000 --- a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.test.tsx +++ /dev/null @@ -1,277 +0,0 @@ -import { act, render, screen, waitFor } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import { afterEach, describe, expect, it, vi } from 'vitest'; - -import { clerkApiError } from '../../../__tests__/clerk-errors'; -import { Button } from '../../../components/button'; -import type { MosaicLocalization } from '../../../localization'; -import { MosaicProvider } from '../../../mosaic-provider'; -import type { ConfirmationControlledProps, ConfirmationHandleProps } from '../confirmation'; -import { Confirmation } from '../confirmation'; - -function renderBlock(overrides: Partial = {}) { - return render( - - - , - ); -} - -const confirmButton = () => screen.getByRole('button', { name: 'Remove' }); - -describe('Confirmation', () => { - it('renders nothing until the caller opens it', () => { - renderBlock({ open: false }); - - expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); - }); - - it('asks to open from the trigger', async () => { - const onOpenChange = vi.fn(); - const user = userEvent.setup(); - renderBlock({ open: false, onOpenChange, trigger: }); - - await user.click(confirmButton()); - - expect(onOpenChange).toHaveBeenCalledWith(true, expect.anything()); - }); - - it('confirms from the action', async () => { - const onConfirm = vi.fn(); - const user = userEvent.setup(); - renderBlock({ onConfirm }); - - await user.click(confirmButton()); - - expect(onConfirm).toHaveBeenCalledOnce(); - }); - - it('renders markup in the description', () => { - renderBlock({ - description: ( - <> - preston@clerk.dev will be removed from this account. - - ), - }); - - expect(screen.getByRole('alertdialog')).toHaveAccessibleDescription( - 'preston@clerk.dev will be removed from this account.', - ); - expect(screen.getByText('preston@clerk.dev').tagName).toBe('STRONG'); - }); - - it('asks to close from cancel', async () => { - const onOpenChange = vi.fn(); - const user = userEvent.setup(); - renderBlock({ onOpenChange }); - - await user.click(screen.getByRole('button', { name: 'Cancel' })); - - expect(onOpenChange).toHaveBeenCalledWith(false, expect.anything()); - }); - - it('explains a failed attempt', () => { - renderBlock({ errorMessage: 'Google is your only way to sign in.' }); - - expect(screen.getByRole('alert')).toHaveTextContent('Google is your only way to sign in.'); - }); - - it('stays inert while the caller is confirming', async () => { - const onConfirm = vi.fn(); - const user = userEvent.setup(); - renderBlock({ isConfirming: true, onConfirm }); - - expect(confirmButton()).toHaveAttribute('aria-busy', 'true'); - await user.click(confirmButton()); - expect(onConfirm).not.toHaveBeenCalled(); - }); -}); - -interface Member { - id: string; - name: string; -} - -const preston: Member = { id: 'mem_1', name: 'Preston Booth' }; - -function renderWithHandle( - onConfirm: ConfirmationHandleProps['onConfirm'] = () => Promise.resolve(), - { errorFallback, localization }: { errorFallback?: string; localization?: MosaicLocalization } = {}, -) { - const handle = Confirmation.createHandle(); - render( - - ( - <> - {member.name} will be removed from the organization. - - )} - actionLabel={member => `Remove ${member.name}`} - onConfirm={onConfirm} - /> - , - ); - return handle; -} - -afterEach(() => { - vi.restoreAllMocks(); -}); - -const removeButton = () => screen.getByRole('button', { name: 'Remove Preston Booth' }); - -describe('Confirmation with a handle', () => { - it('renders nothing until opened through the handle', () => { - renderWithHandle(); - - expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); - }); - - it('opens with a payload and renders the copy from it', () => { - const handle = renderWithHandle(); - - act(() => { - handle.open(preston); - }); - - expect(screen.getByRole('alertdialog')).toHaveAccessibleName('Remove member'); - expect(screen.getByRole('alertdialog')).toHaveAccessibleDescription( - 'Preston Booth will be removed from the organization.', - ); - expect(screen.getByText('Preston Booth').tagName).toBe('STRONG'); - expect(removeButton()).toBeInTheDocument(); - }); - - it('confirms with the payload and closes once the action lands', async () => { - const onConfirm = vi.fn(() => Promise.resolve()); - const user = userEvent.setup(); - const handle = renderWithHandle(onConfirm); - act(() => { - handle.open(preston); - }); - - await user.click(removeButton()); - - expect(onConfirm).toHaveBeenCalledWith(preston); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); - }); - - it('stays inert and open while the action is pending', async () => { - const user = userEvent.setup(); - const handle = renderWithHandle(() => new Promise(() => {})); - act(() => { - handle.open(preston); - }); - - await user.click(removeButton()); - - expect(removeButton()).toHaveAttribute('aria-busy', 'true'); - await user.keyboard('{Escape}'); - expect(screen.getByRole('alertdialog')).toBeInTheDocument(); - }); - - it('keeps the dialog open and explains a failed attempt', async () => { - const user = userEvent.setup(); - const handle = renderWithHandle(() => - Promise.reject(clerkApiError('organization_minimum_permissions_needed', 'Preston Booth is the last admin.')), - ); - act(() => { - handle.open(preston); - }); - - await user.click(removeButton()); - - await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Preston Booth is the last admin.')); - expect(screen.getByRole('alertdialog')).toBeInTheDocument(); - expect(removeButton()).not.toHaveAttribute('aria-busy', 'true'); - }); - - it('shows the localized copy for the code a refusal carries', async () => { - const user = userEvent.setup(); - const handle = renderWithHandle(() => Promise.reject(clerkApiError('action_blocked', 'Raw server sentence.')), { - localization: { overrides: { 'errors.action_blocked': 'Acción bloqueada.' } }, - }); - act(() => { - handle.open(preston); - }); - - await user.click(removeButton()); - - await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Acción bloqueada.')); - }); - - it('shows the fallback rather than what an unexpected error says', async () => { - vi.spyOn(console, 'error').mockImplementation(() => {}); - const user = userEvent.setup(); - const handle = renderWithHandle(() => Promise.reject(new TypeError('Cannot read properties of undefined')), { - errorFallback: 'Unable to remove this member.', - }); - act(() => { - handle.open(preston); - }); - - await user.click(removeButton()); - - await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Unable to remove this member.')); - }); - - it('shows the generic error when no fallback is given', async () => { - vi.spyOn(console, 'error').mockImplementation(() => {}); - const user = userEvent.setup(); - const handle = renderWithHandle(() => Promise.reject(new Error('internal'))); - act(() => { - handle.open(preston); - }); - - await user.click(removeButton()); - - await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Something went wrong. Please try again.')); - }); - - it('starts the next open clean after a failure', async () => { - const user = userEvent.setup(); - const handle = renderWithHandle(() => Promise.reject(clerkApiError('action_blocked', 'nope'))); - act(() => { - handle.open(preston); - }); - await user.click(removeButton()); - await waitFor(() => expect(screen.getByRole('alert')).not.toBeEmptyDOMElement()); - - await user.click(screen.getByRole('button', { name: 'Cancel' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); - act(() => { - handle.open(preston); - }); - - expect(screen.getByRole('alertdialog')).toBeInTheDocument(); - expect(screen.getByRole('alert').textContent).toBe(''); - }); - - it('closes through the handle', async () => { - const handle = renderWithHandle(); - act(() => { - handle.open(preston); - }); - expect(handle.isOpen).toBe(true); - - act(() => { - handle.close(); - }); - - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); - expect(handle.isOpen).toBe(false); - }); -}); diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.controller.ts b/packages/mosaic/src/blocks/confirmation/confirmation.controller.ts deleted file mode 100644 index 09aace70042..00000000000 --- a/packages/mosaic/src/blocks/confirmation/confirmation.controller.ts +++ /dev/null @@ -1,78 +0,0 @@ -import type { ErrorDescription } from '../../localization'; -import { useErrorText } from '../../localization'; -import { setup } from '../../machine/setup'; -import { useMachine } from '../../machine/use-machine'; -import { toLocalizableError } from '../../utils/errors'; - -export interface ConfirmationContext { - run: () => Promise; - error: ErrorDescription | undefined; -} - -export type ConfirmationEvent = { type: 'OPEN' } | { type: 'CONFIRM'; run: () => Promise } | { type: 'CANCEL' }; - -const { createMachine, assign, fromPromise } = setup(); - -function notSeated(): Promise { - return Promise.reject(new Error('confirmation run is not seated')); -} - -export const confirmationMachine = createMachine({ - id: 'confirmation', - initial: 'idle', - context: { - run: notSeated, - error: undefined, - }, - states: { - idle: { - on: { - OPEN: { target: 'confirming', actions: assign(() => ({ error: undefined })) }, - }, - }, - confirming: { - on: { - CONFIRM: { target: 'pending', actions: assign((_, event) => ({ run: event.run, error: undefined })) }, - CANCEL: { target: 'idle', actions: assign(() => ({ error: undefined })) }, - }, - }, - pending: { - invoke: fromPromise(context => context.run(), { - onDone: { target: 'idle', actions: assign(() => ({ error: undefined })) }, - onError: { - target: 'confirming', - actions: assign((_, event) => ({ error: toLocalizableError(event.error) })), - }, - }), - }, - }, -}); - -export interface ConfirmationController { - isOpen: boolean; - onOpenChange: (open: boolean) => void; - onConfirm: (run: () => Promise) => void; - isConfirming: boolean; - errorMessage: string | undefined; -} - -export interface ConfirmationControllerOptions { - /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ - errorFallback?: string; -} - -export function useConfirmationController({ - errorFallback, -}: ConfirmationControllerOptions = {}): ConfirmationController { - const [snapshot, send] = useMachine(confirmationMachine); - const errorText = useErrorText(); - const { error } = snapshot.context; - - return { - isOpen: snapshot.value === 'confirming' || snapshot.value === 'pending', - onOpenChange: open => send({ type: open ? 'OPEN' : 'CANCEL' }), - onConfirm: run => send({ type: 'CONFIRM', run }), - isConfirming: snapshot.value === 'pending', - errorMessage: error ? errorText(error, errorFallback) : undefined, - }; -} diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.tsx b/packages/mosaic/src/blocks/confirmation/confirmation.tsx index 9fc63d58d74..900efda95de 100644 --- a/packages/mosaic/src/blocks/confirmation/confirmation.tsx +++ b/packages/mosaic/src/blocks/confirmation/confirmation.tsx @@ -1,269 +1,76 @@ -import type { ReactNode } from 'react'; - import { Button, SubmitButton } from '../../components/button'; import { Card } from '../../components/card'; -import type { DialogFocusTarget, DialogHandle, DialogTriggerProps } from '../../components/dialog'; import { Dialog } from '../../components/dialog'; -import { type FromPayload, resolveFromPayload as resolve } from '../../utils/resolve-from-payload'; -import { useConfirmationController } from './confirmation.controller'; +import type { ConfirmBlockProps } from '../confirm/confirm-shell'; +import { ConfirmShell } from '../confirm/confirm-shell'; /** The weight the confirming button carries: an undoable action takes `primary`. */ export type ConfirmationColor = 'negative' | 'primary'; -interface ConfirmationCardProps { - color: ConfirmationColor; - finalFocus: DialogFocusTarget | undefined; - title: string; - description: ReactNode; - actionLabel: string; - cancelLabel: string; - onConfirm: () => void; - isConfirming: boolean; - errorMessage: string | undefined; -} - -function ConfirmationCard({ - color, - finalFocus, - title, - description, - actionLabel, - cancelLabel, - onConfirm, - isConfirming, - errorMessage, -}: ConfirmationCardProps) { - return ( - - - - {title} - {description} - - - {errorMessage} - - - - {cancelLabel} - - } - /> - - {actionLabel} - - - - - ); -} - -export interface ConfirmationControlledProps { - /** Whether the dialog is open */ - open: boolean; - /** The weight the confirming button carries. An action that can be undone takes `primary` (default: `negative`) */ - color?: ConfirmationColor; - /** - * Where focus returns when the dialog closes. Default: the trigger — which a confirmed removal - * may have taken off the page, so a list hands back the row that replaced it instead. - */ - finalFocus?: DialogFocusTarget; - /** Callback when open state changes */ - onOpenChange: (open: boolean) => void; - /** Element that opens the dialog */ - trigger?: DialogTriggerProps['render']; - /** Dialog heading */ - title: string; - /** What the action does and why it warrants a second look. Takes markup, for a name to emphasise */ - description: ReactNode; - /** Text of the confirming button */ - actionLabel: string; - /** Text of the cancel button (default: "Cancel") */ - cancelLabel?: string; - /** Callback when the action is confirmed */ - onConfirm: () => void; - /** Whether the confirmed action is in progress */ - isConfirming?: boolean; - /** Error message to display if the confirmed action fails */ - errorMessage?: string; -} - -function ControlledConfirmation({ - open, - color = 'negative', - finalFocus, - onOpenChange, - trigger, - title, - description, - actionLabel, - cancelLabel = 'Cancel', - onConfirm, - isConfirming = false, - errorMessage, -}: ConfirmationControlledProps) { - return ( - - {trigger ? : null} - - - ); -} - -/** - * Opens the block from anywhere with the payload the confirmation is about. Create with - * `Confirmation.createHandle()`. The copy is derived from the payload, so `open` requires one. - */ -export interface ConfirmationHandle extends DialogHandle { - open(payload: Payload): void; -} - -function createHandle(): ConfirmationHandle { - return Dialog.createHandle(); -} - -export interface ConfirmationHandleProps { - /** Opens the dialog with a payload from anywhere: `handle.open(payload)` */ - handle: ConfirmationHandle; +export interface ConfirmationProps extends ConfirmBlockProps { /** The weight the confirming button carries. An action that can be undone takes `primary` (default: `negative`) */ color?: ConfirmationColor; - /** - * Where focus returns when the dialog closes. Default: the trigger — which a confirmed removal - * may have taken off the page, so a list hands back the row that replaced it instead. - */ - finalFocus?: DialogFocusTarget; - /** Dialog heading, or a function of the payload */ - title: FromPayload; - /** What the action does and why it warrants a second look, or a function of the payload. Takes markup, for a name to emphasise */ - description: FromPayload; - /** Text of the confirming button, or a function of the payload */ - actionLabel: FromPayload; - /** Text of the cancel button (default: "Cancel") */ - cancelLabel?: string; - /** Runs the action for the payload. Resolve to close the dialog; reject to keep it open showing why */ - onConfirm: (payload: Payload) => Promise | void; - /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ - errorFallback?: string; -} - -function HandleConfirmation({ - handle, - color = 'negative', - finalFocus, - title, - description, - actionLabel, - cancelLabel = 'Cancel', - onConfirm, - errorFallback, -}: ConfirmationHandleProps) { - const controller = useConfirmationController({ errorFallback }); - - return ( - - {({ payload }) => - payload === undefined ? null : ( - - controller.onConfirm(async () => { - await onConfirm(payload); - }) - } - isConfirming={controller.isConfirming} - errorMessage={controller.errorMessage} - /> - ) - } - - ); } -export type ConfirmationProps = ConfirmationControlledProps | ConfirmationHandleProps; - /** * Confirmation dialog for an action worth a second look but not worth making the user type for. * Use `Destructive` for the destructive actions that are. `color` sets the weight the confirming * button carries: `negative` for what cannot be undone, `primary` for what can. * - * An `alertdialog`: it announces as an interruption, an outside press cannot answer it, and the - * card withholds its corner dismiss. Escape still closes it, the way the cancel action does. Under - * the phone band it arrives as a bottom sheet, within reach of the thumb that has to answer it. - * - * Two forms. Controlled: the caller owns `open`, `isConfirming`, and `errorMessage`, and the - * block holds nothing of its own. With a `handle`: the block owns all three. Mount it once, - * open it from anywhere with `handle.open(payload)`, and derive the copy and the action from - * that payload; the promise `onConfirm` returns decides whether it closes or explains a failure. + * Driven by a `useConfirmController`. A dialog that sits next to its button takes it as `trigger`; + * a dialog shared by a list is opened from each row with `controller.open(payload)`, and the copy + * is derived from that payload. * * @example - * send({ type: open ? 'OPEN' : 'CANCEL' })} - * trigger={} - * title='Remove connected account' - * description='Google will be removed from this account. You will no longer be able to use this connected account and any dependent features will no longer work.' - * actionLabel='Remove' - * onConfirm={() => send({ type: 'CONFIRM' })} - * isConfirming={snapshot.value === 'removing'} - * errorMessage={snapshot.context.errorMessage} - * /> + * const remove = useConfirmController({ onConfirm: member => removeMember(member.id) }); * - * @example - * const removeMember = Confirmation.createHandle(); + * remove.open(member)}>Remove * * <>{member.name} will be removed from the organization.} * actionLabel='Remove' - * onConfirm={member => api.removeMember(member.id)} * /> - * - * removeMember.open(member)}>Remove */ -export function Confirmation(props: ConfirmationProps) { - return 'handle' in props ? {...props} /> : ; +export function Confirmation({ color = 'negative', ...props }: ConfirmationProps) { + return ( + + {({ title, description, actionLabel, cancelLabel, isPending, errorMessage, confirm }) => ( + <> + + {title} + {description} + + + {errorMessage} + + + + {cancelLabel} + + } + /> + + {actionLabel} + + + + )} + + ); } - -Confirmation.createHandle = createHandle; diff --git a/packages/mosaic/src/blocks/confirmation/index.ts b/packages/mosaic/src/blocks/confirmation/index.ts index b161658f063..e19abdf3d95 100644 --- a/packages/mosaic/src/blocks/confirmation/index.ts +++ b/packages/mosaic/src/blocks/confirmation/index.ts @@ -1,8 +1,2 @@ export { Confirmation } from './confirmation'; -export type { - ConfirmationColor, - ConfirmationControlledProps, - ConfirmationHandle, - ConfirmationHandleProps, - ConfirmationProps, -} from './confirmation'; +export type { ConfirmationColor, ConfirmationProps } from './confirmation'; diff --git a/packages/mosaic/src/blocks/destructive/__tests__/destructive.controller.test.ts b/packages/mosaic/src/blocks/destructive/__tests__/destructive.controller.test.ts deleted file mode 100644 index ddae78d1491..00000000000 --- a/packages/mosaic/src/blocks/destructive/__tests__/destructive.controller.test.ts +++ /dev/null @@ -1,291 +0,0 @@ -import { ClerkAPIResponseError, ClerkRuntimeError } from '@clerk/shared/error'; -import { act, renderHook, waitFor } from '@testing-library/react'; -import { afterEach, describe, expect, it, vi } from 'vitest'; - -import { deferred } from '../../../__tests__/async'; -import type { ReverificationController } from '../../../features/reverification'; -import { SaveError } from '../../../utils/errors'; -import { useDestructiveController } from '../destructive.controller'; - -const idleReverification = { status: 'idle', visible: false, reset: vi.fn() } as ReverificationController; - -afterEach(() => { - vi.restoreAllMocks(); -}); - -describe('useDestructiveController', () => { - it('starts closed and opens from the opener or from onOpenChange', () => { - const { result } = renderHook(() => useDestructiveController({ onDelete: () => Promise.resolve() })); - expect(result.current.open).toBe(false); - expect(result.current.isDeleting).toBe(false); - - act(() => result.current.openDestructiveDialog()); - expect(result.current.open).toBe(true); - - act(() => result.current.onOpenChange(false)); - expect(result.current.open).toBe(false); - - act(() => result.current.onOpenChange(true)); - expect(result.current.open).toBe(true); - expect(result.current.isDeleting).toBe(false); - }); - - it('ignores the opener once the dialog is already open', () => { - const { result } = renderHook(() => useDestructiveController({ onDelete: () => Promise.resolve() })); - act(() => result.current.onOpenChange(true)); - act(() => { - void result.current.onDelete(); - }); - - act(() => result.current.openDestructiveDialog()); - - expect(result.current.isDeleting).toBe(true); - }); - - it('shows the fallback, never the message, of an unexpected error', async () => { - vi.spyOn(console, 'error').mockImplementation(() => {}); - const onDelete = () => Promise.reject(new Error('Cannot read properties of undefined')); - const { result } = renderHook(() => - useDestructiveController({ onDelete, errorFallback: 'Unable to delete this account.' }), - ); - act(() => result.current.onOpenChange(true)); - - await act(async () => { - await result.current.onDelete(); - }); - - expect(result.current.errorMessage).toBe('Unable to delete this account.'); - }); - - it('stays open and pending until the action resolves, then closes', async () => { - const pending = deferred(); - const onDelete = vi.fn(() => pending.promise); - const { result } = renderHook(() => useDestructiveController({ onDelete })); - act(() => result.current.onOpenChange(true)); - - act(() => { - void result.current.onDelete(); - }); - expect(onDelete).toHaveBeenCalledOnce(); - expect(result.current.open).toBe(true); - expect(result.current.isDeleting).toBe(true); - - await act(async () => { - pending.resolve(); - await pending.promise; - }); - await waitFor(() => expect(result.current.open).toBe(false)); - expect(result.current.isDeleting).toBe(false); - }); - - it('stays open with a message when the action rejects, and a retry can succeed', async () => { - const onDelete = vi - .fn<() => Promise>() - .mockRejectedValueOnce(new SaveError({ global: { code: 'action_blocked', message: 'Raw server sentence.' } })) - .mockResolvedValueOnce(undefined); - const { result } = renderHook(() => useDestructiveController({ onDelete })); - act(() => result.current.onOpenChange(true)); - - await act(async () => { - await result.current.onDelete(); - }); - expect(result.current.open).toBe(true); - expect(result.current.isDeleting).toBe(false); - expect(result.current.errorMessage).toMatch(/contact support/); - - await act(async () => { - await result.current.onDelete(); - }); - await waitFor(() => expect(result.current.open).toBe(false)); - expect(onDelete).toHaveBeenCalledTimes(2); - }); - - it('shows the API error when the action is rejected by Clerk', async () => { - const onDelete = vi.fn(() => - Promise.reject( - new ClerkAPIResponseError('Forbidden', { - status: 403, - data: [ - { - code: 'organization_minimum_permissions_needed', - message: 'Short', - long_message: 'There has to be at least one organization member with the minimum required permissions.', - }, - ], - }), - ), - ); - const { result } = renderHook(() => useDestructiveController({ onDelete })); - act(() => result.current.onOpenChange(true)); - - await act(async () => { - await result.current.onDelete(); - }); - - expect(result.current.open).toBe(true); - expect(result.current.errorMessage).toBe( - 'There has to be at least one organization member with the minimum required permissions.', - ); - }); - - it('closes without a message when reverification is cancelled', async () => { - const onDelete = vi.fn(() => - Promise.reject(new ClerkRuntimeError('cancelled', { code: 'reverification_cancelled' })), - ); - const { result } = renderHook(() => useDestructiveController({ onDelete, reverification: idleReverification })); - act(() => result.current.onOpenChange(true)); - - await act(async () => { - await result.current.onDelete(); - }); - - expect(result.current.open).toBe(false); - expect(result.current.errorMessage).toBeUndefined(); - }); - - it('ignores a close while the action is in flight, unless reverification is active', async () => { - const pending = deferred(); - const { result } = renderHook(() => - useDestructiveController({ onDelete: () => pending.promise, reverification: idleReverification }), - ); - act(() => result.current.onOpenChange(true)); - act(() => { - void result.current.onDelete(); - }); - - act(() => result.current.onOpenChange(false)); - expect(result.current.open).toBe(true); - expect(result.current.isDeleting).toBe(true); - - await act(async () => { - pending.resolve(); - await pending.promise; - }); - }); - - it('lets an active reverification close the dialog while the action is still pending', () => { - const { result } = renderHook(() => - useDestructiveController({ - onDelete: () => new Promise(() => {}), - reverification: { - status: 'ready', - visible: true, - reset: vi.fn(), - onCancel: vi.fn(), - } as ReverificationController, - }), - ); - act(() => result.current.onOpenChange(true)); - act(() => { - void result.current.onDelete(); - }); - - act(() => result.current.onOpenChange(false)); - - expect(result.current.open).toBe(false); - }); - - it('ignores a close while reverification is retrying the action', () => { - const { result } = renderHook(() => - useDestructiveController({ - onDelete: () => new Promise(() => {}), - reverification: { status: 'retrying', visible: true, reset: vi.fn() } as ReverificationController, - }), - ); - act(() => result.current.onOpenChange(true)); - act(() => { - void result.current.onDelete(); - }); - - act(() => result.current.onOpenChange(false)); - - expect(result.current.open).toBe(true); - expect(result.current.isDeleting).toBe(true); - }); - - it('passes reverification through', () => { - const { result } = renderHook(() => - useDestructiveController({ onDelete: () => Promise.resolve(), reverification: idleReverification }), - ); - - expect(result.current.reverification).toBe(idleReverification); - }); - - it('is on the confirm step without reverification', () => { - const { result } = renderHook(() => useDestructiveController({ onDelete: () => Promise.resolve() })); - - expect(result.current.step).toBe('confirm'); - }); - - it.each([ - [false, 'confirm'], - [true, 'verify'], - ] as const)('while deleting, reverification visible=%s uses the %s step', (visible, step) => { - const { result } = renderHook(() => - useDestructiveController({ - onDelete: () => new Promise(() => {}), - reverification: { status: 'ready', visible, reset: vi.fn() } as ReverificationController, - }), - ); - act(() => result.current.onOpenChange(true)); - act(() => { - void result.current.onDelete(); - }); - - expect(result.current.step).toBe(step); - }); - - it('stays on the confirm step when not deleting, whatever the reverification status', () => { - const { result } = renderHook(() => - useDestructiveController({ - onDelete: () => Promise.resolve(), - reverification: { - status: 'ready', - visible: true, - reset: vi.fn(), - onCancel: vi.fn(), - } as ReverificationController, - }), - ); - act(() => result.current.onOpenChange(true)); - - expect(result.current.step).toBe('confirm'); - }); - - it('releases the previous reverification before a new delete starts', () => { - const reset = vi.fn(); - const { result } = renderHook(() => - useDestructiveController({ - onDelete: () => new Promise(() => {}), - reverification: { status: 'idle', visible: false, reset } as ReverificationController, - }), - ); - act(() => result.current.onOpenChange(true)); - expect(reset).not.toHaveBeenCalled(); - - act(() => { - void result.current.onDelete(); - }); - - expect(reset).toHaveBeenCalledOnce(); - }); - - it('cancels reverification when the dialog closes', () => { - const onCancel = vi.fn(); - const { result } = renderHook(() => - useDestructiveController({ - onDelete: () => new Promise(() => {}), - reverification: { status: 'ready', visible: true, reset: vi.fn(), onCancel } as ReverificationController, - }), - ); - act(() => result.current.onOpenChange(true)); - act(() => { - void result.current.onDelete(); - }); - - act(() => result.current.onOpenChange(false)); - - expect(onCancel).toHaveBeenCalledOnce(); - expect(result.current.open).toBe(false); - }); -}); diff --git a/packages/mosaic/src/blocks/destructive/__tests__/destructive.feature.test.tsx b/packages/mosaic/src/blocks/destructive/__tests__/destructive.feature.test.tsx index d5cf162028c..1f0a71eb509 100644 --- a/packages/mosaic/src/blocks/destructive/__tests__/destructive.feature.test.tsx +++ b/packages/mosaic/src/blocks/destructive/__tests__/destructive.feature.test.tsx @@ -1,216 +1,239 @@ import { reverificationError } from '@clerk/shared/authorization-errors'; -import { screen, waitFor } from '@testing-library/react'; +import { act, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { http, HttpResponse } from 'msw'; +import type { ReactNode } from 'react'; import { describe, expect, it, vi } from 'vitest'; +import { deferred } from '../../../__tests__/async'; +import { clerkApiError } from '../../../__tests__/clerk-errors'; import { openDialog } from '../../../__tests__/feature/dialog'; -import { fapiUrl, serveFapi, worker } from '../../../__tests__/feature/fake-fapi'; +import { serveFapi } from '../../../__tests__/feature/fake-fapi'; import { fapiClient, fapiSession, fapiUser } from '../../../__tests__/feature/fapi'; import { renderWithClerk } from '../../../__tests__/feature/render'; +import { Button } from '../../../components/button'; +import { Menu } from '../../../components/menu'; import { useReverificationFlow } from '../../../features/reverification'; +import type { ConfirmController, ConfirmControllerOptions } from '../../confirm'; +import { useConfirmController } from '../../confirm'; import { Destructive } from '../destructive'; -import { useDestructiveController } from '../destructive.controller'; const session = fapiSession({ id: 'sess_1', user: fapiUser({ id: 'user_1' }) }); +const TITLE = 'Delete account?'; -const CONFIRM_TITLE = 'Delete account?'; -const VERIFY = '/v1/client/sessions/sess_1/verify'; - -function Host({ - action, - cleanupMs, - settleMs, - beforeRunMs, +function Host({ + options, + children, }: { - action: () => Promise; - cleanupMs?: number; - settleMs?: number; - beforeRunMs?: number; + options: ConfirmControllerOptions; + children: (controller: ConfirmController) => ReactNode; }) { - const [run, reverification] = useReverificationFlow(action); - const controller = useDestructiveController({ - onDelete: async () => { - if (beforeRunMs !== undefined) { - await new Promise(resolve => setTimeout(resolve, beforeRunMs)); - } - try { - const result = await run(); - if (settleMs !== undefined) { - await new Promise(resolve => setTimeout(resolve, settleMs)); - } - return result; - } catch (error) { - if (cleanupMs !== undefined) { - await new Promise(resolve => setTimeout(resolve, cleanupMs)); - } - throw error; - } - }, - reverification, - }); + const controller = useConfirmController(options); + return <>{children(controller)}; +} - return ( - <> - - {controller.open ? 'open' : 'closed'} - - +async function renderAccount(options: ConfirmControllerOptions) { + const ref: { current: ConfirmController | undefined } = { current: undefined }; + serveFapi({ client: fapiClient([session]) }); + await renderWithClerk( + options={options}> + {controller => { + ref.current = controller; + return ( + Delete my account} + title={TITLE} + description='All of your data will be permanently deleted.' + fieldLabel='Type Delete to continue' + confirmationValue='Delete' + actionLabel='Delete account' + /> + ); + }} + , ); + const user = userEvent.setup(); + const open = () => openDialog(user, screen.getByRole('button', { name: 'Delete my account' }), { name: TITLE }); + const closed = () => waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + return { user, ref, open, closed }; } -async function typeConfirmation(user: ReturnType) { - await openDialog(user, screen.getByRole('button', { name: 'Open' })); - await user.type(screen.getByRole('textbox'), 'Delete'); -} +const field = () => screen.getByRole('textbox'); +const confirmButton = () => screen.getByRole('button', { name: 'Delete account' }); -async function openVerification() { - const user = userEvent.setup(); - await typeConfirmation(user); - await user.click(screen.getByRole('button', { name: 'Delete account' })); - return { user }; -} +describe('Destructive', () => { + it('focuses the field, holds the action until the phrase matches, and submits on Enter', async () => { + const onConfirm = vi.fn(() => Promise.resolve()); + const { user, open, closed } = await renderAccount({ onConfirm }); + await open(); -async function untilConfirmStepIsGone() { - await waitFor(() => expect(screen.queryByText(CONFIRM_TITLE)).toBeNull()); -} + await waitFor(() => expect(field()).toHaveFocus()); + expect(confirmButton()).toHaveAttribute('aria-disabled', 'true'); -function watchForConfirmStep() { - const log: string[] = []; - const inspect = (records: MutationRecord[]) => { - for (const record of records) { - for (const node of record.addedNodes) { - if (node.textContent?.includes(CONFIRM_TITLE)) { - log.push(`added: ${(node as Element).outerHTML?.slice(0, 120) ?? node.textContent}`); - } - } - } - }; - const observer = new MutationObserver(inspect); - observer.observe(document.body, { childList: true, subtree: true }); - - return () => { - inspect(observer.takeRecords()); - observer.disconnect(); - return log; - }; -} + await user.keyboard('Delet{Enter}'); + expect(onConfirm).not.toHaveBeenCalled(); + expect(confirmButton()).toHaveAttribute('aria-disabled', 'true'); -const passwordVerification = { - client: fapiClient([session]), - verification: { secrets: { password: 'hunter2' }, firstFactors: [{ strategy: 'password' as const }] }, -}; + await user.keyboard('e'); + expect(confirmButton()).not.toHaveAttribute('aria-disabled'); + await user.keyboard('{Enter}'); + expect(onConfirm).toHaveBeenCalledOnce(); + await closed(); + }); -describe('Destructive with reverification', () => { - it('closes from the error card without going back to the confirm step', async () => { - const action = vi.fn().mockResolvedValueOnce(reverificationError()).mockResolvedValue({ done: true }); + it('disables the field and ignores every way out while the action is pending', async () => { + const run = deferred(); + const onConfirm = vi.fn(() => run.promise); + const { user, ref, open, closed } = await renderAccount({ onConfirm }); + await open(); + await user.type(field(), 'Delete{Enter}'); + + expect(field()).toBeDisabled(); + expect(confirmButton()).toHaveAttribute('aria-busy', 'true'); + await user.click(screen.getByRole('button', { name: 'Cancel' })); + await user.keyboard('{Escape}'); + act(() => ref.current?.onOpenChange(false)); + expect(screen.getByRole('dialog')).toBeInTheDocument(); + expect(onConfirm).toHaveBeenCalledOnce(); + + run.resolve(); + await closed(); + }); + + it('marks the field invalid and keeps the phrase on failure, starts the next open clean, and lets a retry succeed', async () => { + const onConfirm = vi + .fn<() => Promise>() + .mockRejectedValueOnce(clerkApiError('subscription_active', 'Your subscription is still active.')) + .mockResolvedValueOnce(undefined); + const { user, open, closed } = await renderAccount({ onConfirm }); + await open(); + + await user.type(field(), 'Delete{Enter}'); + + expect(await screen.findByText('Your subscription is still active.')).toBeInTheDocument(); + expect(field()).toHaveAttribute('aria-invalid', 'true'); + expect(field()).toHaveValue('Delete'); + expect(confirmButton()).not.toHaveAttribute('aria-busy', 'true'); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + await closed(); + await open(); + expect(field()).toHaveValue(''); + expect(screen.queryByText('Your subscription is still active.')).toBeNull(); + + await user.type(field(), 'Delete'); + await user.click(confirmButton()); + await closed(); + expect(onConfirm).toHaveBeenCalledTimes(2); + }); +}); + +interface Project { + id: string; + name: string; +} + +describe('Destructive shared by rows', () => { + it('asks for each row its own phrase and starts every row with an empty field', async () => { + const projects: Project[] = [ + { id: 'project_1', name: 'Production' }, + { id: 'project_2', name: 'Staging' }, + ]; + const onConfirm = vi.fn((_: Project) => Promise.resolve()); serveFapi({ client: fapiClient([session]) }); await renderWithClerk( - , - ); - worker.use( - http.post(fapiUrl(VERIFY), () => - HttpResponse.json({ errors: [{ code: 'some_new_error', message: 'Nope' }] }, { status: 400 }), - ), + options={{ onConfirm }}> + {controller => ( + <> + {projects.map(project => ( + + Actions for {project.name}} /> + + controller.open(project)} + > + Delete + + + + ))} + `Delete ${project.name}?`} + description={project => `${project.name} will be permanently deleted.`} + fieldLabel={project => `Type ${project.name} to continue`} + confirmationValue={project => project.name} + actionLabel='Delete project' + /> + + )} + , ); - const { user } = await openVerification(); + const user = userEvent.setup(); - expect(await screen.findByText('Something went wrong')).toBeVisible(); - const buttons = screen.getAllByRole('button', { name: 'Close' }); - await untilConfirmStepIsGone(); - const stopWatching = watchForConfirmStep(); - await user.click(buttons[buttons.length - 1]); + await user.click(screen.getByRole('button', { name: 'Actions for Production' })); + await user.click(await screen.findByRole('menuitem', { name: 'Delete' })); + expect(await screen.findByRole('dialog')).toHaveAccessibleName('Delete Production?'); + await user.type(screen.getByRole('textbox', { name: 'Type Production to continue' }), 'Production'); + await user.click(screen.getByRole('button', { name: 'Cancel' })); await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); - expect(stopWatching()).toEqual([]); - expect(screen.getByLabelText('Outcome')).toHaveTextContent('closed'); + await user.click(screen.getByRole('button', { name: 'Actions for Staging' })); + await user.click(await screen.findByRole('menuitem', { name: 'Delete' })); + expect(await screen.findByRole('dialog')).toHaveAccessibleName('Delete Staging?'); + const stagingField = screen.getByRole('textbox', { name: 'Type Staging to continue' }); + expect(stagingField).toHaveValue(''); + await user.type(stagingField, 'Production{Enter}'); + expect(onConfirm).not.toHaveBeenCalled(); + await user.clear(stagingField); + await user.type(stagingField, 'Staging'); + await user.click(screen.getByRole('button', { name: 'Delete project' })); + expect(onConfirm).toHaveBeenCalledExactlyOnceWith(projects[1]); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); }); +}); - for (const cleanupMs of [undefined, 400]) { - it(`closes from the factor card without going back to the confirm step (host cleans up after ${cleanupMs ?? 0}ms)`, async () => { - const action = vi.fn().mockResolvedValueOnce(reverificationError()).mockResolvedValue({ done: true }); - serveFapi(passwordVerification); - await renderWithClerk( - , - ); - const { user } = await openVerification(); - - expect(await screen.findByLabelText('Password')).toBeVisible(); - await untilConfirmStepIsGone(); - const stopWatching = watchForConfirmStep(); - await user.click(screen.getAllByRole('button', { name: 'Close' })[0]); - await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); - expect(stopWatching()).toEqual([]); - - expect(screen.getByLabelText('Outcome')).toHaveTextContent('closed'); - }); - } - - for (const settleMs of [undefined, 400]) { - it(`does not go back to the confirm step after a successful verification (host settles after ${settleMs ?? 0}ms)`, async () => { - const action = vi.fn().mockResolvedValueOnce(reverificationError()).mockResolvedValue({ done: true }); - serveFapi(passwordVerification); - await renderWithClerk( - , - ); - const { user } = await openVerification(); - - const field = await screen.findByLabelText('Password'); - await untilConfirmStepIsGone(); - const stopWatching = watchForConfirmStep(); - await user.type(field, 'hunter2{Enter}'); - await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull(), { timeout: 3000 }); - expect(stopWatching()).toEqual([]); - - expect(screen.getByLabelText('Outcome')).toHaveTextContent('closed'); - }); - } +function ReverifiedHost({ action }: { action: () => Promise }) { + const [run, reverification] = useReverificationFlow(action); + const controller = useConfirmController({ onConfirm: run, reverification }); + + return ( + Delete my account} + title={TITLE} + description='This action asks for verification first.' + fieldLabel='Type Delete to continue' + confirmationValue='Delete' + actionLabel='Delete account' + /> + ); +} - it('does not show the previous attempt again when the host starts the next run after a delay', async () => { +describe('Destructive with reverification', () => { + it('keeps the typed phrase through the verify step when the retried action fails', async () => { const action = vi .fn() .mockResolvedValueOnce(reverificationError()) - .mockResolvedValueOnce(reverificationError()) - .mockResolvedValue({ done: true }); - serveFapi(passwordVerification); - await renderWithClerk( - , - ); - const { user } = await openVerification(); - expect(await screen.findByLabelText('Password')).toBeVisible(); - await user.click(screen.getAllByRole('button', { name: 'Close' })[0]); - await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); - - await typeConfirmation(user); - const stopWatching = watchForConfirmStep(); - await user.click(screen.getByRole('button', { name: 'Delete account' })); - await new Promise(resolve => setTimeout(resolve, 150)); - expect(screen.queryByLabelText('Password')).toBeNull(); - expect(await screen.findByLabelText('Password', undefined, { timeout: 3000 })).toBeVisible(); - expect(stopWatching()).toEqual([]); + .mockRejectedValueOnce(clerkApiError('subscription_active', 'Your subscription is still active.')); + serveFapi({ + client: fapiClient([session]), + verification: { secrets: { password: 'hunter2' }, firstFactors: [{ strategy: 'password' as const }] }, + }); + await renderWithClerk(); + const user = userEvent.setup(); + await openDialog(user, screen.getByRole('button', { name: 'Delete my account' }), { name: TITLE }); + await user.type(field(), 'Delete{Enter}'); + + const password = await screen.findByLabelText('Password'); + await waitFor(() => expect(screen.queryByText(TITLE)).toBeNull()); + await user.type(password, 'hunter2{Enter}'); + + expect(await screen.findByText('Your subscription is still active.')).toBeInTheDocument(); + expect(screen.getAllByRole('dialog')).toHaveLength(1); + expect(field()).toHaveValue('Delete'); + expect(field()).toHaveAttribute('aria-invalid', 'true'); + expect(action).toHaveBeenCalledTimes(2); }); }); diff --git a/packages/mosaic/src/blocks/destructive/__tests__/destructive.test.tsx b/packages/mosaic/src/blocks/destructive/__tests__/destructive.test.tsx deleted file mode 100644 index 5cfc11a1e73..00000000000 --- a/packages/mosaic/src/blocks/destructive/__tests__/destructive.test.tsx +++ /dev/null @@ -1,378 +0,0 @@ -import { createDeferredPromise } from '@clerk/shared/utils'; -import { act, render, screen, waitFor } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import { describe, expect, it, vi } from 'vitest'; - -import { Button } from '../../../components/button'; -import { Menu } from '../../../components/menu'; -import type { ReverificationController } from '../../../features/reverification'; -import { MosaicProvider } from '../../../mosaic-provider'; -import { SaveError } from '../../../utils/errors'; -import type { DestructiveControlledProps, DestructiveHandleProps } from '../destructive'; -import { Destructive } from '../destructive'; - -vi.mock('../../../features/reverification', () => ({ - Reverification: ({ status }: { status: string }) => {status}, -})); - -function renderBlock(overrides: Partial = {}) { - return render( - - - , - ); -} - -const confirmButton = () => screen.getByRole('button', { name: 'Delete account' }); -const startingReverification = { - status: 'loading' as const, - onCancel: vi.fn(), - visible: false, - reset: vi.fn(), -}; - -function readyReverification(status: 'ready' | 'retrying'): ReverificationController { - const view = { - step: 'password' as const, - value: '', - onValueChange: () => {}, - isPending: status === 'retrying', - onSubmit: () => {}, - onShowMethods: () => {}, - onShowHelp: () => {}, - onBack: () => {}, - onEmailSupport: () => {}, - onResend: () => {}, - canResend: false, - methods: [], - onSelectMethod: () => {}, - }; - return status === 'retrying' - ? { status, visible: true, reset: () => {}, ...view } - : { status, visible: true, reset: () => {}, ...view }; -} - -describe('Destructive', () => { - it('renders nothing until the caller opens it', () => { - renderBlock({ open: false }); - - expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); - }); - - it('focuses the confirmation field on open', async () => { - renderBlock(); - - await waitFor(() => expect(screen.getByRole('textbox')).toHaveFocus()); - }); - - it('asks to open from the trigger', async () => { - const onOpenChange = vi.fn(); - const user = userEvent.setup(); - renderBlock({ open: false, onOpenChange, trigger: }); - - await user.click(confirmButton()); - - expect(onOpenChange).toHaveBeenCalledWith(true, expect.anything()); - }); - - it('holds the action until the typed phrase matches', async () => { - const onDelete = vi.fn(); - const user = userEvent.setup(); - renderBlock({ onDelete }); - - expect(confirmButton()).toHaveAttribute('aria-disabled', 'true'); - - await user.type(screen.getByRole('textbox'), 'Delete accoun'); - expect(confirmButton()).toHaveAttribute('aria-disabled', 'true'); - - await user.type(screen.getByRole('textbox'), 't'); - expect(confirmButton()).not.toHaveAttribute('aria-disabled'); - - await user.click(confirmButton()); - expect(onDelete).toHaveBeenCalledOnce(); - }); - - it('submits on enter in the confirmation field, once the typed phrase matches', async () => { - const onDelete = vi.fn(); - const user = userEvent.setup(); - renderBlock({ onDelete }); - - await user.type(screen.getByRole('textbox'), 'Delete accoun{Enter}'); - expect(onDelete).not.toHaveBeenCalled(); - - await user.type(screen.getByRole('textbox'), 't{Enter}'); - expect(onDelete).toHaveBeenCalledOnce(); - }); - - it('asks to close from cancel', async () => { - const onOpenChange = vi.fn(); - const user = userEvent.setup(); - renderBlock({ onOpenChange }); - - await user.click(screen.getByRole('button', { name: 'Cancel' })); - - expect(onOpenChange).toHaveBeenCalledWith(false, expect.anything()); - }); - - it('clears the typed phrase once the caller closes it', async () => { - const user = userEvent.setup(); - const view = renderBlock(); - - await user.type(screen.getByRole('textbox'), 'Delete account'); - view.rerender( - - - , - ); - view.rerender( - - - , - ); - - expect(screen.getByRole('textbox')).toHaveValue(''); - }); - - it('marks the field invalid and explains a failed attempt', () => { - renderBlock({ errorMessage: 'Your subscription is still active.' }); - - expect(screen.getByText('Your subscription is still active.')).toBeInTheDocument(); - expect(screen.getByRole('textbox')).toHaveAttribute('aria-invalid', 'true'); - }); - - it('stays inert while the caller is deleting', async () => { - const onDelete = vi.fn(); - const user = userEvent.setup(); - renderBlock({ isDeleting: true, onDelete }); - - await user.type(screen.getByRole('textbox'), 'Delete account'); - - expect(screen.getByRole('textbox')).toBeDisabled(); - // Busy, not unavailable: the block leaves the pending affordance to `isPending` rather than - // disabling the action a second time. - expect(confirmButton()).toHaveAttribute('aria-busy', 'true'); - await user.click(confirmButton()); - expect(onDelete).not.toHaveBeenCalled(); - }); - - it('keeps the delete confirmation pending while verification is starting', () => { - renderBlock({ isDeleting: true, reverification: startingReverification }); - - expect(document.querySelector('.cl-flow-root')).toHaveAttribute('data-value', 'confirm'); - expect(screen.getByRole('textbox')).toBeDisabled(); - expect(confirmButton()).toHaveAttribute('aria-busy', 'true'); - expect(screen.queryByTestId('reverification')).not.toBeInTheDocument(); - }); - - it('shows reverification in the same dialog and card', () => { - renderBlock({ isDeleting: true, step: 'verify', reverification: readyReverification('ready') }); - - expect(screen.getAllByRole('dialog')).toHaveLength(1); - expect(document.querySelectorAll('.cl-card-root')).toHaveLength(1); - expect(document.querySelector('.cl-flow-root')).toHaveAttribute('data-value', 'verify'); - expect(screen.getByTestId('reverification')).toHaveTextContent('ready'); - expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); - }); - - it('keeps the retrying factor visible until the hook becomes inactive', () => { - const props = { - open: true, - onOpenChange: vi.fn(), - title: 'Delete account?', - description: 'All of your data will be permanently deleted.', - fieldLabel: 'Type “Delete account” below to continue', - confirmationValue: 'Delete account', - actionLabel: 'Delete account', - onDelete: vi.fn(), - }; - const { rerender } = render( - - - , - ); - expect(document.querySelector('.cl-flow-root')).toHaveAttribute('data-value', 'verify'); - expect(screen.getByTestId('reverification')).toHaveTextContent('retrying'); - - rerender( - - {} }} - /> - , - ); - expect(document.querySelector('.cl-flow-root')).toHaveAttribute('data-value', 'confirm'); - expect(screen.getByRole('textbox')).toBeInTheDocument(); - expect(screen.getByText('Delete failed.')).toBeInTheDocument(); - }); -}); - -function renderWithHandle(onDelete: DestructiveHandleProps['onDelete']) { - const handle = Destructive.createHandle(); - render( - - {name} will be permanently deleted.} - fieldLabel='Type the account name to continue' - confirmationValue={name => name} - actionLabel={name => `Delete ${name}`} - onDelete={onDelete} - /> - , - ); - act(() => handle.open('account')); - return handle; -} - -describe('Destructive with a handle', () => { - it('shares one dialog between menu items and deletes the confirmed payload', async () => { - const handle = Destructive.createHandle<{ id: string; name: string }>(); - const projects = [ - { id: 'project_1', name: 'Production' }, - { id: 'project_2', name: 'Staging' }, - ]; - const onDelete = vi.fn(() => Promise.resolve()); - const user = userEvent.setup(); - render( - - {projects.map(project => ( - - Actions for {project.name}} /> - - handle.open(project)} - > - Delete - - - - ))} - `Delete ${project.name}?`} - description={project => `${project.name} will be permanently deleted.`} - fieldLabel={project => `Type ${project.name} to continue`} - confirmationValue={project => project.name} - actionLabel='Delete project' - onDelete={onDelete} - /> - , - ); - - expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); - const firstTrigger = screen.getByRole('button', { name: 'Actions for Production' }); - await user.click(firstTrigger); - await user.click(screen.getByRole('menuitem', { name: 'Delete' })); - expect(screen.getAllByRole('dialog')).toHaveLength(1); - expect(screen.getByRole('dialog')).toHaveAccessibleName('Delete Production?'); - await user.type(screen.getByRole('textbox'), 'Production'); - await user.click(screen.getByRole('button', { name: 'Cancel' })); - await waitFor(() => expect(firstTrigger).toHaveFocus()); - - const secondTrigger = screen.getByRole('button', { name: 'Actions for Staging' }); - secondTrigger.focus(); - await user.keyboard('{Enter}{Enter}'); - expect(screen.getAllByRole('dialog')).toHaveLength(1); - expect(screen.getByRole('dialog')).toHaveAccessibleName('Delete Staging?'); - expect(screen.getByRole('dialog')).toHaveAccessibleDescription('Staging will be permanently deleted.'); - expect(screen.getByRole('textbox', { name: 'Type Staging to continue' })).toHaveValue(''); - await user.type(screen.getByRole('textbox'), 'Stagin{Enter}'); - expect(onDelete).not.toHaveBeenCalled(); - await user.type(screen.getByRole('textbox'), 'g{Enter}'); - expect(onDelete).toHaveBeenCalledExactlyOnceWith({ id: 'project_2', name: 'Staging' }); - await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); - expect(secondTrigger).toHaveFocus(); - }); - - it('keeps the dialog open and prevents duplicate submissions while deletion is pending', async () => { - const deletion = createDeferredPromise(); - const onDelete = vi.fn(async () => { - await deletion.promise; - }); - const user = userEvent.setup(); - const handle = renderWithHandle(onDelete); - - await user.type(screen.getByRole('textbox'), 'account'); - await user.click(confirmButton()); - - expect(screen.getByRole('textbox')).toBeDisabled(); - expect(confirmButton()).toHaveAttribute('aria-busy', 'true'); - await user.click(confirmButton()); - await user.click(screen.getByRole('button', { name: 'Cancel' })); - await user.keyboard('{Escape}'); - act(() => handle.close()); - expect(screen.getByRole('dialog')).toBeInTheDocument(); - expect(onDelete).toHaveBeenCalledExactlyOnceWith('account'); - - await act(async () => { - deletion.resolve(); - await deletion.promise; - }); - await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); - }); - - it('shows a deletion failure, clears it on reopening, and allows retrying', async () => { - const onDelete = vi - .fn['onDelete']>() - .mockRejectedValueOnce( - new SaveError({ global: { code: 'account_has_projects', message: 'The account still has active projects.' } }), - ) - .mockResolvedValueOnce(undefined); - const user = userEvent.setup(); - const handle = renderWithHandle(onDelete); - - await user.type(screen.getByRole('textbox'), 'account{Enter}'); - expect(await screen.findByText('The account still has active projects.')).toBeInTheDocument(); - expect(screen.getByRole('textbox')).toHaveAttribute('aria-invalid', 'true'); - expect(screen.getByRole('textbox')).toHaveValue('account'); - expect(confirmButton()).not.toHaveAttribute('aria-busy', 'true'); - - act(() => handle.close()); - await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); - act(() => handle.open('account')); - expect(screen.getByRole('textbox')).toHaveValue(''); - expect(screen.queryByText('The account still has active projects.')).not.toBeInTheDocument(); - - await user.type(screen.getByRole('textbox'), 'account{Enter}'); - await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); - expect(onDelete).toHaveBeenCalledTimes(2); - }); -}); diff --git a/packages/mosaic/src/blocks/destructive/destructive.controller.ts b/packages/mosaic/src/blocks/destructive/destructive.controller.ts deleted file mode 100644 index 526639325fe..00000000000 --- a/packages/mosaic/src/blocks/destructive/destructive.controller.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { isReverificationCancelledError } from '@clerk/shared/error'; -import { useState } from 'react'; - -import type { ReverificationController } from '../../features/reverification'; -import type { ErrorDescription } from '../../localization'; -import { useErrorText } from '../../localization'; -import { toLocalizableError } from '../../utils/errors'; -import type { DestructiveControlledProps } from './destructive'; - -export type DestructiveController = Pick< - DestructiveControlledProps, - 'open' | 'isDeleting' | 'errorMessage' | 'onOpenChange' | 'reverification' | 'step' -> & { - onDelete: () => Promise; - openDestructiveDialog: () => void; -}; - -type DestructiveState = - | { status: 'closed' } - | { status: 'open-needs-confirmation' } - | { status: 'open-pending' } - | { status: 'open-error'; error: ErrorDescription }; - -/** Optional controller for use with Destructive block */ -export function useDestructiveController({ - onDelete, - reverification, - errorFallback, -}: { - onDelete: () => Promise; - reverification?: ReverificationController; - /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ - errorFallback?: string; -}): DestructiveController { - const [destructiveState, setDestructiveState] = useState({ status: 'closed' }); - const errorText = useErrorText(); - const { status } = destructiveState; - - const openDestructiveDialog = () => { - if (status === 'closed') { - setDestructiveState({ status: 'open-needs-confirmation' }); - } - }; - - const isDeleting = status === 'open-pending'; - const step = isDeleting && reverification?.visible ? 'verify' : 'confirm'; - - return { - open: status !== 'closed', - isDeleting, - errorMessage: - destructiveState.status === 'open-error' ? errorText(destructiveState.error, errorFallback) : undefined, - reverification, - step, - openDestructiveDialog, - onDelete: async () => { - if (status === 'open-needs-confirmation' || status === 'open-error') { - reverification?.reset(); - setDestructiveState({ status: 'open-pending' }); - try { - await onDelete(); - setDestructiveState({ status: 'closed' }); - } catch (error: unknown) { - if (isReverificationCancelledError(error)) { - setDestructiveState({ status: 'closed' }); - return; - } - setDestructiveState({ status: 'open-error', error: toLocalizableError(error) }); - } - } - }, - onOpenChange: nextIsOpen => { - const canCancel = Boolean(reverification?.onCancel); - if (!nextIsOpen && isDeleting && !canCancel) { - return; - } - - if (nextIsOpen) { - setDestructiveState({ status: 'open-needs-confirmation' }); - } else { - reverification?.onCancel?.(); - setDestructiveState({ status: 'closed' }); - } - }, - }; -} diff --git a/packages/mosaic/src/blocks/destructive/destructive.tsx b/packages/mosaic/src/blocks/destructive/destructive.tsx index 2a17818eaa7..760a22672a9 100644 --- a/packages/mosaic/src/blocks/destructive/destructive.tsx +++ b/packages/mosaic/src/blocks/destructive/destructive.tsx @@ -1,79 +1,45 @@ -import type { FormEvent, ReactNode } from 'react'; +import type { FormEvent, RefObject } from 'react'; import { useEffect, useId, useRef, useState } from 'react'; import { Button, SubmitButton } from '../../components/button'; import { Card } from '../../components/card'; -import type { DialogFocusTarget, DialogHandle, DialogRootProps, DialogTriggerProps } from '../../components/dialog'; import { Dialog } from '../../components/dialog'; import { Field } from '../../components/field'; -import { Flow } from '../../components/flow'; import { Input } from '../../components/input'; -import { Reverification, type ReverificationController } from '../../features/reverification'; import { type FromPayload, resolveFromPayload as resolve } from '../../utils/resolve-from-payload'; -import { useConfirmationController } from '../confirmation/confirmation.controller'; +import type { ConfirmBlockProps, ConfirmStepProps } from '../confirm/confirm-shell'; +import { ConfirmShell } from '../confirm/confirm-shell'; -export interface DestructiveControlledProps { - finalFocus?: DialogFocusTarget; - /** Whether the dialog is open */ - open: boolean; - /** Callback when open state changes */ - onOpenChange: NonNullable; - /** Element that opens the dialog */ - trigger?: DialogTriggerProps['render']; - /** Dialog heading */ - title: string; - /** What the action destroys */ - description: ReactNode; - /** Label above the confirmation input */ +export interface DestructiveProps extends ConfirmBlockProps { + /** Label above the confirmation input, or a function of the payload */ + fieldLabel: FromPayload; + /** Phrase the user must type to confirm, or a function of the payload. Also the input's placeholder */ + confirmationValue: FromPayload; +} + +interface DestructiveStepProps extends ConfirmStepProps { fieldLabel: string; - /** Phrase the user must type to confirm. Also the input's placeholder */ confirmationValue: string; - /** Text of the delete button */ - actionLabel: string; - /** Text of the cancel button (default: "Cancel") */ - cancelLabel?: string; - /** Callback when delete is confirmed, by button or by Enter */ - onDelete: () => Promise | void; - /** Whether the delete action is in progress */ - isDeleting?: boolean; - /** Error message to display if the delete action fails */ - errorMessage?: string; - reverification?: ReverificationController; - step?: 'confirm' | 'verify'; + typedValue: string; + onTypedValueChange: (value: string) => void; + inputRef: RefObject; } -type DestructiveCardProps = Omit & { - onClose?: () => void; -}; - -function DestructiveCard({ - open, - finalFocus, +function DestructiveStep({ title, description, - fieldLabel, - confirmationValue, actionLabel, - cancelLabel = 'Cancel', - onDelete, - isDeleting = false, + cancelLabel, + isPending, errorMessage, - reverification, - step = 'confirm', - onClose, -}: DestructiveCardProps) { + confirm, + fieldLabel, + confirmationValue, + typedValue, + onTypedValueChange, + inputRef, +}: DestructiveStepProps) { const formId = useId(); - const [typedValue, setTypedValue] = useState(''); - - // The caller may close the dialog without going through the trigger or Cancel, so the - // field is cleared on close rather than in a handler. - useEffect(() => { - if (!open) { - setTypedValue(''); - } - }, [open]); - - const inputRef = useRef(null); const isConfirmed = typedValue === confirmationValue; // The action sits in the footer, outside the form, so `form={formId}` associates the two. // That is what makes Enter in the field submit. Both guards are re-checked here because @@ -81,13 +47,12 @@ function DestructiveCard({ // `aria-disabled`, and `isPending` only cancels the press. const handleSubmit = (event: FormEvent) => { event.preventDefault(); - if (isConfirmed && !isDeleting) { - // Errors are meant to be handled on the outside, so we swallow them here - onDelete()?.catch(() => {}); + if (isConfirmed && !isPending) { + confirm(); } }; - const confirmation = ( + return ( <> {title} @@ -107,8 +72,8 @@ function DestructiveCard({ data-1p-ignore placeholder={confirmationValue} value={typedValue} - disabled={isDeleting} - onChange={event => setTypedValue(event.target.value)} + disabled={isPending} + onChange={event => onTypedValueChange(event.target.value)} /> {errorMessage} @@ -131,7 +96,7 @@ function DestructiveCard({ form={formId} fullWidth color='negative' - isPending={isDeleting} + isPending={isPending} disabled={!isConfirmed} focusableWhenDisabled > @@ -140,158 +105,54 @@ function DestructiveCard({ ); - - const content = reverification ? ( - - {() => ( - <> - {confirmation} - - - - - )} - - ) : ( - confirmation - ); - - return ( - - - {content} - - - ); -} - -function ControlledDestructive({ open, onOpenChange, trigger, ...props }: DestructiveControlledProps) { - return ( - - {trigger ? : null} - onOpenChange(false, { trigger: null, triggerId: null, event: undefined })} - {...props} - /> - - ); -} - -export interface DestructiveHandle extends DialogHandle { - open(payload: Payload): void; } -function createHandle(): DestructiveHandle { - return Dialog.createHandle(); -} - -export interface DestructiveHandleProps { - handle: DestructiveHandle; - finalFocus?: DialogFocusTarget; - title: FromPayload; - description: FromPayload; - fieldLabel: FromPayload; - confirmationValue: FromPayload; - actionLabel: FromPayload; - cancelLabel?: string; - onDelete: (payload: Payload) => Promise | void; - /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ - errorFallback?: string; -} - -function HandleDestructive({ - handle, - finalFocus, - title, - description, - fieldLabel, - confirmationValue, - actionLabel, - cancelLabel, - onDelete, - errorFallback, -}: DestructiveHandleProps) { - const controller = useConfirmationController({ errorFallback }); - - return ( - - {({ payload }) => - payload === undefined ? null : ( - - controller.onConfirm(async () => { - await onDelete(payload); - }) - } - isDeleting={controller.isConfirming} - errorMessage={controller.errorMessage} - /> - ) - } - - ); -} - -export type DestructiveProps = DestructiveControlledProps | DestructiveHandleProps; - /** * Type-to-confirm dialog for an action that cannot be undone. The delete button stays inert * until the typed phrase matches `confirmationValue`. * - * Controlled: the caller owns `open`, `isDeleting`, and `errorMessage`. With a `handle`, - * the block owns all three, using the same controller as `Confirmation`. Mount it once and - * call `handle.open(payload)` from menu items; `onDelete` resolves to close or rejects to - * show an error. In both forms the block holds the typed phrase. + * Driven by a `useConfirmController`, the same way as `Confirmation`. The block holds the typed + * phrase and clears it when the dialog closes. * * @example + * const deleteAccount = useConfirmController({ onConfirm: deleteAccountWithReverification, reverification }); + * * send({ type: open ? 'OPEN' : 'CANCEL' })} + * controller={deleteAccount} * trigger={} * title='Delete account?' * description='All of your data will be permanently deleted.' * fieldLabel='Type “Delete account” below to continue' * confirmationValue='Delete account' * actionLabel='Delete account' - * onDelete={() => send({ type: 'CONFIRM' })} - * isDeleting={snapshot.value === 'deleting'} - * errorMessage={snapshot.context.errorMessage} * /> */ -export function Destructive(props: DestructiveProps) { - return 'handle' in props ? {...props} /> : ; -} +export function Destructive({ fieldLabel, confirmationValue, ...props }: DestructiveProps) { + const inputRef = useRef(null); + const [typedValue, setTypedValue] = useState(''); + const { isOpen } = props.controller; -Destructive.createHandle = createHandle; + useEffect(() => { + if (!isOpen) { + setTypedValue(''); + } + }, [isOpen]); + + return ( + + {step => ( + + )} + + ); +} diff --git a/packages/mosaic/src/blocks/destructive/index.ts b/packages/mosaic/src/blocks/destructive/index.ts index 8beba8a0bfa..39b75d99817 100644 --- a/packages/mosaic/src/blocks/destructive/index.ts +++ b/packages/mosaic/src/blocks/destructive/index.ts @@ -1,7 +1,2 @@ export { Destructive } from './destructive'; -export type { - DestructiveControlledProps, - DestructiveHandle, - DestructiveHandleProps, - DestructiveProps, -} from './destructive'; +export type { DestructiveProps } from './destructive'; diff --git a/packages/mosaic/src/utils/__tests__/errors.test.tsx b/packages/mosaic/src/utils/__tests__/errors.test.tsx index 9aea82a8748..e3b1289780c 100644 --- a/packages/mosaic/src/utils/__tests__/errors.test.tsx +++ b/packages/mosaic/src/utils/__tests__/errors.test.tsx @@ -4,7 +4,7 @@ import type { ReactNode } from 'react'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { isLocalizableError, MosaicLocalizationProvider, resolveLocalization, useErrorText } from '../../localization'; -import { SaveError, toLocalizableError } from '../errors'; +import { save, SaveError, toLocalizableError } from '../errors'; function errorText() { const wrapper = ({ children }: { children: ReactNode }) => ( @@ -17,6 +17,29 @@ function silenceConsole() { return vi.spyOn(console, 'error').mockImplementation(() => {}); } +describe('save', () => { + it('wraps a Clerk API error the user can act on', async () => { + const cause = new ClerkAPIResponseError('Forbidden', { + status: 403, + data: [{ code: 'action_blocked', message: 'Short', long_message: 'Long' }], + }); + await expect(save(() => Promise.reject(cause))).rejects.toBeInstanceOf(SaveError); + }); + + it('lets a reverification hint through untouched, so the flow can start a challenge', async () => { + const cause = new ClerkAPIResponseError('Forbidden', { + status: 403, + data: [{ code: 'session_reverification_required', message: 'Reverification required' }], + }); + await expect(save(() => Promise.reject(cause))).rejects.toBe(cause); + }); + + it('lets a cancelled reverification through untouched, so the dialog can close quietly', async () => { + const cause = new ClerkRuntimeError('cancelled', { code: 'reverification_cancelled' }); + await expect(save(() => Promise.reject(cause))).rejects.toBe(cause); + }); +}); + describe('toLocalizableError', () => { afterEach(() => { vi.restoreAllMocks(); diff --git a/packages/mosaic/src/utils/errors.ts b/packages/mosaic/src/utils/errors.ts index ccefd77ddbd..79dc46b9285 100644 --- a/packages/mosaic/src/utils/errors.ts +++ b/packages/mosaic/src/utils/errors.ts @@ -1,4 +1,4 @@ -import { isClerkAPIResponseError, isClerkRuntimeError } from '@clerk/shared/error'; +import { isClerkAPIResponseError, isClerkRuntimeError, isReverificationCancelledError } from '@clerk/shared/error'; import type { ClerkAPIError } from '@clerk/shared/types'; import { snakeToCamel } from '@clerk/shared/underscore'; @@ -11,6 +11,19 @@ export interface FormError { export const UNEXPECTED_ERROR: LocalizableError = { code: 'generic' }; +// TODO: This is not exported from shared currently. Keeping it here for now to not +// trigger a shared changeset from this PR and also to figure out in what form +// we want to export it. +const REVERIFICATION_REQUIRED_CODE = 'session_reverification_required'; + +/** Whether a failure belongs to the reverification flow rather than to the user: a required step-up, or a cancelled one. */ +function isReverificationError(cause: unknown): boolean { + if (isReverificationCancelledError(cause)) { + return true; + } + return isClerkAPIResponseError(cause) && cause.errors.some(error => error.code === REVERIFICATION_REQUIRED_CODE); +} + /** What a save rejects with when the failure is the user's to fix; anything else propagates. */ export class SaveError extends Error { readonly formError: FormError; @@ -85,6 +98,13 @@ export async function save( try { await run(); } catch (cause) { + // We rethrow reverification errors so it's possible to call save() both inside and after a + // useReverification wrap has happened. Normally useReverification() would get called first, + // catch the error and save would be called after, but there are times when it's convenient + // to call save(} inside. + if (isReverificationError(cause)) { + throw cause; + } const formError = toClerkFormError(cause, fields, params); if (!formError) { throw cause; From 245779f97a4b3d3e9d75cc3fdbd4a8df8d513f4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Mon, 12 Oct 2026 21:46:02 +0200 Subject: [PATCH 02/19] Update docs and swingset --- packages/mosaic/AGENTS.md | 6 +- packages/mosaic/ARCHITECTURE.md | 22 ++-- packages/mosaic/docs/controllers.md | 97 ++++++++++++--- packages/mosaic/docs/views.md | 80 +++++------- .../src/blocks/confirm/confirm-shell.tsx | 2 +- .../app/(clerk)/live/reverification/page.tsx | 12 +- .../swingset/src/stories/confirmation.mdx | 117 ++++++------------ .../src/stories/confirmation.stories.tsx | 104 ++++++---------- packages/swingset/src/stories/destructive.mdx | 117 ++++-------------- .../src/stories/destructive.stories.tsx | 53 ++++---- 10 files changed, 260 insertions(+), 350 deletions(-) diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index 2738946e767..0987997508a 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -35,9 +35,9 @@ Most problems a feature hits already have a shared answer. Find the row for your | An async action from a button or row: pending, an error, one lock | `usePendingAction` (`src/hooks/`) | | A form: values, validation, submit, field errors | `useForm` (`src/components/form/`), with `validateAsync` for a server-side check | | A model save whose Clerk errors should land on form fields | `save` in the model (`src/utils/errors.ts`), or reject with `FormSubmitError` | -| A confirm step before an action | `Confirmation` + `useConfirmationController` (`src/blocks/confirmation/`) | -| A delete that needs the user to type a phrase | `Destructive` + `useDestructiveController` (`src/blocks/destructive/`) | -| An action that may need step-up verification | `useReverificationFlow` (`src/features/reverification/`), passed to `useDestructiveController` | +| A confirm step before an action | `useConfirmController` (`src/blocks/confirm/`) with `Confirmation` (`src/blocks/confirmation/`) | +| A delete that needs the user to type a phrase | `useConfirmController` (`src/blocks/confirm/`) with `Destructive` (`src/blocks/destructive/`) | +| An action that may need step-up verification | `useReverificationFlow` (`src/features/reverification/`), passed to `useConfirmController` | | Showing any error to the user | One of the owners above. They call `toLocalizableError` and resolve copy with `useErrorText` | | Moving focus after a list row is removed | `useListRemovalFocus` (`src/hooks/`) | | A debounced, abortable async check as the user types | `useDebouncedAsync` (`src/hooks/`) | diff --git a/packages/mosaic/ARCHITECTURE.md b/packages/mosaic/ARCHITECTURE.md index 41a9282434c..858fdb1dc9f 100644 --- a/packages/mosaic/ARCHITECTURE.md +++ b/packages/mosaic/ARCHITECTURE.md @@ -490,17 +490,17 @@ export function UserButtonView({ open, onOpenChange, pendingKey, onSignOutAll, . A **block** is a view fragment that owns one piece of state nothing outside it can use, and takes the rest as props. `blocks/destructive` is the example: it holds the -half-typed confirmation phrase and compares it, while `open`, `isDeleting`, and -`errorMessage` come from the controller, because those are what decide whether the -dialog closes or explains itself. `blocks/confirmation` is the same shape for a -confirm step with no typed phrase. - -Each block ships its own controller (`useDestructiveController`, -`useConfirmationController`) that owns the pending and error state, so a feature -passes it the effect and spreads the result onto the block rather than wiring that -state by hand. Each also has a handle form (`DestructiveHandle`, and its -`Confirmation` equivalent) for one dialog shared by several triggers, where the -clicked row arrives as a payload. +half-typed confirmation phrase and compares it, while open, pending and the error +come from its controller, because those are what decide whether the dialog closes +or explains itself. `blocks/confirmation` is the same shape for a confirm step +with no typed phrase. + +Both blocks are driven by one controller, `useConfirmController` (`blocks/confirm/`), +created in the wrapper or controller layer and handed to the block as `controller`. +It owns the dialog's lifecycle and the reverification step, and it carries a +payload: a dialog shared by a list is opened from any row with `open(payload)`, the +copy is derived from that payload, and the action receives it back. A dialog that +sits next to its button takes the button as `trigger` instead. ### Testing a flow diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index c64f2589c4b..eb85e76aeca 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -23,14 +23,14 @@ A controller is not a machine wrapper. It is the layer that owns the interaction, and it holds that state in whichever tool the interaction's complexity calls for: -| The interaction… | Hold it in | -| ----------------------------------------------------------------------------- | ------------------------------------------------------------------------- | -| Is a boolean or a controlled value, no async, nothing else depends on it | `useState` | -| Has an async lifecycle, or two values that must change together | A machine, same file | -| Has a coordinated async core plus some UI-only flags beside it | Both — machine for the core | -| Is one async action from a button or a row, needing only pending and an error | `usePendingAction` | -| Is a form | `useForm` | -| Is a confirm step before an action | `useConfirmationController` / `useDestructiveController` with their block | +| The interaction… | Hold it in | +| ----------------------------------------------------------------------------- | ----------------------------------------------------------- | +| Is a boolean or a controlled value, no async, nothing else depends on it | `useState` | +| Has an async lifecycle, or two values that must change together | A machine, same file | +| Has a coordinated async core plus some UI-only flags beside it | Both — machine for the core | +| Is one async action from a button or a row, needing only pending and an error | `usePendingAction` | +| Is a form | `useForm` | +| Is a confirm step before an action | `useConfirmController` with `Confirmation` or `Destructive` | Check `packages/mosaic/AGENTS.md` → "Reuse before you write" before holding any of this by hand. If nothing there fits, ask the user before adding a new pattern. @@ -56,17 +56,33 @@ focus. To hold pending through an OAuth redirect, await a delay inside the action (`user-profile-connected-accounts-section.controller.ts`). It is not for: - a form → `useForm` -- a confirm step → `Confirmation` / `Destructive` and their controllers +- a confirm step → `useConfirmController` with `Confirmation` / `Destructive` - an action whose result drives other state → a machine - a check that reruns as the user types → `useDebouncedAsync` (`hooks/`), or a form field's `validateAsync` -`useConfirmationController` earns a machine: the confirm is async, and a failure -must land back on the confirming step with a reason. `useUserButtonController` -earns one because `open` and `pendingKey` constrain each other and dismissing -must not abandon an in-flight invoke. `useDestructiveController` is the middle -ground: its states never overlap, so one `useState` holding a status union is -enough. +`useConfirmController` earns a machine: the confirm is async, a failure must land +back on the confirming step with a reason, and a cancelled reverification must +close without one. `useUserButtonController` earns one because `open` and +`pendingKey` constrain each other and dismissing must not abandon an in-flight +invoke. + +`useConfirmController({ onConfirm, reverification, errorFallback })` owns one +confirm dialog: open, pending, error, and the verify step a reverified action +swaps in. It is generic over a payload. Create it in the wrapper, pass it to the +view, and the view hands it to the block as `controller` and opens it from a row +with `open(payload)`; the copy props read the payload and `onConfirm` receives it. +A dialog next to its button has no payload and takes the button as `trigger`: + +```ts +const [revoke, reverification] = useReverificationFlow((id: string) => model.revoke(id)); +const removalFocus = useListRemovalFocus({ ids, onRemove: revoke, fallback }); +const remove = useConfirmController({ + onConfirm: invitation => removalFocus.remove(invitation.id), + reverification, + errorFallback: m.revokeError, +}); +``` A single open boolean earns nothing. A two-state machine with one event is a boolean spelled long. `useUserProfileEditUsernameController` holds `isOpen` in @@ -201,6 +217,53 @@ When introducing lazy loading or Suspense, consider a boundary around dialog content that leaves the trigger and parent available. Choose that boundary from the feature's loading behavior; it does not require restructuring every dialog. +## Removing a row takes focus with it + +A dialog returns focus to its trigger on close. When the action removes the row +that trigger sits on — signing a device out, removing an email — the trigger +unmounts and focus falls to ``. Nothing catches it: the menu item that +opened the dialog unmounted with the menu, and a dialog mounted at the section +rather than inside the menu has no floating-tree ancestor to walk back to. A +keyboard user loses their place mid-list and a screen reader announces nothing. + +Hand focus to a surviving element with `useListRemovalFocus` (`hooks/`). It sits +in the controller or wrapper, e.g. between the confirm controller and the model call: +`remove` records which row went, and `finalFocus` picks the row that took its +place, the last row when it was the last, or `fallback()` once the list is empty. +The view receives `registerTrigger` and `finalFocus` as props and passes +`finalFocus` to the block, which resolves it when the dialog closes, after the +row has gone: + +```ts +const sectionRef = useRef(null); +const removalFocus = useListRemovalFocus({ + ids: methods.map(method => method.id), + onRemove, + fallback: () => sectionRef.current?.querySelector('button:not([disabled])') ?? sectionRef.current, +}); +const remove = useConfirmController({ onConfirm: method => removalFocus.remove(method.id) }); + +return { sectionRef, remove, registerTrigger: removalFocus.registerTrigger, finalFocus: removalFocus.finalFocus }; +``` + +```tsx +// each row's trigger +triggerRef={registerTrigger(method.id)} + +// the dialog + +``` + +A cancelled or failed removal keeps its own trigger, so `finalFocus` returns +`null` and the dialog's default (the trigger) applies. + +Prefer the row that took the removed one's place, the last row when it was the +last, and a control that outlives the list once it is empty. + +Test the removal, not just the cancel: `toHaveFocus()` on the row that should +have caught it. A suite that only asserts focus after cancelling passes while +every successful removal drops focus on the floor. + ## Rules - **No Clerk imports.** If a controller needs a Clerk fact, the model supplies it @@ -223,8 +286,8 @@ the feature's loading behavior; it does not require restructuring every dialog. One path, so every error a user sees goes through the `errors.*` catalog: - **Owners map, nothing else does.** A failure is owned by one of - `usePendingAction` (button and row actions), `useConfirmationController`, - `useDestructiveController`, or `useForm` (forms). The first three call + `usePendingAction` (button and row actions), `useConfirmController` (confirm + dialogs), or `useForm` (forms). The first two call `toLocalizableError(cause)` from `utils/errors.ts`. Use one of them instead of catching in a feature. A controller with its own machine maps in the machine's `onError` (`user-profile-rename-passkey.controller.ts`). diff --git a/packages/mosaic/docs/views.md b/packages/mosaic/docs/views.md index 9603b61bf07..9b58f75ec1e 100644 --- a/packages/mosaic/docs/views.md +++ b/packages/mosaic/docs/views.md @@ -67,29 +67,37 @@ use, and takes the rest as props. There are two: and compares it. - `blocks/confirmation` (`Confirmation`) is a confirm step with no typed phrase. -`open`, the pending flag and `errorMessage` come from the block's own controller -(`useDestructiveController`, `useConfirmationController`), because those decide -whether the dialog closes or explains itself. Spread the controller onto the -block rather than wiring that state by hand: +Both take a `controller` from `useConfirmController` (`blocks/confirm/`), which +decides whether the dialog closes or explains itself. The hook runs in the +wrapper or controller, never in the view; the view receives the controller as a +prop. A dialog next to its button takes the button as `trigger`: ```tsx -const destructive = useDestructiveController({ onDelete, reverification }); - {m.actionLabel}} title={m.dialogTitle} confirmationValue={m.fieldPlaceholder} -/>; +/> ``` -When one dialog serves every row in a list, use the handle form: create a handle -with `Confirmation.createHandle()` (or `Destructive.createHandle`), pass it -to each row's trigger and to the block's `handle` prop, and the clicked row -arrives as the payload of `onConfirm` / `onDelete`. In this form the block runs -its own controller, so pass the effect and `errorFallback` straight to it. -`user-profile-mfa-section.view.tsx` and `api-keys-table.view.tsx` are worked -examples. +When one dialog serves every row in a list, the controller carries the row as +its payload. Mount the block once after the list, open it from a row with +`controller.open(row)`, and derive the copy from the payload. `onConfirm` in the +wrapper receives the same row. See "Removing a row takes focus with it" in +`controllers.md` for the focus hand-off that comes with a removal: + +```tsx + remove.open(row) }]} /> + + fill(m.removeTitle, { name: member.name })} + description={m.removeDescription} + actionLabel={m.remove} + finalFocus={finalFocus} +/> +``` ## Pure derivation belongs beside the view, not in it @@ -99,43 +107,11 @@ rearranges a list, are decisions with no React in them. They live in ## Removing a row takes focus with it -A dialog returns focus to its trigger on close. When the action removes the row -that trigger sits on — signing a device out, removing an email — the trigger -unmounts and focus falls to ``. Nothing catches it: the menu item that -opened the dialog unmounted with the menu, and a dialog mounted at the section -rather than inside the menu has no floating-tree ancestor to walk back to. A -keyboard user loses their place mid-list and a screen reader announces nothing. - -Hand focus to a surviving element with `useListRemovalFocus` (`hooks/`). It -records which row was removed and, through `finalFocus`, picks the row that took -its place, the last row when it was the last, or `fallback()` once the list is -empty. `finalFocus` on `Dialog.Popup` and on the `Confirmation` block takes a -function resolved when the dialog closes, after the row has gone: - -```tsx -const removalFocus = useListRemovalFocus({ - ids: methods.map(method => method.id), - onRemove, - fallback: () => section.current?.querySelector('button:not([disabled])') ?? section.current, -}); - -// each row's trigger -triggerRef={removalFocus.registerTrigger(method.id)} - -// the dialog -finalFocus={removalFocus.finalFocus} -onConfirm={method => removalFocus.remove(method.id)} -``` - -A cancelled or failed removal keeps its own trigger, so `finalFocus` returns -`null` and the dialog's default (the trigger) applies. - -Prefer the row that took the removed one's place, the last row when it was the -last, and a control that outlives the list once it is empty. - -Test the removal, not just the cancel: `toHaveFocus()` on the row that should -have caught it. A suite that only asserts focus after cancelling passes while -every successful removal drops focus on the floor. +When a confirmed action removes the row its trigger sits on, focus has to be +handed to a surviving element. The bookkeeping lives in the controller, with +`useListRemovalFocus`; see `controllers.md`. The view's part is to register each +row's trigger with the `registerTrigger` it is given and pass `finalFocus` to the +block. ## Testing diff --git a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx index 2699d02df9e..8023c17d98e 100644 --- a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx +++ b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx @@ -136,7 +136,7 @@ export function ConfirmShell({ // Because it's enforced in the controller that this can only open via a function with the // same signature/Payload as we expect, it's safe to typecast here. It would be very hard // to convince TS of this properly, essentially by copying the Payload into the controller, - // so we take the easy way in this case. This only hold while the controller upholds the + // so we take the easy way in this case. This only holds while the controller upholds the // guarantee. // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- See above payload={payload as Payload} diff --git a/packages/swingset/src/app/(clerk)/live/reverification/page.tsx b/packages/swingset/src/app/(clerk)/live/reverification/page.tsx index a896b4bff6b..86b0f8d1db9 100644 --- a/packages/swingset/src/app/(clerk)/live/reverification/page.tsx +++ b/packages/swingset/src/app/(clerk)/live/reverification/page.tsx @@ -1,7 +1,7 @@ 'use client'; +import { useConfirmController } from '@clerk/mosaic/blocks/confirm'; import { Destructive } from '@clerk/mosaic/blocks/destructive'; -import { useDestructiveController } from '@clerk/mosaic/blocks/destructive/destructive.controller'; import { Button } from '@clerk/mosaic/components/button'; import { Card } from '@clerk/mosaic/components/card'; import { Dialog } from '@clerk/mosaic/components/dialog'; @@ -269,8 +269,8 @@ function DialogHarness() { function DestructiveHarness() { const [successMessage, setSuccessMessage] = useState(null); const [deleteAccount, reverification] = useReverificationFlow(() => mockDelete(true)); - const destructive = useDestructiveController({ - onDelete: async () => { + const destructive = useConfirmController({ + onConfirm: async () => { setSuccessMessage(null); await resetMockDelete(); try { @@ -291,16 +291,16 @@ function DestructiveHarness() { color='negative' onClick={() => { setSuccessMessage(null); - destructive.openDestructiveDialog(); + destructive.open(); }} - disabled={destructive.open} + disabled={destructive.isOpen} > Delete account {successMessage ?

{successMessage}

: null} (); - -const handleConfirm = async () => { - setIsConfirming(true); - setErrorMessage(undefined); - try { - await removeConnectedAccount(); - setOpen(false); - } catch { - setErrorMessage('Google could not be removed. Please try again.'); - } finally { - setIsConfirming(false); - } -}; + +const remove = useConfirmController({ onConfirm: removeConnectedAccount }); Remove} title='Remove connected account' description='Google will be removed from this account. You will no longer be able to use this connected account and any dependent features will no longer work.' actionLabel='Remove' - onConfirm={() => void handleConfirm()} - isConfirming={isConfirming} - errorMessage={errorMessage} />; ``` ## Failure -A failed attempt leaves the dialog up. Pass the sentence the user should read as `errorMessage`, and clear it when the next attempt starts. The message renders as a banner between the description and the actions. +A failed attempt leaves the dialog up. A Clerk error is localized by its code. Anything else renders as the controller's `errorFallback`, or the generic error when none is given. The message renders as a banner between the description and the actions, and clears when the dialog closes. (); +const remove = useConfirmController({ onConfirm: member => api.removeMember(member.id) }); - removeMember.open(member)}>Remove + remove.open(member)}>Remove <>{member.name} will be removed from the organization.} actionLabel='Remove' - onConfirm={member => api.removeMember(member.id)} /> ``` -Opened from a menu item, focus returns to that menu's trigger when the dialog closes. +Opened from a menu item, focus returns to that menu's trigger when the dialog closes. If the action removes the trigger, pass `finalFocus` an element that remains on the page. -## Props +## Reverification -Controlled: - -| Prop | Type | Default | Description | -| -------------- | ------------------------- | ------------ | ------------------------------------------------------------------------------------------ | -| `open` | `boolean` | — (required) | Whether the confirmation is showing. Controlled, the way any dialog is. | -| `onOpenChange` | `(open: boolean) => void` | — (required) | Asks to open or close. Fired by the trigger, Escape, and the `Dialog.Close` Cancel action. | -| `trigger` | `ReactNode` | — | The button that asks to open the dialog. | -| `title` | `string` | — (required) | Names what is about to happen. | -| `description` | `ReactNode` | — (required) | Spells out what it means. Takes markup, for a name to emphasise. | -| `actionLabel` | `string` | — (required) | The confirming button's label. | -| `cancelLabel` | `string` | `'Cancel'` | The cancel button's label. | -| `color` | `'negative' \| 'primary'` | `'negative'` | The weight the confirming button carries. | -| `finalFocus` | `DialogFocusTarget` | the trigger | Where focus returns on close, for a removal that unmounts the trigger. | -| `onConfirm` | `() => void` | — (required) | Asks the caller to run the action. | -| `isConfirming` | `boolean` | `false` | Renders the action pending and ignores further presses. | -| `errorMessage` | `string` | — | Renders as a negative banner above the actions. | - -With a handle: - -| Prop | Type | Default | Description | -| ------------- | ---------------------------------------------- | ------------ | -------------------------------------------------------------------------------------- | -| `handle` | `ConfirmationHandle` | — (required) | From `Confirmation.createHandle()`. `handle.open(payload)` opens the block. | -| `title` | `string \| (payload: Payload) => string` | — (required) | Names what is about to happen. | -| `description` | `ReactNode \| (payload: Payload) => ReactNode` | — (required) | Spells out what it means. Takes markup, for a name to emphasise. | -| `actionLabel` | `string \| (payload: Payload) => string` | — (required) | The confirming button's label. | -| `cancelLabel` | `string` | `'Cancel'` | The cancel button's label. | -| `color` | `'negative' \| 'primary'` | `'negative'` | The weight the confirming button carries. | -| `finalFocus` | `DialogFocusTarget` | the trigger | Where focus returns on close, for a removal that unmounts the trigger. | -| `onConfirm` | `(payload: Payload) => Promise \| void` | — (required) | Runs the action. Resolve to close; reject with an `Error` to keep it open showing why. | - -## Driving it from a machine - -A section that wires the block to a state machine maps the machine's state onto the same props: +Wrap the action with `useReverificationFlow` and pass the result to the controller. The controller decides which step is showing, cancels the verification when the dialog closes, and blocks closing while the action is being retried. Confirmation and verification stay in the same dialog and card. ```tsx - send({ type: open ? 'OPEN' : 'CANCEL' })} - onConfirm={() => send({ type: 'CONFIRM' })} - isConfirming={snapshot.value === 'removing'} - errorMessage={snapshot.context.errorMessage} - {...copy} -/> +import { useReverificationFlow } from '@clerk/mosaic/features/reverification'; + +const [remove, reverification] = useReverificationFlow(removeConnectedAccount); +const controller = useConfirmController({ onConfirm: remove, reverification }); ``` + +## Props + +| Prop | Type | Default | Description | +| ------------- | ---------------------------------------------- | ------------ | ---------------------------------------------------------------------------------------- | +| `controller` | `ConfirmController` | — (required) | From `useConfirmController`. Owns open, pending, the error, and the reverification step. | +| `trigger` | `ReactNode` | — | The button that opens the dialog. Only for a controller without a payload. | +| `title` | `string \| (payload: Payload) => string` | — (required) | Names what is about to happen. | +| `description` | `ReactNode \| (payload: Payload) => ReactNode` | — (required) | Spells out what it means. Takes markup, for a name to emphasise. | +| `actionLabel` | `string \| (payload: Payload) => string` | — (required) | The confirming button's label. | +| `cancelLabel` | `string` | `'Cancel'` | The cancel button's label. | +| `color` | `'negative' \| 'primary'` | `'negative'` | The weight the confirming button carries. | +| `finalFocus` | `DialogFocusTarget` | the trigger | Where focus returns on close, for a removal that unmounts the trigger. | + +`useConfirmController(options)`: + +| Option | Type | Default | Description | +| ---------------- | ------------------------------------------------ | ----------------- | ------------------------------------------------------------------------------------------- | +| `onConfirm` | `(payload: Payload) => Promise \| void` | — (required) | Runs the action. Resolve to close; reject to keep the dialog open showing why. | +| `reverification` | `ReverificationController` | — | From `useReverificationFlow`. Renders verification in the same dialog when the action asks. | +| `errorFallback` | `string` | the generic error | Copy for a failure Clerk cannot describe. | + +The controller exposes `open(payload)`, `isOpen`, `isPending`, `errorMessage`, `step`, `onOpenChange` and `handle`, the dialog handle the block mounts on. diff --git a/packages/swingset/src/stories/confirmation.stories.tsx b/packages/swingset/src/stories/confirmation.stories.tsx index d7203733cc0..d52c12d4dee 100644 --- a/packages/swingset/src/stories/confirmation.stories.tsx +++ b/packages/swingset/src/stories/confirmation.stories.tsx @@ -1,3 +1,5 @@ +import type { ConfirmControllerOptions } from '@clerk/mosaic/blocks/confirm'; +import { useConfirmController } from '@clerk/mosaic/blocks/confirm'; import { Confirmation } from '@clerk/mosaic/blocks/confirmation'; import { Button } from '@clerk/mosaic/components/button'; import React from 'react'; @@ -16,7 +18,7 @@ export const meta: StoryMeta = { }; // A real removal is a network round trip. Without one the action never renders its pending -// state, so both stories wait before they settle. +// state, so the stories wait before they settle. const settleAfter = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); const trigger = ( @@ -28,72 +30,38 @@ const trigger = ( ); -/** - * The block holds nothing of its own. `open` closes it, `isConfirming` marks it busy, - * `errorMessage` explains a failure. - */ -export function Default() { - const [open, setOpen] = React.useState(false); - const [isConfirming, setIsConfirming] = React.useState(false); - - const handleConfirm = async () => { - setIsConfirming(true); - await settleAfter(2000); - setIsConfirming(false); - setOpen(false); - }; - +function Example(options: ConfirmControllerOptions) { + const remove = useConfirmController(options); return ( void handleConfirm()} - isConfirming={isConfirming} /> ); } /** - * A failed attempt leaves the dialog up. Pass the sentence the user should read as - * `errorMessage`, and clear it when the next attempt starts. + * The controller owns the dialog. Resolve `onConfirm` and it closes. */ -export function WithError() { - const [open, setOpen] = React.useState(false); - const [isConfirming, setIsConfirming] = React.useState(false); - const [errorMessage, setErrorMessage] = React.useState(undefined); - - const handleConfirm = async () => { - setErrorMessage(undefined); - setIsConfirming(true); - await settleAfter(2000); - setIsConfirming(false); - setErrorMessage('Google is your only way to sign in. Add a password or another account first.'); - }; - - // The error belongs to the caller, so the caller drops it. Without this a reopened dialog - // still shows why the last attempt failed. - const handleOpenChange = (next: boolean) => { - setOpen(next); - if (!next) { - setErrorMessage(undefined); - } - }; +export function Default() { + return settleAfter(2000)} />; +} +/** + * Reject `onConfirm` and the dialog stays up. A Clerk error is localized by its code; anything + * else renders as `errorFallback`. + */ +export function WithError() { return ( - void handleConfirm()} - isConfirming={isConfirming} - errorMessage={errorMessage} + { + await settleAfter(2000); + throw new Error('internal'); + }} + errorFallback='Google is your only way to sign in. Add a password or another account first.' /> ); } @@ -103,8 +71,6 @@ interface ConnectedAccount { provider: string; } -const removeAccount = Confirmation.createHandle(); - const describeRemoval = (account: ConnectedAccount) => ( <> {account.provider} will be removed from this account. You will no longer be able to use this @@ -113,23 +79,26 @@ const describeRemoval = (account: ConnectedAccount) => ( ); /** - * One block for many rows. `handle.open(account)` opens it with the account it is about, and the - * promise `onConfirm` returns closes it or explains the failure. The block owns the rest. + * One block for many rows. `remove.open(account)` opens it with the account it is about, the copy + * reads that payload, and `onConfirm` receives it back. */ -export function WithHandle() { +export function WithPayload() { const [accounts, setAccounts] = React.useState([ { id: 'eac_1', provider: 'Google' }, { id: 'eac_2', provider: 'GitHub' }, { id: 'eac_3', provider: 'Microsoft' }, ]); - const handleConfirm = async (account: ConnectedAccount) => { - await settleAfter(2000); - if (account.provider === 'Google') { - throw new Error('Google is your only way to sign in. Add a password or another account first.'); - } - setAccounts(current => current.filter(item => item.id !== account.id)); - }; + const remove = useConfirmController({ + onConfirm: async account => { + await settleAfter(2000); + if (account.provider === 'Google') { + throw new Error('internal'); + } + setAccounts(current => current.filter(item => item.id !== account.id)); + }, + errorFallback: 'Google is your only way to sign in. Add a password or another account first.', + }); return ( <> @@ -143,7 +112,7 @@ export function WithHandle() { @@ -151,11 +120,10 @@ export function WithHandle() { ))} ); diff --git a/packages/swingset/src/stories/destructive.mdx b/packages/swingset/src/stories/destructive.mdx index 4dd9998b8ac..c1ba712a1f4 100644 --- a/packages/swingset/src/stories/destructive.mdx +++ b/packages/swingset/src/stories/destructive.mdx @@ -17,19 +17,19 @@ import * as Stories from './destructive.stories'; ## Usage -A confirmation for a destructive action that warrants making the user type. Deleting an account, deleting a project, revoking an API key. For actions that do not warrant typing, use [Confirmation](/blocks/confirmation). +A confirmation for a destructive action that warrants making the user type. Deleting an account, deleting a project, revoking an API key. For actions that do not warrant typing, use [Confirmation](/components/confirmation). -The block holds the typed phrase and compares it to `confirmationValue`. `useDestructiveController` owns the dialog for a single action. Resolve `onDelete` and the dialog closes. Reject it and the dialog stays open. Pass the same props yourself to drive it from `useState` or a machine, or mount one block with a handle when many rows share it. +The block holds the typed phrase and compares it to `confirmationValue`. Everything else comes from a controller from `useConfirmController`, which owns whether the dialog is open, pending, or explaining a failure. Resolve `onConfirm` and the dialog closes. Reject it and the dialog stays open. A dialog that sits next to its button takes the button as `trigger`. ```tsx +import { useConfirmController } from '@clerk/mosaic/blocks/confirm'; import { Destructive } from '@clerk/mosaic/blocks/destructive'; -import { useDestructiveController } from '@clerk/mosaic/blocks/destructive/destructive.controller'; import { Button } from '@clerk/mosaic/components/button'; -const destructive = useDestructiveController({ onDelete: deleteAccount }); +const destroy = useConfirmController({ onConfirm: deleteAccount }); Delete account} title='Delete account?' description='Are you sure you want to delete your account? All of your data will be permanently deleted.' @@ -41,7 +41,7 @@ const destructive = useDestructiveController({ onDelete: deleteAccount }); ## Failure -A rejection leaves the dialog up. The field is marked invalid for as long as the message is set. +A rejection leaves the dialog up. A Clerk error is localized by its code. Anything else renders as the controller's `errorFallback`, or the generic error when none is given. The field is marked invalid for as long as the message is set, and the typed phrase stays so the user can retry. ; +; ``` ## One block, many rows -A table of projects has a Delete in every row. Create a handle, mount the block once after the table, and open it from any row with the project it is about. The copy props take a function of that payload, and `onDelete` receives it. The block owns `open`, the pending state, and the error: a resolved promise closes it, a rejected one keeps it open showing why. +A table of projects has a Delete in every row. Give the controller a payload type, mount the block once after the table, and open it from any row with the project it is about. The copy props take a function of that payload, and `onConfirm` receives it. The user must type `confirmationValue` before the action can run. The typed phrase clears when the dialog closes. ```tsx -const deleteProject = Destructive.createHandle(); +const deleteProject = useConfirmController({ onConfirm: project => api.deleteProject(project.id) }); deleteProject.open(project)}> Delete project `Delete ${project.name}?`} description='All project data will be permanently deleted.' fieldLabel='Type the project name to continue' confirmationValue={project => project.name} actionLabel='Delete project' - onDelete={project => api.deleteProject(project.id)} /> ``` Opened from a menu item, focus returns to that menu's trigger when the dialog closes. If deletion removes the trigger, pass `finalFocus` a ref to an element that remains on the page. -## Controlled - -The same props can be passed directly, from `useState` or from a machine. `open` closes the dialog, `isDeleting` marks it busy, `errorMessage` explains a failure. - -```tsx -import { Destructive } from '@clerk/mosaic/blocks/destructive'; -import { Button } from '@clerk/mosaic/components/button'; - -const [open, setOpen] = useState(false); -const [isDeleting, setIsDeleting] = useState(false); - -const handleDelete = async () => { - setIsDeleting(true); - await deleteAccount(); - setIsDeleting(false); - setOpen(false); -}; - -Delete account} - title='Delete account?' - description='Are you sure you want to delete your account? All of your data will be permanently deleted.' - fieldLabel='Type “Delete account” below to continue' - confirmationValue='Delete account' - actionLabel='Delete account' - onDelete={() => void handleDelete()} - isDeleting={isDeleting} -/>; -``` - -A machine maps onto those same props: - -```tsx - send({ type: open ? 'OPEN' : 'CANCEL' })} - onDelete={() => send({ type: 'CONFIRM' })} - isDeleting={snapshot.value === 'deleting'} - errorMessage={snapshot.context.errorMessage} - {...copy} -/> -``` - ## Props -Controlled: - -| Prop | Type | Default | Description | -| ------------------- | -------------------------- | ------------ | -------------------------------------------------------------------------------- | -| `open` | `boolean` | — (required) | Whether the confirmation is showing. Controlled, the way any dialog is. | -| `onOpenChange` | `(open: boolean) => void` | — (required) | Asks to open or close. Fired by the trigger, Cancel, Escape, and the backdrop. | -| `trigger` | `ReactNode` | — | The button that asks to open the dialog. | -| `title` | `string` | — (required) | Names what is about to happen. | -| `description` | `ReactNode` | — (required) | Spells out what it means. Takes markup, for a name to emphasise. | -| `fieldLabel` | `string` | — (required) | Labels the confirmation field. | -| `confirmationValue` | `string` | — (required) | The phrase the user has to type back. Also the field's placeholder. | -| `actionLabel` | `string` | — (required) | The confirming button's label. | -| `cancelLabel` | `string` | `'Cancel'` | The cancel button's label. | -| `finalFocus` | `DialogFocusTarget` | the trigger | Where focus returns on close, for a removal that unmounts the trigger. | -| `onDelete` | `() => void` | — (required) | Asks the caller to run the action, once the typed phrase matches. | -| `isDeleting` | `boolean` | `false` | Disables the field, renders the action pending, and ignores further submissions. | -| `errorMessage` | `string` | — | Marks the field invalid and renders under it. | -| `reverification` | `ReverificationController` | — | Renders reverification in the same dialog after the action requests it. | - -With a handle: - -| Prop | Type | Default | Description | -| ------------------- | ---------------------------------------------- | ------------ | -------------------------------------------------------------------------------------- | -| `handle` | `DestructiveHandle` | — (required) | From `Destructive.createHandle()`. `handle.open(payload)` opens the block. | -| `title` | `string \| (payload: Payload) => string` | — (required) | Names what is about to happen. | -| `description` | `ReactNode \| (payload: Payload) => ReactNode` | — (required) | Spells out what it means. Takes markup, for a name to emphasise. | -| `fieldLabel` | `string \| (payload: Payload) => string` | — (required) | Labels the confirmation field. | -| `confirmationValue` | `string \| (payload: Payload) => string` | — (required) | The phrase the user has to type back. Also the field's placeholder. | -| `actionLabel` | `string \| (payload: Payload) => string` | — (required) | The confirming button's label. | -| `cancelLabel` | `string` | `'Cancel'` | The cancel button's label. | -| `finalFocus` | `DialogFocusTarget` | the trigger | Where focus returns on close, for a removal that unmounts the trigger. | -| `onDelete` | `(payload: Payload) => Promise \| void` | — (required) | Runs the action. Resolve to close; reject with an `Error` to keep it open showing why. | +| Prop | Type | Default | Description | +| ------------------- | ---------------------------------------------- | ------------ | ---------------------------------------------------------------------------------------- | +| `controller` | `ConfirmController` | — (required) | From `useConfirmController`. Owns open, pending, the error, and the reverification step. | +| `trigger` | `ReactNode` | — | The button that opens the dialog. Only for a controller without a payload. | +| `title` | `string \| (payload: Payload) => string` | — (required) | Names what is about to happen. | +| `description` | `ReactNode \| (payload: Payload) => ReactNode` | — (required) | Spells out what it means. Takes markup, for a name to emphasise. | +| `fieldLabel` | `string \| (payload: Payload) => string` | — (required) | Labels the confirmation field. | +| `confirmationValue` | `string \| (payload: Payload) => string` | — (required) | The phrase the user has to type back. Also the field's placeholder. | +| `actionLabel` | `string \| (payload: Payload) => string` | — (required) | The confirming button's label. | +| `cancelLabel` | `string` | `'Cancel'` | The cancel button's label. | +| `finalFocus` | `DialogFocusTarget` | the trigger | Where focus returns on close, for a removal that unmounts the trigger. | + +The controller's options and surface are documented under [Confirmation](/components/confirmation). diff --git a/packages/swingset/src/stories/destructive.stories.tsx b/packages/swingset/src/stories/destructive.stories.tsx index df33aecc1ae..1d6d7fae759 100644 --- a/packages/swingset/src/stories/destructive.stories.tsx +++ b/packages/swingset/src/stories/destructive.stories.tsx @@ -1,5 +1,6 @@ +import type { ConfirmControllerOptions } from '@clerk/mosaic/blocks/confirm'; +import { useConfirmController } from '@clerk/mosaic/blocks/confirm'; import { Destructive } from '@clerk/mosaic/blocks/destructive'; -import { useDestructiveController } from '@clerk/mosaic/blocks/destructive/destructive.controller'; import { Button } from '@clerk/mosaic/components/button'; import { Menu } from '@clerk/mosaic/components/menu'; import React from 'react'; @@ -18,7 +19,7 @@ export const meta: StoryMeta = { }; // A real delete is a network round trip. Without one the action never renders its pending -// state, so both stories wait before they settle. +// state, so the stories wait before they settle. const settleAfter = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); const copy = { @@ -37,45 +38,54 @@ const copy = { actionLabel: 'Delete account', }; -function Example({ onDelete }: { onDelete: () => Promise }) { - const controller = useDestructiveController({ onDelete }); +function Example(options: ConfirmControllerOptions) { + const deleteAccount = useConfirmController(options); return ( ); } export function Default() { - return ( - { - await settleAfter(2000); - }} - /> - ); + return settleAfter(2000)} />; } export function WithError() { return ( { + onConfirm={async () => { await settleAfter(2000); - throw new Error('Your subscription is still active. Cancel it before you delete your account.'); + throw new Error('internal'); }} + errorFallback='Your subscription is still active. Cancel it before you delete your account.' /> ); } -export function WithHandle() { - const [projects, setProjects] = React.useState([ +interface Project { + id: string; + name: string; +} + +export function WithPayload() { + const [projects, setProjects] = React.useState([ { id: 'project_1', name: 'Production' }, { id: 'project_2', name: 'Staging' }, { id: 'project_3', name: 'Development' }, ]); - const deleteProject = React.useMemo(() => Destructive.createHandle<{ id: string; name: string }>(), []); const listRef = React.useRef(null); + const deleteProject = useConfirmController({ + onConfirm: async project => { + await settleAfter(2000); + if (project.name === 'Production') { + throw new Error('internal'); + } + setProjects(current => current.filter(item => item.id !== project.id)); + }, + errorFallback: 'Production still has active deployments. Remove them before deleting the project.', + }); return ( <> @@ -116,20 +126,13 @@ export function WithHandle() { ))} `Delete ${project.name}?`} description={project => `${project.name} and all of its data will be permanently deleted.`} fieldLabel={project => `Type “${project.name}” below to continue`} confirmationValue={project => project.name} actionLabel='Delete project' - onDelete={async project => { - await settleAfter(2000); - if (project.name === 'Production') { - throw new Error('Production still has active deployments. Remove them before deleting the project.'); - } - setProjects(current => current.filter(item => item.id !== project.id)); - }} /> ); From 4ec6429813ed0999da4ae355186c965711ed8ff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Mon, 12 Oct 2026 22:11:22 +0200 Subject: [PATCH 03/19] Move the contact sections' removal confirm and focus hand-off into their controllers --- .../user-profile-email-actions.test.tsx | 18 ++++---- .../user-profile-email-section.controller.ts | 24 ++++++++++- .../user-profile-email-section.types.ts | 10 ++++- .../user-profile-email-section.view.tsx | 42 +++++++------------ .../user-profile-phone-actions.test.tsx | 42 ++++++++----------- ...ser-profile-phone-section.feature.test.tsx | 4 +- .../user-profile-phone-section.controller.ts | 23 +++++++++- .../user-profile-phone-section.types.ts | 10 ++++- .../user-profile-phone-section.view.tsx | 42 +++++++------------ 9 files changed, 120 insertions(+), 95 deletions(-) diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-actions.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-actions.test.tsx index cd0fa69240d..ba0b6184c63 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-actions.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-actions.test.tsx @@ -52,11 +52,11 @@ describe('email actions', () => { trigger.focus(); await user.keyboard('{Enter}'); await user.keyboard('{Enter}'); - expect(screen.getByRole('alertdialog', { name: 'Remove email address?' })).toBeInTheDocument(); + expect(screen.getByRole('dialog', { name: 'Remove email address?' })).toBeInTheDocument(); await user.keyboard('{Escape}'); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); expect(onRemoveEmail).not.toHaveBeenCalled(); }); @@ -83,9 +83,9 @@ describe('email actions', () => { render(); await user.click(screen.getByRole('button', { name: 'Manage test@example.com' })); await user.click(screen.getByRole('menuitem', { name: 'Remove email' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); expect(screen.queryByRole('button', { name: 'Manage test@example.com' })).not.toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Add email' })).toBeEnabled(); }); @@ -98,7 +98,7 @@ describe('email actions', () => { await user.click(screen.getByRole('menuitem', { name: 'Set as primary' })); expect(onSetPrimaryEmail).toHaveBeenCalledExactlyOnceWith('email_1'); await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Not verified yet.')); - expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); }); it('shows the generic message and logs a primary update that threw unexpectedly', async () => { @@ -121,7 +121,7 @@ describe('email actions', () => { renderEmail({ onRemoveEmail }); await user.click(screen.getByRole('button', { name: 'Manage test@example.com' })); await user.click(screen.getByRole('menuitem', { name: 'Remove email' })); - const dialog = screen.getByRole('alertdialog', { name: 'Remove email address?' }); + const dialog = screen.getByRole('dialog', { name: 'Remove email address?' }); expect(dialog).toHaveAccessibleDescription(/test@example.com/); await user.click(within(dialog).getByRole('button', { name: 'Remove' })); expect(within(dialog).getByRole('button', { name: 'Remove' })).toHaveAttribute('aria-busy', 'true'); @@ -132,7 +132,7 @@ describe('email actions', () => { }); expect(within(dialog).getByRole('alert')).toHaveTextContent('Unable to remove email.'); await user.click(within(dialog).getByRole('button', { name: 'Remove' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); expect(onRemoveEmail).toHaveBeenNthCalledWith(1, 'email_1'); expect(onRemoveEmail).toHaveBeenNthCalledWith(2, 'email_1'); }); @@ -147,7 +147,7 @@ describe('email actions', () => { }); await user.click(screen.getByRole('button', { name: 'Manage test@example.com' })); await user.click(screen.getByRole('menuitem', { name: 'Remove email' })); - const dialog = screen.getByRole('alertdialog'); + const dialog = screen.getByRole('dialog'); if (warning) { expect(dialog).toHaveTextContent(warning); } else { @@ -190,7 +190,7 @@ describe('email actions', () => { expect(removeEmail).toHaveAttribute('data-color', 'negative'); await user.click(removeEmail); await user.click( - within(screen.getByRole('alertdialog', { name: 'Remove email address?' })).getByRole('button', { + within(screen.getByRole('dialog', { name: 'Remove email address?' })).getByRole('button', { name: 'Remove', }), ); diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts index 6cd1052ead5..daadd5fa1fb 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts @@ -1,4 +1,9 @@ +import { useRef } from 'react'; + +import { useConfirmController } from '../../../blocks/confirm'; +import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; +import type { UserProfileEmail } from '../user-profile-contact.types'; import { useUserProfileAddEmailController } from './user-profile-add-email.controller'; import type { ReadyEmailSectionModel, UserProfileEmailSectionViewProps } from './user-profile-email-section.types'; @@ -12,19 +17,36 @@ export function useUserProfileEmailSectionController({ }: Omit): UserProfileEmailSectionViewProps { const verification = useUserProfileAddEmailController({ username, onCreate: onCreateEmail }); const setPrimary = usePendingAction(); + const rowRef = useRef(null); + const verifyingId = useRef(undefined); + const removalFocus = useListRemovalFocus({ + ids: emails.map(email => email.id), + onRemove: onRemoveEmail, + fallback: () => rowRef.current?.querySelector('button:not([disabled])') ?? rowRef.current, + }); + const remove = useConfirmController({ onConfirm: email => removalFocus.remove(email.id) }); return { emails, canAdd: Boolean(onCreateEmail), verification, error: setPrimary.error, + rowRef, + registerTrigger: removalFocus.registerTrigger, + remove: onRemoveEmail ? remove : undefined, + finalFocus: removalFocus.finalFocus, + verificationFinalFocus: () => { + const id = verifyingId.current; + verifyingId.current = undefined; + return id ? removalFocus.trigger(id) : null; + }, onVerify: id => { + verifyingId.current = id; const email = emails.find(email => email.id === id); if (email) { verification.onVerifyEmail(email.value, getEmailVerifier(id)); } }, onSetPrimary: id => void setPrimary.run(id, () => onSetPrimaryEmail(id)), - onRemove: onRemoveEmail, }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.types.ts index 5b8ac79dcbb..2d94d934ec5 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.types.ts @@ -1,3 +1,7 @@ +import type { Ref } from 'react'; + +import type { ConfirmController } from '../../../blocks/confirm'; +import type { DialogFocusTarget } from '../../../components/dialog'; import type { UserProfileEmail, UserProfileEmailVerifier } from '../user-profile-contact.types'; import type { UserProfileAddEmailController } from './user-profile-add-email.controller'; @@ -6,9 +10,13 @@ export interface UserProfileEmailSectionViewProps { canAdd: boolean; verification: UserProfileAddEmailController; error: string | undefined; + rowRef: Ref; + registerTrigger: (id: string) => Ref; + remove?: ConfirmController; + finalFocus: DialogFocusTarget; + verificationFinalFocus: DialogFocusTarget; onVerify: (id: string) => void; onSetPrimary: (id: string) => void; - onRemove?: (id: string) => Promise; } export type UserProfileEmailSectionModel = diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx index e7f5478686e..cce2160f4d1 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx @@ -1,13 +1,11 @@ -import { useMemo, useRef } from 'react'; +import { useMemo } from 'react'; import { Confirmation } from '../../../blocks/confirmation'; import { Button } from '../../../components/button'; import { Dialog } from '../../../components/dialog'; import { Icon } from '../../../components/icon'; import { Section } from '../../../components/section'; -import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { fill, useMessages } from '../../../localization'; -import type { UserProfileEmail } from '../user-profile-contact.types'; import { UserProfileContactListRowView } from '../user-profile-contact-list-row.view'; import { UserProfileAddEmailDialog } from './user-profile-add-email.dialog'; import type { UserProfileEmailSectionViewProps } from './user-profile-email-section.types'; @@ -17,20 +15,16 @@ export function UserProfileEmailSectionView({ canAdd, verification, error, + rowRef, + registerTrigger, + remove, + finalFocus, + verificationFinalFocus, onVerify, onSetPrimary, - onRemove, }: UserProfileEmailSectionViewProps) { const m = useMessages('userProfileContact'); - const row = useRef(null); - const removalFocus = useListRemovalFocus({ - ids: emails.map(email => email.id), - onRemove, - fallback: () => row.current?.querySelector('button:not([disabled])') ?? row.current, - }); const verificationDialog = useMemo(() => Dialog.createHandle(), []); - const removeEmailConfirmation = useMemo(() => Confirmation.createHandle(), []); - const verifyingId = useRef(undefined); const addEmailAction = canAdd ? ( removeEmailConfirmation.open(email) : undefined} + onRemove={remove ? email => remove.open(email) : undefined} onSetPrimary={onSetPrimary} - onVerify={id => { - verifyingId.current = id; - onVerify(id); - }} + onVerify={onVerify} > {error} { - const id = verifyingId.current; - verifyingId.current = undefined; - return id ? removalFocus.trigger(id) : null; - }} + finalFocus={verificationFinalFocus} /> - {onRemove ? ( + {remove ? ( fill(email.isVerified ? m.email.removeDialog.verifiedDescription : m.email.removeDialog.description, { @@ -90,8 +77,7 @@ export function UserProfileEmailSectionView({ } actionLabel={m.email.removeDialog.confirm} cancelLabel={m.email.removeDialog.cancel} - finalFocus={removalFocus.finalFocus} - onConfirm={email => removalFocus.remove(email.id)} + finalFocus={finalFocus} /> ) : null} diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-actions.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-actions.test.tsx index e13324c4612..f395bd865a7 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-actions.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-actions.test.tsx @@ -33,19 +33,13 @@ function renderPhone(overrides: PhoneSectionProps = {}) { } describe('phone actions', () => { - it('ignores backdrop clicks and allows Escape to cancel removal', async () => { + it('keeps the phone when removal is canceled with Escape', async () => { const user = userEvent.setup(); const onRemovePhone = vi.fn(); renderPhone({ onRemovePhone }); await user.click(screen.getByRole('button', { name: 'Manage +1 (801) 555-0100' })); await user.click(screen.getByRole('menuitem', { name: 'Remove phone number' })); - const dialog = screen.getByRole('alertdialog', { name: 'Remove phone number?' }); - const backdrop = document.querySelector('.cl-dialog-backdrop'); - if (!backdrop) { - throw new Error('Expected a dialog backdrop'); - } - await user.click(backdrop); - expect(dialog).toBeInTheDocument(); + const dialog = screen.getByRole('dialog', { name: 'Remove phone number?' }); await user.keyboard('{Escape}'); await waitFor(() => expect(dialog).not.toBeInTheDocument()); expect(onRemovePhone).not.toHaveBeenCalled(); @@ -61,7 +55,7 @@ describe('phone actions', () => { renderPhone({ onRemovePhone }); await user.click(screen.getByRole('button', { name: 'Manage +1 (801) 555-0100' })); await user.click(screen.getByRole('menuitem', { name: 'Remove phone number' })); - const dialog = screen.getByRole('alertdialog'); + const dialog = screen.getByRole('dialog'); const remove = within(dialog).getByRole('button', { name: 'Remove' }); await user.click(remove); expect(dialog).toBeInTheDocument(); @@ -69,7 +63,7 @@ describe('phone actions', () => { await user.click(remove); expect(onRemovePhone).toHaveBeenCalledOnce(); finish(); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); }); it('ignores a second set primary while the first is pending', async () => { @@ -124,7 +118,7 @@ describe('phone actions', () => { await user.click(screen.getByRole('button', { name: 'Manage +1 (801) 555-0100' })); await user.click(screen.getByRole('menuitem', { name: 'Set as primary' })); expect(screen.getByText('Primary')).toBeInTheDocument(); - expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'Manage +1 (801) 555-0100' })).not.toBeInTheDocument(); }); @@ -134,8 +128,8 @@ describe('phone actions', () => { renderPhone({ onRemovePhone }); await user.click(screen.getByRole('button', { name: 'Manage +1 (801) 555-0100' })); await user.click(screen.getByRole('menuitem', { name: 'Remove phone number' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Cancel' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Cancel' })); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); expect(onRemovePhone).not.toHaveBeenCalled(); }); @@ -151,11 +145,11 @@ describe('phone actions', () => { trigger.focus(); await user.keyboard('{Enter}'); await user.keyboard('{Enter}'); - expect(screen.getByRole('alertdialog', { name: 'Remove phone number?' })).toBeInTheDocument(); + expect(screen.getByRole('dialog', { name: 'Remove phone number?' })).toBeInTheDocument(); await user.keyboard('{Escape}'); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); expect(onRemovePhone).not.toHaveBeenCalled(); }); @@ -181,9 +175,9 @@ describe('phone actions', () => { render(); await user.click(screen.getByRole('button', { name: 'Manage +1 (801) 555-0100' })); await user.click(screen.getByRole('menuitem', { name: 'Remove phone number' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); expect(screen.queryByRole('button', { name: 'Manage +1 (801) 555-0100' })).not.toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Add phone number' })).toBeEnabled(); }); @@ -197,10 +191,10 @@ describe('phone actions', () => { renderPhone({ onRemovePhone }); await user.click(screen.getByRole('button', { name: 'Manage +1 (801) 555-0100' })); await user.click(screen.getByRole('menuitem', { name: 'Remove phone number' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove' })); await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Cannot remove this phone.')); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove' })); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); expect(onRemovePhone).toHaveBeenCalledTimes(2); }); @@ -221,7 +215,7 @@ describe('phone actions', () => { await user.click(screen.getByRole('menuitem', { name: 'Set as primary' })); expect(onSetPrimaryPhone).toHaveBeenCalledExactlyOnceWith('phone_1'); await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Not verified yet.')); - expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); }); it('shows the generic message and logs a primary update that threw unexpectedly', async () => { @@ -243,11 +237,11 @@ describe('phone actions', () => { await user.click(screen.getByRole('button', { name: 'Manage +1 (801) 555-0100' })); await user.click(screen.getByRole('menuitem', { name: 'Remove phone number' })); expect(onRemovePhone).not.toHaveBeenCalled(); - const dialog = screen.getByRole('alertdialog', { name: 'Remove phone number?' }); + const dialog = screen.getByRole('dialog', { name: 'Remove phone number?' }); expect(dialog).toHaveTextContent('+1 (801) 555-0100'); await user.click(within(dialog).getByRole('button', { name: 'Remove' })); expect(onRemovePhone).toHaveBeenCalledExactlyOnceWith('phone_1'); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); }); it.each([ [true, 'You won’t be able to use it to sign in.'], @@ -260,7 +254,7 @@ describe('phone actions', () => { }); await user.click(screen.getByRole('button', { name: 'Manage +1 (801) 555-0100' })); await user.click(screen.getByRole('menuitem', { name: 'Remove phone number' })); - const dialog = screen.getByRole('alertdialog'); + const dialog = screen.getByRole('dialog'); if (warning) { expect(dialog).toHaveTextContent(warning); } else { diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx index 7e3bbc817d5..413fe854710 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx @@ -161,7 +161,7 @@ describe('the user profile phone section', () => { const { actor } = await renderSection(signedIn([HOME, WORK])); await manage(actor, WORK_LABEL, 'Remove phone number'); - const dialog = await screen.findByRole('alertdialog', { name: 'Remove phone number?' }); + const dialog = await screen.findByRole('dialog', { name: 'Remove phone number?' }); expect(dialog).toHaveTextContent(WORK_LABEL); await actor.click(within(dialog).getByRole('button', { name: 'Remove' })); @@ -173,7 +173,7 @@ describe('the user profile phone section', () => { const destroy = holdRequests('post', '/v1/me/phone_numbers/:id'); await manage(actor, WORK_LABEL, 'Remove phone number'); - const dialog = await screen.findByRole('alertdialog', { name: 'Remove phone number?' }); + const dialog = await screen.findByRole('dialog', { name: 'Remove phone number?' }); await actor.click(within(dialog).getByRole('button', { name: 'Remove' })); await waitFor(() => expect(destroy.requests).toHaveLength(1)); destroy.fail('action_blocked', 'Raw server sentence.'); diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.controller.ts index 431fbe9b86d..5bad8fedfd7 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.controller.ts @@ -1,6 +1,10 @@ import { stringToFormattedPhoneString } from '@clerk/shared/phone'; +import { useRef } from 'react'; +import { useConfirmController } from '../../../blocks/confirm'; +import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; +import type { UserProfilePhone } from '../user-profile-contact.types'; import { useUserProfileAddPhoneController } from './user-profile-add-phone.controller'; import type { ReadyPhoneSectionModel, UserProfilePhoneSectionViewProps } from './user-profile-phone-section.types'; @@ -14,6 +18,14 @@ export function useUserProfilePhoneSectionController({ }: Omit): UserProfilePhoneSectionViewProps { const verification = useUserProfileAddPhoneController({ onCreate: onCreatePhone }); const setPrimary = usePendingAction(); + const rowRef = useRef(null); + const verifyingId = useRef(undefined); + const removalFocus = useListRemovalFocus({ + ids: phones.map(phone => phone.id), + onRemove: onRemovePhone, + fallback: () => rowRef.current?.querySelector('button:not([disabled])') ?? rowRef.current, + }); + const remove = useConfirmController({ onConfirm: phone => removalFocus.remove(phone.id) }); return { phones: phones.map(phone => ({ ...phone, value: stringToFormattedPhoneString(phone.value) })), @@ -21,13 +33,22 @@ export function useUserProfilePhoneSectionController({ canAdd: Boolean(onCreatePhone), verification, error: setPrimary.error, + rowRef, + registerTrigger: removalFocus.registerTrigger, + remove: onRemovePhone ? remove : undefined, + finalFocus: removalFocus.finalFocus, + verificationFinalFocus: () => { + const id = verifyingId.current; + verifyingId.current = undefined; + return id ? removalFocus.trigger(id) : null; + }, onVerify: id => { + verifyingId.current = id; const phone = phones.find(phone => phone.id === id); if (phone) { verification.onVerifyPhone(phone.value, getPhoneVerifier(id)); } }, onSetPrimary: id => void setPrimary.run(id, () => onSetPrimaryPhone(id)), - onRemove: onRemovePhone, }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.types.ts index e71d766ebd7..a8c869de768 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.types.ts @@ -1,3 +1,7 @@ +import type { Ref } from 'react'; + +import type { ConfirmController } from '../../../blocks/confirm'; +import type { DialogFocusTarget } from '../../../components/dialog'; import type { CountryIso } from '../../../components/phone-input'; import type { UserProfilePhone, UserProfilePhoneVerifier } from '../user-profile-contact.types'; import type { UserProfileAddPhoneController } from './user-profile-add-phone.controller'; @@ -8,9 +12,13 @@ export interface UserProfilePhoneSectionViewProps { canAdd: boolean; verification: UserProfileAddPhoneController; error: string | undefined; + rowRef: Ref; + registerTrigger: (id: string) => Ref; + remove?: ConfirmController; + finalFocus: DialogFocusTarget; + verificationFinalFocus: DialogFocusTarget; onVerify: (id: string) => void; onSetPrimary: (id: string) => void; - onRemove?: (id: string) => Promise; } export type UserProfilePhoneSectionModel = diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx index 07c89c6ebc7..9ccc7dc9053 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx @@ -1,13 +1,11 @@ -import { useMemo, useRef } from 'react'; +import { useMemo } from 'react'; import { Confirmation } from '../../../blocks/confirmation'; import { Button } from '../../../components/button'; import { Dialog } from '../../../components/dialog'; import { Icon } from '../../../components/icon'; import { Section } from '../../../components/section'; -import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { fill, useMessages } from '../../../localization'; -import type { UserProfilePhone } from '../user-profile-contact.types'; import { UserProfileContactListRowView } from '../user-profile-contact-list-row.view'; import { UserProfileAddPhoneDialog } from './user-profile-add-phone.dialog'; import type { UserProfilePhoneSectionViewProps } from './user-profile-phone-section.types'; @@ -18,20 +16,16 @@ export function UserProfilePhoneSectionView({ canAdd, verification, error, + rowRef, + registerTrigger, + remove, + finalFocus, + verificationFinalFocus, onVerify, onSetPrimary, - onRemove, }: UserProfilePhoneSectionViewProps) { const m = useMessages('userProfileContact'); - const row = useRef(null); - const removalFocus = useListRemovalFocus({ - ids: phones.map(phone => phone.id), - onRemove, - fallback: () => row.current?.querySelector('button:not([disabled])') ?? row.current, - }); const verificationDialog = useMemo(() => Dialog.createHandle(), []); - const removePhoneConfirmation = useMemo(() => Confirmation.createHandle(), []); - const verifyingId = useRef(undefined); const addPhoneAction = canAdd ? ( removePhoneConfirmation.open(phone) : undefined} + onRemove={remove ? phone => remove.open(phone) : undefined} onSetPrimary={onSetPrimary} - onVerify={id => { - verifyingId.current = id; - onVerify(id); - }} + onVerify={onVerify} > {error} @@ -75,15 +66,11 @@ export function UserProfilePhoneSectionView({ {...verification} defaultCountry={defaultPhoneCountry} handle={verificationDialog} - finalFocus={() => { - const id = verifyingId.current; - verifyingId.current = undefined; - return id ? removalFocus.trigger(id) : null; - }} + finalFocus={verificationFinalFocus} /> - {onRemove ? ( + {remove ? ( fill(phone.isVerified ? m.phone.removeDialog.verifiedDescription : m.phone.removeDialog.description, { @@ -92,8 +79,7 @@ export function UserProfilePhoneSectionView({ } actionLabel={m.phone.removeDialog.confirm} cancelLabel={m.phone.removeDialog.cancel} - finalFocus={removalFocus.finalFocus} - onConfirm={phone => removalFocus.remove(phone.id)} + finalFocus={finalFocus} /> ) : null} From 7634290af439a1cb13b91fc0caed44b8bb129ed5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Mon, 12 Oct 2026 22:15:17 +0200 Subject: [PATCH 04/19] Drive both danger sections from useConfirmController --- .../organization-profile-danger-section.tsx | 6 +++--- .../organization-profile-danger-section.view.tsx | 10 +++++----- .../user-profile-profile-panel.view.test.tsx | 6 +++--- .../user-profile-security-panel.view.test.tsx | 6 +++--- .../user-profile-danger-section.tsx | 11 ++++------- .../user-profile-danger-section.view.tsx | 10 +++++++--- .../src/stories/fixtures/organization-profile.tsx | 6 +++--- .../swingset/src/stories/fixtures/user-profile.tsx | 6 +++--- .../organization-profile-danger-section.stories.tsx | 6 +++--- .../stories/user-profile-danger-section.stories.tsx | 6 +++--- 10 files changed, 37 insertions(+), 36 deletions(-) diff --git a/packages/mosaic/src/features/organization-profile/organization-profile-danger-section/organization-profile-danger-section.tsx b/packages/mosaic/src/features/organization-profile/organization-profile-danger-section/organization-profile-danger-section.tsx index ed15ce8388e..14bc6fbdb48 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile-danger-section/organization-profile-danger-section.tsx +++ b/packages/mosaic/src/features/organization-profile/organization-profile-danger-section/organization-profile-danger-section.tsx @@ -1,4 +1,4 @@ -import { useDestructiveController } from '../../../blocks/destructive/destructive.controller'; +import { useConfirmController } from '../../../blocks/confirm'; import type { OrganizationProfileDangerSectionModel } from './organization-profile-danger-section.model'; import { useOrganizationProfileDangerSectionModel } from './organization-profile-danger-section.model'; import { OrganizationProfileDangerSectionView } from './organization-profile-danger-section.view'; @@ -32,8 +32,8 @@ function OrganizationDangerZone({ }: { model: Extract; }) { - const leave = useDestructiveController({ onDelete: model.leaveOrganization }); - const destroy = useDestructiveController({ onDelete: async () => model.deleteOrganization?.() }); + const leave = useConfirmController({ onConfirm: model.leaveOrganization }); + const destroy = useConfirmController({ onConfirm: async () => model.deleteOrganization?.() }); return ( Promise.resolve() }); - return ; + const controller = useConfirmController({ onConfirm: () => Promise.resolve() }); + return ; } function renderView(overrides: UserProfileProfilePanelViewProps = {}) { diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-security-panel.view.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-security-panel.view.test.tsx index f4649c0cf4d..7833f7b3d8f 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-security-panel.view.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-security-panel.view.test.tsx @@ -3,7 +3,7 @@ import { act, render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { describe, expect, it, vi } from 'vitest'; -import { useDestructiveController } from '../../../blocks/destructive/destructive.controller'; +import { useConfirmController } from '../../../blocks/confirm'; import { MosaicProvider } from '../../../mosaic-provider'; import type { UserProfileDevice } from '../user-profile-active-devices-section/user-profile-active-devices-section.view'; import { UserProfileActiveDevicesSectionView } from '../user-profile-active-devices-section/user-profile-active-devices-section.view'; @@ -13,8 +13,8 @@ import type { UserProfileSecurityPanelViewProps } from '../user-profile-security import { UserProfileSecurityPanelView } from '../user-profile-security-panel.view'; function DeleteAccount() { - const controller = useDestructiveController({ onDelete: () => Promise.resolve() }); - return ; + const controller = useConfirmController({ onConfirm: () => Promise.resolve() }); + return ; } const devices: UserProfileDevice[] = [ { diff --git a/packages/mosaic/src/features/user-profile/user-profile-danger-section/user-profile-danger-section.tsx b/packages/mosaic/src/features/user-profile/user-profile-danger-section/user-profile-danger-section.tsx index cf47e53943b..f1b0fa31a09 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-danger-section/user-profile-danger-section.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-danger-section/user-profile-danger-section.tsx @@ -1,6 +1,6 @@ import { useClerk, useUser } from '@clerk/shared/react'; -import { useDestructiveController } from '../../../blocks/destructive/destructive.controller'; +import { useConfirmController } from '../../../blocks/confirm'; import { useReverificationFlow } from '../../reverification'; import { UserProfileDangerSectionView } from './user-profile-danger-section.view'; @@ -36,11 +36,8 @@ export function UserProfileDangerSection(props: UserProfileDangerSectionProps) { }; // -- Controllers -- - const [deleteAccountWithReverification, reverificationState] = useReverificationFlow(deleteAccount); - const destructiveController = useDestructiveController({ - onDelete: deleteAccountWithReverification, - reverification: reverificationState, - }); + const [deleteAccountWithReverification, reverification] = useReverificationFlow(deleteAccount); + const controller = useConfirmController({ onConfirm: deleteAccountWithReverification, reverification }); // -- View -- if (!isLoaded) { @@ -51,5 +48,5 @@ export function UserProfileDangerSection(props: UserProfileDangerSectionProps) { return null; } - return ; + return ; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-danger-section/user-profile-danger-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-danger-section/user-profile-danger-section.view.tsx index 1d8e94cda99..e238cc92561 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-danger-section/user-profile-danger-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-danger-section/user-profile-danger-section.view.tsx @@ -1,10 +1,14 @@ +import type { ConfirmController } from '../../../blocks/confirm'; import { Destructive } from '../../../blocks/destructive'; -import type { DestructiveController } from '../../../blocks/destructive/destructive.controller'; import { Button } from '../../../components/button'; import { Section } from '../../../components/section'; import { fill, useMessages } from '../../../localization'; -export function UserProfileDangerSectionView(destructiveProps: DestructiveController) { +export interface UserProfileDangerSectionViewProps { + controller: ConfirmController; +} + +export function UserProfileDangerSectionView({ controller }: UserProfileDangerSectionViewProps) { const m = useMessages('userProfileDangerSection'); return ( @@ -24,7 +28,7 @@ export function UserProfileDangerSectionView(destructiveProps: DestructiveContro new Promise(resolve => setTimeout(resolve, ms)); export function OrganizationProfileDangerPreview({ name, memberCount }: { name: string; memberCount: number }) { - const leave = useDestructiveController({ onDelete: () => settleAfter(1200) }); - const destroy = useDestructiveController({ onDelete: () => settleAfter(1200) }); + const leave = useConfirmController({ onConfirm: () => settleAfter(1200) }); + const destroy = useConfirmController({ onConfirm: () => settleAfter(1200) }); return ( Promise.resolve() }); - return ; + const controller = useConfirmController({ onConfirm: () => Promise.resolve() }); + return ; } const exampleEmails: UserProfileEmail[] = [ diff --git a/packages/swingset/src/stories/organization-profile-danger-section.stories.tsx b/packages/swingset/src/stories/organization-profile-danger-section.stories.tsx index 151bdb9a5ba..46bc7c4316e 100644 --- a/packages/swingset/src/stories/organization-profile-danger-section.stories.tsx +++ b/packages/swingset/src/stories/organization-profile-danger-section.stories.tsx @@ -1,4 +1,4 @@ -import { useDestructiveController } from '@clerk/mosaic/blocks/destructive/destructive.controller'; +import { useConfirmController } from '@clerk/mosaic/blocks/confirm'; import { OrganizationProfileDangerSectionView } from '@clerk/mosaic/features/organization-profile/organization-profile-danger-section/organization-profile-danger-section.view'; import { ClerkAPIResponseError } from '@clerk/shared/error'; import { useState } from 'react'; @@ -34,8 +34,8 @@ function DangerSectionHarness({ onLeave: () => Promise; onDelete?: () => Promise; }) { - const leave = useDestructiveController({ onDelete: onLeave }); - const destroy = useDestructiveController({ onDelete: onDelete ?? (() => Promise.resolve()) }); + const leave = useConfirmController({ onConfirm: onLeave }); + const destroy = useConfirmController({ onConfirm: onDelete ?? (() => Promise.resolve()) }); return ( new Promise(resolve => setTimeout(resolve, ms)); function DangerSectionHarness({ onDelete }: { onDelete: () => Promise }) { - const controller = useDestructiveController({ onDelete }); - return ; + const controller = useConfirmController({ onConfirm: onDelete }); + return ; } export function Default() { From 304916018791fdd53f4dcff1b624bcaf255779a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Mon, 12 Oct 2026 22:19:58 +0200 Subject: [PATCH 05/19] Move the passkeys, web3 wallets and connected accounts removal confirms into their controllers --- .../user-profile-profile-panel.view.test.tsx | 33 ++++++++++++--- .../user-profile-security-panel.view.test.tsx | 34 +++++++++++----- ...file-web3-wallets-section.feature.test.tsx | 20 +++++----- ...onnected-accounts.removal.feature.test.tsx | 18 ++++----- ...e-connected-accounts-section.controller.ts | 32 +++++++++++++++ ...ser-profile-connected-accounts-section.tsx | 10 ++--- ...rofile-connected-accounts-section.types.ts | 11 ++++- ...rofile-connected-accounts-section.view.tsx | 38 +++++------------- .../user-profile-passkeys-section.view.tsx | 40 +++++++++---------- ...ile-passkeys-interactions.feature.test.tsx | 4 +- ...-profile-passkeys-section.feature.test.tsx | 20 +++++----- ...ser-profile-passkeys-section.controller.ts | 23 ++++++++++- ...profile-web3-wallets-section.controller.ts | 35 +++++++++++++--- .../user-profile-web3-wallets-section.tsx | 8 ++-- ...user-profile-web3-wallets-section.types.ts | 10 ++++- ...user-profile-web3-wallets-section.view.tsx | 38 +++++------------- .../user-profile-connected-accounts.ts | 12 ++++-- .../stories/fixtures/user-profile-passkeys.ts | 32 +++++---------- .../fixtures/user-profile-web3-wallets.ts | 36 ++++++++--------- .../src/stories/fixtures/user-profile.tsx | 18 ++------- .../user-profile-profile-panel.stories.tsx | 18 ++------- 21 files changed, 274 insertions(+), 216 deletions(-) diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-profile-panel.view.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-profile-panel.view.test.tsx index b0074d4b0c0..0a99ed47816 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-profile-panel.view.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-profile-panel.view.test.tsx @@ -9,8 +9,30 @@ import { MosaicProvider } from '../../../mosaic-provider'; import { UserProfileDangerSectionView } from '../user-profile-danger-section/user-profile-danger-section.view'; import type { UserProfileProfilePanelViewProps } from '../user-profile-profile-panel.view'; import { UserProfileProfilePanelView } from '../user-profile-profile-panel.view'; +import { useUserProfileWeb3WalletsController } from '../user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller'; +import type { UserProfileWeb3Wallet } from '../user-profile-web3-wallets-section/user-profile-web3-wallets-section.types'; import { UserProfileWeb3WalletsSectionView } from '../user-profile-web3-wallets-section/user-profile-web3-wallets-section.view'; +function Web3Wallets({ + wallets, + fallbackFocus, + onRemove, +}: { + wallets: UserProfileWeb3Wallet[]; + fallbackFocus: () => HTMLElement | null; + onRemove: (id: string) => Promise; +}) { + const controller = useUserProfileWeb3WalletsController({ + wallets, + availableProviders: [], + connect: () => Promise.resolve(), + setPrimary: () => Promise.resolve(), + remove: onRemove, + fallbackFocus, + }); + return ; +} + function DeleteAccount() { const controller = useConfirmController({ onConfirm: () => Promise.resolve() }); return ; @@ -58,7 +80,7 @@ describe('UserProfileProfilePanelView', () => { const { rerender } = renderView({ titleRef, web3WalletsSlot: ( - titleRef.current} onRemove={onRemoveWeb3Wallet} @@ -67,13 +89,13 @@ describe('UserProfileProfilePanelView', () => { }); await user.click(screen.getByRole('button', { name: 'Manage MetaMask' })); await user.click(screen.getByRole('menuitem', { name: 'Remove wallet' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove' })); rerender( titleRef.current} onRemove={onRemoveWeb3Wallet} @@ -83,12 +105,12 @@ describe('UserProfileProfilePanelView', () => { , ); expect(screen.queryByRole('heading', { name: 'Web3 wallets' })).not.toBeInTheDocument(); - expect(screen.getByRole('alertdialog')).toHaveTextContent('0x1234'); + expect(screen.getByRole('dialog')).toHaveTextContent('0x1234'); await act(async () => { removal.resolve(); await removal.promise; }); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); }); it('renders the danger zone when provided', () => { @@ -163,7 +185,6 @@ describe('UserProfileProfilePanelView', () => { availableProviders={[{ id: 'disconnected', provider: 'Coinbase Wallet' }]} onConnect={vi.fn()} onSetPrimary={vi.fn()} - onRemove={vi.fn()} /> ), }); diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-security-panel.view.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-security-panel.view.test.tsx index 7833f7b3d8f..7e4547fa4d7 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-security-panel.view.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-security-panel.view.test.tsx @@ -8,7 +8,9 @@ import { MosaicProvider } from '../../../mosaic-provider'; import type { UserProfileDevice } from '../user-profile-active-devices-section/user-profile-active-devices-section.view'; import { UserProfileActiveDevicesSectionView } from '../user-profile-active-devices-section/user-profile-active-devices-section.view'; import { UserProfileDangerSectionView } from '../user-profile-danger-section/user-profile-danger-section.view'; +import type { UserProfilePasskey } from '../user-profile-passkeys-section.view'; import { UserProfilePasskeysSectionView } from '../user-profile-passkeys-section.view'; +import { useUserProfilePasskeysSectionController } from '../user-profile-passkeys-section/user-profile-passkeys-section.controller'; import type { UserProfileSecurityPanelViewProps } from '../user-profile-security-panel.view'; import { UserProfileSecurityPanelView } from '../user-profile-security-panel.view'; @@ -16,6 +18,16 @@ function DeleteAccount() { const controller = useConfirmController({ onConfirm: () => Promise.resolve() }); return ; } + +function Passkeys({ passkeys, onRemove }: { passkeys: UserProfilePasskey[]; onRemove: (id: string) => Promise }) { + const controller = useUserProfilePasskeysSectionController({ + passkeys, + onRename: () => Promise.resolve(), + validateName: () => undefined, + onRemove, + }); + return ; +} const devices: UserProfileDevice[] = [ { id: 'current', @@ -148,14 +160,14 @@ describe('UserProfileSecurityPanelView', () => { signOutAll, ); await user.click(signOutAll); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Sign out' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Sign out' })); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); const activeDevices = screen.getByRole('group', { name: 'Active devices' }); await user.click(within(activeDevices).getByRole('button', { name: 'Manage Safari on iOS' })); await user.click(screen.getByRole('menuitem', { name: 'Sign out' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Sign out' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Sign out' })); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); expect(onSignOutDevice).toHaveBeenCalledWith('mobile'); expect(onSignOutAllOtherDevices).toHaveBeenCalledOnce(); @@ -227,7 +239,7 @@ describe('UserProfileSecurityPanelView', () => { const { rerender } = renderView({ mfaMethods: undefined, passkeysSlot: ( - @@ -236,13 +248,13 @@ describe('UserProfileSecurityPanelView', () => { await user.click(screen.getByRole('button', { name: 'Manage Passkey' })); await user.click(screen.getByRole('menuitem', { name: 'Remove passkey' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove' })); rerender( @@ -251,13 +263,13 @@ describe('UserProfileSecurityPanelView', () => { , ); expect(screen.getByRole('region', { name: 'Authentication' })).toBeInTheDocument(); - expect(screen.getByRole('alertdialog', { name: 'Remove passkey' })).toBeVisible(); + expect(screen.getByRole('dialog', { name: 'Remove passkey' })).toBeVisible(); await act(async () => { removal.resolve(); await removal.promise; }); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); expect(screen.getByRole('region', { name: 'Authentication' })).toBeVisible(); expect(screen.getByRole('heading', { name: 'Passkeys' })).toBeVisible(); expect(screen.getByText('No passkeys added')).toBeVisible(); @@ -288,13 +300,13 @@ describe('UserProfileSecurityPanelView', () => { await user.click(screen.getByRole('button', { name: 'Manage SMS verification' })); expect(screen.queryByRole('menuitem', { name: 'Manage' })).not.toBeInTheDocument(); await user.click(screen.getByRole('menuitem', { name: 'Remove method' })); - const dialog = screen.getByRole('alertdialog', { name: 'Remove SMS verification' }); + const dialog = screen.getByRole('dialog', { name: 'Remove SMS verification' }); expect(dialog).toHaveAccessibleDescription( 'This phone number will no longer receive sign-in verification codes. It will remain on your account.', ); expect(onRemoveMfaMethod).not.toHaveBeenCalled(); await user.click(within(dialog).getByRole('button', { name: 'Remove', exact: true })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); await user.click(screen.getByRole('button', { name: 'Manage Backup codes' })); expect(screen.queryByRole('menuitem', { name: 'Remove method' })).not.toBeInTheDocument(); await user.click(screen.getByRole('menuitem', { name: 'Regenerate' })); diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-web3-wallets-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-web3-wallets-section.feature.test.tsx index b3072c883b8..88b6ec0639f 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-web3-wallets-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-web3-wallets-section.feature.test.tsx @@ -76,9 +76,9 @@ describe('Web3 wallets', () => { await user.click(screen.getByRole('menuitem', { name: 'Remove wallet' })); const warning = 'You will no longer be able to sign in using this web3 wallet.'; if (status === 'verified') { - expect(screen.getByRole('alertdialog')).toHaveTextContent(warning); + expect(screen.getByRole('dialog')).toHaveTextContent(warning); } else { - expect(screen.getByRole('alertdialog')).not.toHaveTextContent(warning); + expect(screen.getByRole('dialog')).not.toHaveTextContent(warning); } }, ); @@ -521,8 +521,8 @@ describe('Web3 wallets', () => { await user.click(await screen.findByRole('button', { name: 'Manage MetaMask' })); await user.click(screen.getByRole('menuitem', { name: 'Remove wallet' })); - expect(screen.getByRole('alertdialog')).toHaveTextContent('0x1234...cdef'); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove' })); + expect(screen.getByRole('dialog')).toHaveTextContent('0x1234...cdef'); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove' })); await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(0)); expect(fapi.client.sessions[0]?.user.primary_web3_wallet_id).toBeNull(); @@ -537,20 +537,20 @@ describe('Web3 wallets', () => { await user.click(await screen.findByRole('button', { name: 'Manage MetaMask' })); await user.click(screen.getByRole('menuitem', { name: 'Remove wallet' })); - const confirm = within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove' }); + const confirm = within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove' }); await user.click(confirm); await waitFor(() => expect(hold.requests).toHaveLength(1)); expect(confirm).toHaveAttribute('aria-busy', 'true'); hold.fail('wallet_removal_failed'); await waitFor(() => - expect(within(screen.getByRole('alertdialog')).getByRole('alert')).toHaveTextContent('wallet_removal_failed'), + expect(within(screen.getByRole('dialog')).getByRole('alert')).toHaveTextContent('wallet_removal_failed'), ); await waitFor(() => expect(confirm).not.toHaveAttribute('aria-busy', 'true')); const retryFapi = serveFapi(fapi); await user.click(confirm); await waitFor(() => expect(retryFapi.client.sessions[0]?.user.web3_wallets).toHaveLength(0)); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); }); it('cancels removal, then removes the newly selected wallet', async () => { @@ -567,13 +567,13 @@ describe('Web3 wallets', () => { await user.keyboard('{Enter}'); await user.click(screen.getByRole('menuitem', { name: 'Remove wallet' })); await user.keyboard('{Escape}'); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(2); await user.click(screen.getAllByRole('button', { name: 'Manage MetaMask' })[0]); await user.click(screen.getByRole('menuitem', { name: 'Remove wallet' })); - expect(screen.getByRole('alertdialog')).toHaveTextContent('0xabcd...7890'); + expect(screen.getByRole('dialog')).toHaveTextContent('0xabcd...7890'); await user.click(screen.getByRole('button', { name: 'Remove' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); expect(fapi.client.sessions[0]?.user.web3_wallets.map(wallet => wallet.id)).toEqual(['wallet_1']); expect(fapi.client.sessions[0]?.user.primary_web3_wallet_id).toBe('wallet_1'); expect(screen.getByText('Primary')).toBeInTheDocument(); diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/__tests__/user-profile-connected-accounts.removal.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/__tests__/user-profile-connected-accounts.removal.feature.test.tsx index d9c5296a924..51a967d2356 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/__tests__/user-profile-connected-accounts.removal.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/__tests__/user-profile-connected-accounts.removal.feature.test.tsx @@ -11,7 +11,7 @@ import { github, google, renderSection, signedIn } from './user-profile-connecte async function openRemoval(user: ReturnType, provider: string) { await user.click(screen.getByRole('button', { name: `Manage ${provider}` })); await user.click(screen.getByRole('menuitem', { name: 'Remove' })); - return screen.getByRole('alertdialog'); + return screen.getByRole('dialog'); } describe('connected accounts', () => { @@ -29,7 +29,7 @@ describe('connected accounts', () => { await waitFor(() => expect(request.requests).toHaveLength(1)); request.release(); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); expect(screen.queryByRole('button', { name: 'Manage GitHub' })).toBeNull(); expect(screen.getByRole('button', { name: 'Manage Google' })).toBeVisible(); expect(fapi.client.sessions[0]?.user.external_accounts.map(account => account.id)).toEqual(['idn_google']); @@ -47,7 +47,7 @@ describe('connected accounts', () => { expect(fapi.client.sessions[0]?.user.external_accounts).toHaveLength(2); request.release(); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); expect(fapi.client.sessions[0]?.user.external_accounts).toHaveLength(1); }); @@ -66,7 +66,7 @@ describe('connected accounts', () => { serveFapi(signedIn([google])); await user.click(within(dialog).getByRole('button', { name: 'Remove' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); }); it('keeps the confirmation open when removal needs reverification', async () => { @@ -79,7 +79,7 @@ describe('connected accounts', () => { request.fail('session_reverification_required', 'Verify your session.'); expect(await within(dialog).findByText('Verify your session.')).toBeInTheDocument(); expect(dialog).toHaveAccessibleName('Remove connected account'); - expect(screen.queryByRole('dialog')).toBeNull(); + expect(screen.getAllByRole('dialog')).toEqual([dialog]); expect(within(dialog).queryByRole('textbox')).toBeNull(); expect(within(dialog).queryByLabelText('Password')).toBeNull(); expect(within(dialog).getByRole('button', { name: 'Remove' })).not.toHaveAttribute('aria-busy', 'true'); @@ -92,12 +92,12 @@ describe('connected accounts', () => { const first = await openRemoval(user, 'Google'); expect(first).toHaveAccessibleDescription(/Google will be removed/); await user.click(within(first).getByRole('button', { name: 'Cancel' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); const second = await openRemoval(user, 'GitHub'); expect(second).toHaveAccessibleDescription(/GitHub will be removed/); await user.click(within(second).getByRole('button', { name: 'Remove' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); expect(fapi.client.sessions[0]?.user.external_accounts.map(account => account.id)).toEqual(['idn_google']); }); @@ -125,7 +125,7 @@ describe('connected accounts', () => { const user = userEvent.setup(); const dialog = await openRemoval(user, 'Google'); await user.click(within(dialog).getByRole('button', { name: 'Remove' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); expect(screen.queryByRole('group', { name: 'Connected accounts' })).toBeNull(); }); @@ -146,7 +146,7 @@ describe('connected accounts', () => { const destroy = vi.spyOn(original, 'destroy'); await openRemoval(userEvent.setup(), 'Google'); await act(() => (change === 'switch' ? clerk.setActive({ session: 'sess_2' }) : clerk.signOut())); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); expect(clerk.user?.id).toBe(change === 'switch' ? 'user_2' : undefined); expect(destroy).not.toHaveBeenCalled(); if (change === 'switch') { diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts index 1c06846fbb0..6e84a9901be 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts @@ -1,3 +1,10 @@ +import type { Ref } from 'react'; +import { useRef } from 'react'; + +import type { ConfirmController } from '../../../blocks/confirm'; +import { useConfirmController } from '../../../blocks/confirm'; +import type { DialogFocusTarget } from '../../../components/dialog'; +import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; import { useMessages } from '../../../localization'; import type { @@ -11,6 +18,8 @@ export interface UserProfileConnectedAccountsControllerOptions { availableProviders: UserProfileConnectionProvider[]; onConnect: (id: string) => Promise; onReconnect: (id: string) => Promise; + onRemove?: (id: string) => void | Promise; + fallbackFocus?: () => HTMLElement | null; } export interface UserProfileConnectedAccountsController { @@ -19,6 +28,10 @@ export interface UserProfileConnectedAccountsController { pendingId: string | undefined; onConnect: (id: string) => void; onReconnect: (id: string) => void; + sectionRef: Ref; + registerTrigger: (id: string) => Ref; + remove: ConfirmController | undefined; + finalFocus: DialogFocusTarget; } const OAUTH_REDIRECT_HOLD_MS = 2000; @@ -28,9 +41,24 @@ export function useUserProfileConnectedAccountsController({ availableProviders, onConnect, onReconnect, + onRemove, + fallbackFocus, }: UserProfileConnectedAccountsControllerOptions): UserProfileConnectedAccountsController { const messages = useMessages('userProfileConnectedAccounts'); const actions = usePendingAction({ errorFallback: messages.errors.generic }); + const sectionRef = useRef(null); + const removalFocus = useListRemovalFocus({ + ids: accounts.map(account => account.id), + onRemove, + fallback: () => + sectionRef.current?.querySelector('button:not([disabled])') ?? + sectionRef.current ?? + fallbackFocus?.() ?? + null, + }); + const remove = useConfirmController({ + onConfirm: account => removalFocus.remove(account.id), + }); const run = (id: string, action: (id: string) => Promise) => void actions.run(id, async () => { @@ -49,5 +77,9 @@ export function useUserProfileConnectedAccountsController({ pendingId: actions.pendingKey, onConnect: id => run(id, onConnect), onReconnect: id => run(id, onReconnect), + sectionRef, + registerTrigger: removalFocus.registerTrigger, + remove: onRemove ? remove : undefined, + finalFocus: removalFocus.finalFocus, }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.tsx b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.tsx index d601119b69c..e95ed35e0b9 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.tsx @@ -49,13 +49,9 @@ function ConnectedAccounts({ availableProviders: model.availableProviders, onConnect: model.connect, onReconnect: model.reconnect, + onRemove: model.remove, + fallbackFocus, }); - return ( - - ); + return ; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.types.ts index adcd5ffc3d5..bbee8248558 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.types.ts @@ -1,3 +1,8 @@ +import type { Ref } from 'react'; + +import type { ConfirmController } from '../../../blocks/confirm'; +import type { DialogFocusTarget } from '../../../components/dialog'; + export type ConnectedAccountActionResult = 'redirecting' | void; export interface ConnectedAccountProviderDisplay { @@ -20,11 +25,13 @@ export interface UserProfileConnectedAccount extends ConnectedAccountProviderDis } export interface UserProfileConnectedAccountsSectionViewProps { - fallbackFocus?: () => HTMLElement | null; accounts: UserProfileConnectedAccount[]; + sectionRef?: Ref; availableProviders?: UserProfileConnectionProvider[]; pendingId?: string; onConnect?: (id: string) => void; onReconnect?: (id: string) => void; - onRemove?: (id: string) => void | Promise; + registerTrigger?: (id: string) => Ref; + remove?: ConfirmController; + finalFocus?: DialogFocusTarget; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.view.tsx index 7a806d89c06..272cb2afc4b 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.view.tsx @@ -1,17 +1,11 @@ -import { useMemo, useRef } from 'react'; - import { Confirmation } from '../../../blocks/confirmation'; import { Section } from '../../../components/section'; -import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { fill, useMessages } from '../../../localization'; import { UserProfileConnectedAccountRowView, UserProfileConnectProviderRowView, } from './user-profile-connected-account-row.view'; -import type { - UserProfileConnectedAccount, - UserProfileConnectedAccountsSectionViewProps, -} from './user-profile-connected-accounts-section.types'; +import type { UserProfileConnectedAccountsSectionViewProps } from './user-profile-connected-accounts-section.types'; export type { UserProfileConnectedAccount, @@ -21,25 +15,16 @@ export type { export function UserProfileConnectedAccountsSectionView({ accounts, - fallbackFocus, + sectionRef, availableProviders = [], pendingId, onConnect, onReconnect, - onRemove, + registerTrigger, + remove, + finalFocus, }: UserProfileConnectedAccountsSectionViewProps) { const m = useMessages('userProfileConnectedAccounts'); - const section = useRef(null); - const removalFocus = useListRemovalFocus({ - ids: accounts.map(account => account.id), - onRemove, - fallback: () => - section.current?.querySelector('button:not([disabled])') ?? - section.current ?? - fallbackFocus?.() ?? - null, - }); - const removeAccount = useMemo(() => Confirmation.createHandle(), []); const hasRows = accounts.length > 0 || (availableProviders.length > 0 && Boolean(onConnect)); const isBusy = pendingId !== undefined; @@ -48,7 +33,7 @@ export function UserProfileConnectedAccountsSectionView({ {hasRows ? ( @@ -61,10 +46,10 @@ export function UserProfileConnectedAccountsSectionView({ removeAccount.open(account) : undefined} + onRemove={remove ? account => remove.open(account) : undefined} /> ))} {onConnect @@ -82,15 +67,14 @@ export function UserProfileConnectedAccountsSectionView({ ) : null} - {onRemove ? ( + {remove ? ( fill(m.removeDialog.description, { provider: account.provider })} actionLabel={m.removeDialog.confirm} cancelLabel={m.removeDialog.cancel} - finalFocus={removalFocus.finalFocus} - onConfirm={account => removalFocus.remove(account.id)} + finalFocus={finalFocus} /> ) : null} diff --git a/packages/mosaic/src/features/user-profile/user-profile-passkeys-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-passkeys-section.view.tsx index 5a0c24d00b1..b06196d9702 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-passkeys-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-passkeys-section.view.tsx @@ -1,11 +1,12 @@ -import { useMemo, useRef } from 'react'; +import type { Ref } from 'react'; +import type { ConfirmController } from '../../blocks/confirm'; import { Confirmation } from '../../blocks/confirmation'; import { Button } from '../../components/button'; +import type { DialogFocusTarget } from '../../components/dialog'; import { Field } from '../../components/field'; import { Icon } from '../../components/icon'; import { Section } from '../../components/section'; -import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; import { fill, useMessages } from '../../localization'; import { UserProfilePasskeyRowView } from './user-profile-passkey-row.view'; import { styles } from './user-profile-passkeys-section.styles'; @@ -18,37 +19,37 @@ export type { UserProfilePasskey } from './user-profile-passkeys-section/user-pr export interface UserProfilePasskeysSectionViewProps { passkeys: UserProfilePasskey[]; + sectionRef?: Ref; + addButtonRef?: Ref; onAdd?: () => void; isAdding?: boolean; addError?: string; onRename?: (id: string, name: string) => void | Promise; validateName?: UserProfilePasskeyNameValidator; - onRemove?: (id: string) => void | Promise; + registerTrigger?: (id: string) => Ref; + remove?: ConfirmController; + finalFocus?: DialogFocusTarget; } export function UserProfilePasskeysSectionView({ passkeys, + sectionRef, + addButtonRef, onAdd, isAdding, addError, onRename, validateName, - onRemove, + registerTrigger, + remove, + finalFocus, }: UserProfilePasskeysSectionViewProps) { const m = useMessages('userProfilePasskeys'); - const addButton = useRef(null); - const section = useRef(null); - const removalFocus = useListRemovalFocus({ - ids: passkeys.map(passkey => passkey.id), - onRemove, - fallback: () => addButton.current ?? section.current, - }); - const removePasskey = useMemo(() => Confirmation.createHandle(), []); return ( <> @@ -64,7 +65,7 @@ export function UserProfilePasskeysSectionView({ {onAdd ? ( @@ -93,8 +64,8 @@ export function UserProfileActiveDevicesSectionView({ deviceDetails.open(device)} + triggerRef={device.id === currentDevices[0]?.id ? currentDeviceTriggerRef : undefined} + onViewDetails={device => details.open(device)} /> )) ) : ( @@ -108,52 +79,41 @@ export function UserProfileActiveDevicesSectionView({ deviceDetails.open(device)} + triggerRef={registerTrigger(device.id)} + onSignOut={signOut ? device => signOut.open(device) : undefined} + onViewDetails={device => details.open(device)} /> ))} - {onSignOutAllOtherDevices ? ( + {signOutAll ? ( - signOutAll.onConfirm(async () => { - await onSignOutAllOtherDevices(); - signedOutAll.current = true; - }) - } - isConfirming={signOutAll.isConfirming} - errorMessage={signOutAll.errorMessage} /> ) : null} - {onSignOutDevice ? ( + {signOut ? ( fill(m.signOutDialog.description, { name: device.name })} actionLabel={m.signOutDialog.confirm} cancelLabel={m.signOutDialog.cancel} - finalFocus={removalFocus.finalFocus} - onConfirm={device => signOutActions?.remove(device)} - errorFallback={m.detailsDialog.signOutError} + finalFocus={finalFocus} /> ) : null} diff --git a/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-device-details.dialog.tsx b/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-device-details.dialog.tsx index b88bfe54248..f4084681971 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-device-details.dialog.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-device-details.dialog.tsx @@ -1,31 +1,28 @@ -import { useConfirmationController } from '../../../blocks/confirmation/confirmation.controller'; +import type { ConfirmController } from '../../../blocks/confirm'; import { Button, SubmitButton } from '../../../components/button'; import { Card } from '../../../components/card'; import { DataList } from '../../../components/data-list'; -import type { DialogFocusTarget, DialogHandle } from '../../../components/dialog'; +import type { DialogFocusTarget } from '../../../components/dialog'; import { Dialog } from '../../../components/dialog'; import { fill, useMessages } from '../../../localization'; import type { UserProfileDevice } from './user-profile-active-devices.types'; export interface UserProfileDeviceDetailsDialogProps { - handle: DialogHandle; + controller: ConfirmController; devices: UserProfileDevice[]; finalFocus?: DialogFocusTarget; - onSignOut?: (device: UserProfileDevice) => void | Promise; + canSignOut?: boolean; } export function UserProfileDeviceDetailsDialog({ - handle, + controller, devices, finalFocus, - onSignOut, + canSignOut = false, }: UserProfileDeviceDetailsDialogProps) { - const m = useMessages('userProfileActiveDevices'); - const controller = useConfirmationController({ errorFallback: m.detailsDialog.signOutError }); - return ( @@ -44,8 +41,8 @@ export function UserProfileDeviceDetailsDialog({ > controller.onConfirm(async () => onSignOut(device)) : undefined} - isSigningOut={controller.isConfirming} + onSignOut={canSignOut ? () => controller.confirm(device) : undefined} + isSigningOut={controller.isPending} errorMessage={controller.errorMessage} /> @@ -62,7 +59,7 @@ function DeviceDetailsCard({ errorMessage, }: { device: UserProfileDevice; - onSignOut?: (device: UserProfileDevice) => void; + onSignOut?: () => void; isSigningOut: boolean; errorMessage: string | undefined; }) { @@ -111,7 +108,7 @@ function DeviceDetailsCard({ type='button' fullWidth isPending={isSigningOut} - onClick={() => onSignOut(device)} + onClick={onSignOut} > {m.detailsDialog.signOut} diff --git a/packages/swingset/src/stories/fixtures/user-profile-active-devices.ts b/packages/swingset/src/stories/fixtures/user-profile-active-devices.ts index b00ab3a2abd..2fb5b8c2e44 100644 --- a/packages/swingset/src/stories/fixtures/user-profile-active-devices.ts +++ b/packages/swingset/src/stories/fixtures/user-profile-active-devices.ts @@ -1,3 +1,4 @@ +import { useUserProfileActiveDevicesController } from '@clerk/mosaic/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller'; import type { UserProfileDevice } from '@clerk/mosaic/features/user-profile/user-profile-active-devices-section/user-profile-active-devices.types'; import { useRef, useState } from 'react'; @@ -108,15 +109,15 @@ export function useUserProfileActiveDevicesFixture({ } }; - return { + return useUserProfileActiveDevicesController({ devices, - onSignOutDevice: async (id: string) => { + revoke: async (id: string) => { await settle('device'); setDevices(current => current.filter(device => device.id !== id)); }, - onSignOutAllOtherDevices: async () => { + revokeAll: async () => { await settle('all'); setDevices(current => current.filter(device => device.isCurrent)); }, - }; + }); } diff --git a/packages/swingset/src/stories/user-profile-active-devices-section.stories.tsx b/packages/swingset/src/stories/user-profile-active-devices-section.stories.tsx index 984ea0ca1fe..90a0b8c93ef 100644 --- a/packages/swingset/src/stories/user-profile-active-devices-section.stories.tsx +++ b/packages/swingset/src/stories/user-profile-active-devices-section.stories.tsx @@ -38,8 +38,8 @@ export function Impersonation() { return ( ); } From 63c54843f9cb977801386a0d5c51459f13367195 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Mon, 12 Oct 2026 22:31:27 +0200 Subject: [PATCH 08/19] Move the API keys and organization table confirms and focus hand-offs into their controllers --- .../api-keys/api-keys-table.controller.ts | 42 ++++++++---- .../features/api-keys/api-keys-table.types.ts | 10 ++- .../features/api-keys/api-keys-table.view.tsx | 44 ++++++------- .../invitations-table-tab.view.test.tsx | 49 +++++++++++--- .../__tests__/members-table-tab.view.test.tsx | 64 +++++++++++++------ ...ation-profile-invitations.feature.test.tsx | 36 +++++------ ...ion-profile-members-panel.feature.test.tsx | 4 +- .../invitations-table-tab.controller.ts | 43 ++++++++++--- .../invitations-table-tab.types.ts | 11 +++- .../invitations-table-tab.view.tsx | 41 +++++------- .../members-table-tab.controller.ts | 35 +++++++--- .../members-table-tab.types.ts | 11 +++- .../members-table-tab.utils.ts | 3 + .../members-table-tab.view.tsx | 57 ++++++++--------- .../requests-table-tab.controller.ts | 48 ++++++++++---- .../requests-table-tab.types.ts | 6 +- .../requests-table-tab.view.tsx | 56 ++++++---------- .../src/stories/api-keys-table.stories.tsx | 2 +- .../src/stories/fixtures/api-keys-table.tsx | 49 +++++++++----- .../stories/fixtures/invitations-table-tab.ts | 44 +++++++++---- .../src/stories/fixtures/members-table-tab.ts | 20 +++++- 21 files changed, 432 insertions(+), 243 deletions(-) create mode 100644 packages/mosaic/src/features/organization-profile/members-table-tab.utils.ts diff --git a/packages/mosaic/src/features/api-keys/api-keys-table.controller.ts b/packages/mosaic/src/features/api-keys/api-keys-table.controller.ts index 456ed7cb808..cadf2bede49 100644 --- a/packages/mosaic/src/features/api-keys/api-keys-table.controller.ts +++ b/packages/mosaic/src/features/api-keys/api-keys-table.controller.ts @@ -1,5 +1,7 @@ import { useEffect, useRef, useState } from 'react'; +import { useConfirmController } from '../../blocks/confirm'; +import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; import { useNow } from '../../hooks/use-now'; import { useLocale } from '../../localization'; import { formatDate } from '../../utils/format-date'; @@ -18,13 +20,37 @@ export function useAPIKeysTableController(subject: string, messages: APIKeysTabl const [searchValue, setSearchValue] = useState(''); const timer = useRef | undefined>(undefined); const create = useCreateAPIKeyController({ messages, onCreate: model.create }); + const apiKeys = model.data.map(key => toAPIKey(key, locale, now)); + const searchInputRef = useRef(null); + const createButtonRef = useRef(null); + const removalFocus = useListRemovalFocus({ + ids: apiKeys.map(key => key.id), + onRemove: model.canManage + ? async id => { + const emptiesPage = model.page > 1 && model.data.length === 1; + await model.revoke(id); + if (emptiesPage) { + model.fetchPage(model.page - 1); + } + } + : undefined, + fallback: () => createButtonRef.current ?? searchInputRef.current, + }); + const revoke = useConfirmController({ + onConfirm: key => removalFocus.remove(key.id), + errorFallback: messages.revokeError, + }); useEffect(() => () => clearTimeout(timer.current), []); return { isLoaded: model.isLoaded, isAvailable: model.isAvailable, - apiKeys: model.data.map(key => toAPIKey(key, locale, now)), + apiKeys, + searchInputRef, + createButtonRef, + registerTrigger: removalFocus.registerTrigger, + finalFocus: removalFocus.finalFocus, totalCount: model.count, page: model.page, searchValue, @@ -41,19 +67,7 @@ export function useAPIKeysTableController(subject: string, messages: APIKeysTabl model.fetchPage(1); }, SEARCH_DEBOUNCE_MS); }, - manage: model.canManage - ? { - onCreate: create.onOpen, - createDialog: create.dialog, - onRevoke: async (id: string) => { - const emptiesPage = model.page > 1 && model.data.length === 1; - await model.revoke(id); - if (emptiesPage) { - model.fetchPage(model.page - 1); - } - }, - } - : undefined, + manage: model.canManage ? { onCreate: create.onOpen, createDialog: create.dialog, revoke } : undefined, }; } diff --git a/packages/mosaic/src/features/api-keys/api-keys-table.types.ts b/packages/mosaic/src/features/api-keys/api-keys-table.types.ts index 5068e8761d8..c54c1794e86 100644 --- a/packages/mosaic/src/features/api-keys/api-keys-table.types.ts +++ b/packages/mosaic/src/features/api-keys/api-keys-table.types.ts @@ -1,6 +1,8 @@ import type { APIKeyResource } from '@clerk/shared/types'; -import type { MouseEventHandler } from 'react'; +import type { MouseEventHandler, Ref } from 'react'; +import type { ConfirmController } from '../../blocks/confirm'; +import type { DialogFocusTarget } from '../../components/dialog'; import type { MosaicMessages } from '../../localization'; import type { CreateAPIKeyDialogProps } from './create-api-key.dialog'; @@ -41,9 +43,13 @@ export interface APIKeysTableViewProps { onPageChange: (page: number) => void; onPageSizeChange?: (pageSize: number) => void; onSearchChange: (value: string) => void; + searchInputRef?: Ref; + createButtonRef?: Ref; onCreate?: MouseEventHandler; createDialog?: CreateAPIKeyDialogProps; - onRevoke?: (id: string) => Promise; + registerTrigger?: (id: string) => Ref; + revoke?: ConfirmController; + finalFocus?: DialogFocusTarget; onBulkAction?: (ids: string[]) => void; sort?: APIKeysTableSort | null; onSortChange?: (sort: APIKeysTableSort | null) => void; diff --git a/packages/mosaic/src/features/api-keys/api-keys-table.view.tsx b/packages/mosaic/src/features/api-keys/api-keys-table.view.tsx index 6f8f4db3b27..c9e0b815ec0 100644 --- a/packages/mosaic/src/features/api-keys/api-keys-table.view.tsx +++ b/packages/mosaic/src/features/api-keys/api-keys-table.view.tsx @@ -1,5 +1,6 @@ import * as stylex from '@stylexjs/stylex'; -import { useMemo, useRef, useState } from 'react'; +import type { Ref } from 'react'; +import { useState } from 'react'; import { Destructive } from '../../blocks/destructive'; import { Button } from '../../components/button'; @@ -10,7 +11,6 @@ import { Spinner } from '../../components/spinner'; import { Table } from '../../components/table'; import { Text } from '../../components/text'; import { VisuallyHidden } from '../../components/visually-hidden'; -import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; import { useServerDataTable } from '../../hooks/use-server-data-table'; import { fill } from '../../localization'; import { mergeStyleProps, themeProps } from '../../props'; @@ -31,9 +31,13 @@ export function APIKeysTableView({ onPageSizeChange, searchValue, onSearchChange, + searchInputRef, + createButtonRef, onCreate, createDialog, - onRevoke, + registerTrigger, + revoke, + finalFocus, onBulkAction, sort, onSortChange, @@ -42,14 +46,6 @@ export function APIKeysTableView({ isError = false, onRetry, }: APIKeysTableViewProps) { - const searchInput = useRef(null); - const createButton = useRef(null); - const removalFocus = useListRemovalFocus({ - ids: apiKeys.map(getRowId), - onRemove: onRevoke, - fallback: () => createButton.current ?? searchInput.current, - }); - const revokeKey = useMemo(() => Destructive.createHandle(), []); const { table, sortHeader, pagination } = useServerDataTable({ data: apiKeys, totalCount, @@ -64,7 +60,7 @@ export function APIKeysTableView({ searchValue, onSearchChange, }); - const columnCount = 3 + Number(Boolean(onRevoke)) + Number(Boolean(onBulkAction)); + const columnCount = 3 + Number(Boolean(revoke)) + Number(Boolean(onBulkAction)); const query = searchValue.trim(); const emptyState = query ? { label: m.empty, description: fill(m.emptyDescription, { query }) } @@ -74,7 +70,7 @@ export function APIKeysTableView({
{onCreate ? (
{createDialog ? : null} - {onRevoke ? ( + {revoke ? ( fill(m.revokeTitle, { name: apiKey.name })} description={m.revokeDescription} fieldLabel={apiKey => fill(m.revokeFieldLabel, { name: apiKey.name })} confirmationValue={apiKey => apiKey.name} actionLabel={m.revoke} cancelLabel={m.cancel} - onDelete={apiKey => removalFocus.remove(apiKey.id)} - errorFallback={m.revokeError} - finalFocus={removalFocus.finalFocus} + finalFocus={finalFocus} /> ) : null} @@ -251,10 +245,10 @@ function APIKeyActions({ }: { messages: APIKeysTableMessages; apiKey: APIKey; - registerTrigger: (id: string) => (element: HTMLButtonElement | null) => void; + registerTrigger?: (id: string) => Ref; onSelect: (key: APIKey) => void; }) { - const [triggerRef] = useState(() => registerTrigger(apiKey.id)); + const [triggerRef] = useState(() => registerTrigger?.(apiKey.id)); return ( = {}): InvitationsTableTabViewProps { +type InvitationsProps = Omit & { onRevoke?: (id: string) => Promise }; + +function Invitations({ onRevoke, ...props }: InvitationsProps) { + const m = useMessages('invitationsTableTab'); + const inviteButtonRef = useRef(null); + const removalFocus = useListRemovalFocus({ + ids: props.invitations.map(invitation => invitation.id), + onRemove: onRevoke, + fallback: () => inviteButtonRef.current, + }); + const revoke = useConfirmController({ + onConfirm: invitation => removalFocus.remove(invitation.id), + errorFallback: m.revokeError, + }); + return ( + + ); +} + +function propsFor(overrides: Partial = {}): InvitationsProps { return { invitations: [{ id: 'invite-1', email: 'ada@example.com', invitedAtLabel: 'Sep 1, 2026', roleLabel: 'Admin' }], totalCount: 1, @@ -17,13 +46,13 @@ function propsFor(overrides: Partial = {}): Invita ...overrides, }; } -function renderView(overrides: Partial = {}) { +function renderView(overrides: Partial = {}) { const props = propsFor(overrides); return { props, ...render( - + , ), }; @@ -43,13 +72,13 @@ describe('InvitationsTableTabView', () => { - + , ); await user.click(screen.getByRole('button', { name: 'Manage ada@example.com' })); await user.click(screen.getByRole('menuitem', { name: 'Revoke invitation' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Revoke invitation' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Revoke invitation' })); await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Could not revoke invitation.')); }, @@ -60,7 +89,7 @@ describe('InvitationsTableTabView', () => { expect(screen.getByRole('status')).toHaveTextContent('Loading invitations'); rerender( - @@ -69,7 +98,7 @@ describe('InvitationsTableTabView', () => { expect(screen.getByRole('status')).toHaveTextContent('No pending invitations'); rerender( - @@ -79,7 +108,7 @@ describe('InvitationsTableTabView', () => { expect(screen.getByRole('table')).toHaveAttribute('aria-busy', 'true'); rerender( - { expect(screen.getByRole('button', { name: 'Manage ada@example.com' })).toBeVisible(); rerender( - = {}): MembersTableTabViewProps { +type MembersProps = Omit & { onRemove?: (id: string) => Promise }; + +function Members({ onRemove, ...props }: MembersProps) { + const m = useMessages('membersTableTab'); + const inviteButtonRef = useRef(null); + const removalFocus = useListRemovalFocus({ + ids: props.members.map(member => member.id), + onRemove, + fallback: () => inviteButtonRef.current, + }); + const remove = useConfirmController({ + onConfirm: member => removalFocus.remove(member.id), + errorFallback: m.removeError, + }); + return ( + + ); +} + +function propsFor(overrides: Partial = {}): MembersProps { return { members: [ { @@ -44,13 +72,13 @@ function propsFor(overrides: Partial = {}): MembersTab }; } -function renderView(overrides: Partial = {}) { +function renderView(overrides: Partial = {}) { const props = propsFor(overrides); return { props, ...render( - + , ), }; @@ -66,7 +94,7 @@ describe('MembersTableTabView', () => { const user = userEvent.setup(); render( - , @@ -74,7 +102,7 @@ describe('MembersTableTabView', () => { await user.click(screen.getByRole('button', { name: 'Manage Grace Hopper' })); await user.click(screen.getByRole('menuitem', { name: 'Remove from organization' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove from organization' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove from organization' })); await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Could not remove member.')); }); @@ -84,7 +112,7 @@ describe('MembersTableTabView', () => { expect(screen.getByRole('status')).toHaveTextContent('Loading members'); rerender( - @@ -93,7 +121,7 @@ describe('MembersTableTabView', () => { expect(screen.getByText('No members yet')).toBeVisible(); rerender( - { expect(screen.getByRole('status')).toHaveTextContent('No members found'); rerender( - { await user.click(screen.getByRole('checkbox', { name: 'Select Linus Torvalds' })); await user.click(screen.getByRole('button', { name: 'Manage Grace Hopper' })); await user.click(screen.getByRole('menuitem', { name: 'Remove from organization' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove from organization' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove from organization' })); await waitFor(() => expect(screen.getByRole('checkbox', { name: 'Select Linus Torvalds' })).not.toBeChecked()); }); it('does not retain hidden selection for protected members within a Shift-click range', async () => { @@ -161,7 +189,7 @@ describe('MembersTableTabView', () => { expect(screen.getByRole('checkbox', { name: 'Select Last' })).toBeChecked(); rerender( - ({ ...item, isCurrentUser: false, isDeprovisioned: false }))} /> @@ -211,7 +239,7 @@ it('removes the confirmed member', async () => { ]); return ( - { await user.click(screen.getByRole('button', { name: 'Manage Ada Lovelace' })); await user.click(screen.getByRole('menuitem', { name: 'Remove from organization' })); expect(onMutation).not.toHaveBeenCalled(); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove from organization' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove from organization' })); expect(onMutation).toHaveBeenCalledExactlyOnceWith('ada'); - expect(screen.getByRole('alertdialog')).toBeInTheDocument(); + expect(screen.getByRole('dialog')).toBeInTheDocument(); await act(async () => { pending.resolve(); await pending.promise; }); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); await user.click(screen.getByRole('button', { name: 'Manage Grace' })); await user.click(screen.getByRole('menuitem', { name: 'Remove from organization' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove from organization' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove from organization' })); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); expect(onMutation).toHaveBeenLastCalledWith('grace'); }); diff --git a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile-invitations.feature.test.tsx b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile-invitations.feature.test.tsx index 99bd92d8bc3..c2c7fe3a6b3 100644 --- a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile-invitations.feature.test.tsx +++ b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile-invitations.feature.test.tsx @@ -109,10 +109,10 @@ describe('connected organization invitations', () => { await user.click(screen.getByRole('button', { name: 'Manage ada@example.com' })); await user.click(screen.getByRole('menuitem', { name: 'Revoke invitation' })); expect(fapi.organizationInvitations[0]?.status).toBe('pending'); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Revoke invitation' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Revoke invitation' })); await waitFor(() => expect(fapi.organizationInvitations[0]?.status).toBe('revoked')); expect(await screen.findByText('No pending invitations')).toBeVisible(); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); await waitFor(() => expect(screen.getByRole('group', { name: 'Invitations' })).toContainElement(document.activeElement), ); @@ -125,17 +125,17 @@ describe('connected organization invitations', () => { await user.click(await screen.findByRole('button', { name: 'Manage ada@example.com' })); await user.click(screen.getByRole('menuitem', { name: 'Revoke invitation' })); const revoke = holdRequests('post', '/v1/organizations/:organizationId/invitations/:invitationId/revoke'); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Revoke invitation' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Revoke invitation' })); await waitFor(() => expect(revoke.requests).toHaveLength(1)); revoke.fail('network_error', 'Unavailable'); await waitFor(() => - expect(within(screen.getByRole('alertdialog')).getByRole('alert')).toHaveTextContent( + expect(within(screen.getByRole('dialog')).getByRole('alert')).toHaveTextContent( 'Unable to reach the server. Check your connection and try again.', ), ); expect(fapi.organizationInvitations[0]?.status).toBe('pending'); const retry = serve(['org:sys_memberships:manage']); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Revoke invitation' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Revoke invitation' })); await waitFor(() => expect(retry.organizationInvitations[0]?.status).toBe('revoked')); }); @@ -146,9 +146,9 @@ describe('connected organization invitations', () => { await user.click(await screen.findByRole('button', { name: 'Manage ada@example.com' })); await user.click(screen.getByRole('menuitem', { name: 'Revoke invitation' })); fapi.organizationInvitations = [{ ...invitation, status: 'accepted' }]; - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Revoke invitation' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Revoke invitation' })); await waitFor(() => - expect(within(screen.getByRole('alertdialog')).getByRole('alert')).toHaveTextContent( + expect(within(screen.getByRole('dialog')).getByRole('alert')).toHaveTextContent( 'This invitation is no longer pending.', ), ); @@ -162,13 +162,13 @@ describe('connected organization invitations', () => { await user.click(await screen.findByRole('button', { name: 'Manage ada@example.com' })); await user.click(screen.getByRole('menuitem', { name: 'Revoke invitation' })); const revoke = holdRequests('post', '/v1/organizations/:organizationId/invitations/:invitationId/revoke'); - const confirm = within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Revoke invitation' }); + const confirm = within(screen.getByRole('dialog')).getByRole('button', { name: 'Revoke invitation' }); await user.click(confirm); await waitFor(() => expect(revoke.requests).toHaveLength(1)); confirm.click(); confirm.click(); revoke.release(); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); expect(revoke.requests).toHaveLength(1); }); @@ -181,11 +181,11 @@ describe('connected organization invitations', () => { await user.click(await screen.findByRole('button', { name: 'Manage ada@example.com' })); await user.click(screen.getByRole('menuitem', { name: 'Revoke invitation' })); const refresh = holdRequests('get', '/v1/organizations/:organizationId/invitations'); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Revoke invitation' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Revoke invitation' })); await waitFor(() => expect(refresh.requests).toHaveLength(1)); refresh.fail('network_error', 'Unavailable'); await waitFor(() => expect(screen.getByText('Unable to load invitations')).toBeVisible(), { timeout: 12_000 }); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); expect(fapi.organizationInvitations.find(item => item.id === invitation.id)?.status).toBe('revoked'); expect(screen.queryByText('other@example.com')).toBeNull(); const table = within(screen.getByRole('table', { name: 'Invitations' })); @@ -247,7 +247,7 @@ describe('connected organization invitations', () => { nextPage.release(); await user.click(await screen.findByRole('button', { name: 'Manage ada@example.com' })); await user.click(screen.getByRole('menuitem', { name: 'Revoke invitation' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Revoke invitation' })); + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Revoke invitation' })); expect(await screen.findByText('user0@example.com')).toBeVisible(); expect(screen.queryByText('ada@example.com')).toBeNull(); }); @@ -266,9 +266,9 @@ describe('connected organization invitations', () => { const user = userEvent.setup(); await user.click(await screen.findByRole('button', { name: 'Manage ada@example.com' })); await user.click(screen.getByRole('menuitem', { name: 'Revoke invitation' })); - expect(await screen.findByRole('alertdialog')).toBeInTheDocument(); + expect(await screen.findByRole('dialog')).toBeInTheDocument(); await act(() => clerk.setActive({ session: 'sess_second' })); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); expect(fapi.organizationInvitations[0]?.status).toBe('pending'); }); @@ -297,11 +297,11 @@ describe('connected organization invitations', () => { const actor = userEvent.setup(); await actor.click(await screen.findByRole('button', { name: 'Manage ada@example.com' })); await actor.click(screen.getByRole('menuitem', { name: 'Revoke invitation' })); - expect(await screen.findByRole('alertdialog')).toBeInTheDocument(); + expect(await screen.findByRole('dialog')).toBeInTheDocument(); await act(() => clerk.setActive({ organization: otherOrganization.id })); expect(await screen.findByText('other@example.com')).toBeVisible(); expect(screen.queryByText('ada@example.com')).toBeNull(); - await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); }); it('ignores an old revoke completion after switching organizations', async () => { @@ -327,14 +327,14 @@ describe('connected organization invitations', () => { await actor.click(await screen.findByRole('button', { name: 'Manage ada@example.com' })); await actor.click(screen.getByRole('menuitem', { name: 'Revoke invitation' })); const revoke = holdRequests('post', '/v1/organizations/:organizationId/invitations/:invitationId/revoke'); - await actor.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Revoke invitation' })); + await actor.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Revoke invitation' })); await waitFor(() => expect(revoke.requests).toHaveLength(1)); await act(() => clerk.setActive({ organization: otherOrganization.id })); expect(await screen.findByText('other@example.com')).toBeVisible(); revoke.release(); await waitFor(() => expect(fapi.organizationInvitations[0]?.status).toBe('revoked')); expect(screen.getByText('other@example.com')).toBeVisible(); - expect(screen.queryByRole('alertdialog')).toBeNull(); + expect(screen.queryByRole('dialog')).toBeNull(); expect(screen.queryByText('ada@example.com')).toBeNull(); }); }); diff --git a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile-members-panel.feature.test.tsx b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile-members-panel.feature.test.tsx index 23c8af68c8f..8e9e7ded4e7 100644 --- a/packages/mosaic/src/features/organization-profile/__tests__/organization-profile-members-panel.feature.test.tsx +++ b/packages/mosaic/src/features/organization-profile/__tests__/organization-profile-members-panel.feature.test.tsx @@ -121,7 +121,7 @@ describe('OrganizationProfileMembersPanel', () => { members.release(); await user.click(await screen.findByRole('button', { name: 'Manage Bob Smith' })); await user.click(await screen.findByRole('menuitem', { name: 'Remove from organization' })); - const dialog = await screen.findByRole('alertdialog', { name: 'Remove Bob Smith?' }); + const dialog = await screen.findByRole('dialog', { name: 'Remove Bob Smith?' }); await user.click(within(dialog).getByRole('button', { name: 'Remove from organization' })); await waitFor(() => expect(fapi.memberships.some(item => item.id === bob.id)).toBe(false)); await waitFor(() => expect(screen.queryByText('Bob Smith')).toBeNull()); @@ -297,7 +297,7 @@ describe('OrganizationProfileMembersPanel', () => { await user.click(screen.getByRole('button', { name: 'Manage Bob Smith' })); await user.click(await screen.findByRole('menuitem', { name: 'Remove from organization' })); await user.click( - within(await screen.findByRole('alertdialog')).getByRole('button', { name: 'Remove from organization' }), + within(await screen.findByRole('dialog')).getByRole('button', { name: 'Remove from organization' }), ); await waitFor(() => expect(fapi.memberships.some(item => item.id === bob.id)).toBe(false)); expect(await screen.findByText('Extra0')).toBeVisible(); diff --git a/packages/mosaic/src/features/organization-profile/invitations-table-tab.controller.ts b/packages/mosaic/src/features/organization-profile/invitations-table-tab.controller.ts index ae92b62e214..d139c977aab 100644 --- a/packages/mosaic/src/features/organization-profile/invitations-table-tab.controller.ts +++ b/packages/mosaic/src/features/organization-profile/invitations-table-tab.controller.ts @@ -1,17 +1,38 @@ +import { useRef } from 'react'; + +import { useConfirmController } from '../../blocks/confirm'; +import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; import { useLocale, useMessages } from '../../localization'; import { formatDate } from '../../utils/format-date'; import type { useInvitationsTableModel } from './invitations-table-tab.model'; +import type { InvitationsTableTabViewProps, OrganizationProfileInvitation } from './invitations-table-tab.types'; -export function useInvitationsTableController(model: ReturnType) { +export function useInvitationsTableController( + model: ReturnType, +): InvitationsTableTabViewProps { const locale = useLocale(); const roleNames = useMessages('roles'); + const m = useMessages('invitationsTableTab'); + const invitations = model.rows.map(({ id, emailAddress, createdAt, role, roleName }) => ({ + id, + email: emailAddress, + invitedAtLabel: formatDate(createdAt, locale), + roleLabel: roleNames[role] ?? roleName, + })); + const inviteButtonRef = useRef(null); + const tableContainerRef = useRef(null); + const removalFocus = useListRemovalFocus({ + ids: invitations.map(invitation => invitation.id), + onRemove: model.revoke, + fallback: () => inviteButtonRef.current ?? tableContainerRef.current, + }); + const revoke = useConfirmController({ + onConfirm: invitation => removalFocus.remove(invitation.id), + errorFallback: m.revokeError, + }); + return { - invitations: model.rows.map(({ id, emailAddress, createdAt, role, roleName }) => ({ - id, - email: emailAddress, - invitedAtLabel: formatDate(createdAt, locale), - roleLabel: roleNames[role] ?? roleName, - })), + invitations, pageSize: model.pageSize, totalCount: model.totalCount, page: model.page, @@ -19,7 +40,11 @@ export function useInvitationsTableController(model: ReturnType void model.retry(), + inviteButtonRef, + tableContainerRef, + registerTrigger: removalFocus.registerTrigger, + revoke: model.revoke ? revoke : undefined, + finalFocus: removalFocus.finalFocus, }; } diff --git a/packages/mosaic/src/features/organization-profile/invitations-table-tab.types.ts b/packages/mosaic/src/features/organization-profile/invitations-table-tab.types.ts index 3a294795f9b..836a6e0ae21 100644 --- a/packages/mosaic/src/features/organization-profile/invitations-table-tab.types.ts +++ b/packages/mosaic/src/features/organization-profile/invitations-table-tab.types.ts @@ -1,4 +1,7 @@ -import type { MouseEventHandler } from 'react'; +import type { MouseEventHandler, Ref } from 'react'; + +import type { ConfirmController } from '../../blocks/confirm'; +import type { DialogFocusTarget } from '../../components/dialog'; export interface OrganizationProfileInvitation { id: string; @@ -18,7 +21,11 @@ export interface InvitationsTableTabViewProps { isError?: boolean; onRetry?: () => void; onPageChange: (page: number) => void; + inviteButtonRef?: Ref; + tableContainerRef?: Ref; onInvite?: MouseEventHandler; - onRevoke?: (id: string) => void | Promise; + registerTrigger?: (id: string) => Ref; + revoke?: ConfirmController; + finalFocus?: DialogFocusTarget; onBulkAction?: (ids: string[]) => void; } diff --git a/packages/mosaic/src/features/organization-profile/invitations-table-tab.view.tsx b/packages/mosaic/src/features/organization-profile/invitations-table-tab.view.tsx index 47e91e1c39d..d5c46986c2a 100644 --- a/packages/mosaic/src/features/organization-profile/invitations-table-tab.view.tsx +++ b/packages/mosaic/src/features/organization-profile/invitations-table-tab.view.tsx @@ -1,5 +1,5 @@ import * as stylex from '@stylexjs/stylex'; -import { type Ref, useMemo, useRef } from 'react'; +import type { Ref } from 'react'; import { Confirmation } from '../../blocks/confirmation'; import { ActionMenu } from '../../components/action-menu'; @@ -12,7 +12,6 @@ import { Pagination } from '../../components/pagination'; import { Spinner } from '../../components/spinner'; import { Table } from '../../components/table'; import { VisuallyHidden } from '../../components/visually-hidden'; -import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; import { useServerDataTable } from '../../hooks/use-server-data-table'; import { fill, useMessages } from '../../localization'; import { mergeStyleProps, themeProps } from '../../props'; @@ -23,8 +22,12 @@ const getRowId = (invitation: OrganizationProfileInvitation) => invitation.id; export function InvitationsTableTabView({ invitations, + inviteButtonRef, + tableContainerRef, onInvite, - onRevoke, + registerTrigger, + revoke, + finalFocus, totalCount, page, pageSize, @@ -36,14 +39,6 @@ export function InvitationsTableTabView({ onRetry, }: InvitationsTableTabViewProps) { const m = useMessages('invitationsTableTab'); - const inviteButton = useRef(null); - const tableContainer = useRef(null); - const removalFocus = useListRemovalFocus({ - ids: invitations.map(getRowId), - onRemove: onRevoke, - fallback: () => inviteButton.current ?? tableContainer.current, - }); - const revokeDialog = useMemo(() => Confirmation.createHandle(), []); const { table, pagination } = useServerDataTable({ data: invitations, totalCount, @@ -55,12 +50,12 @@ export function InvitationsTableTabView({ searchValue: '', onSearchChange: () => {}, }); - const columnCount = 3 + Number(Boolean(onRevoke)) + Number(Boolean(onBulkAction)); + const columnCount = 3 + Number(Boolean(revoke)) + Number(Boolean(onBulkAction)); return ( <>
- {onRevoke ? ( + {revoke ? ( fill(m.revokeTitle, { name: invitation.email })} description={m.revokeDescription} actionLabel={m.revoke} cancelLabel={m.cancel} - onConfirm={invitation => removalFocus.remove(invitation.id)} - errorFallback={m.revokeError} - finalFocus={removalFocus.finalFocus} + finalFocus={finalFocus} /> ) : null} @@ -159,7 +152,7 @@ function InvitationsTableBody({ onRetry?: () => void; hasBulkAction: boolean; onRevokeClick?: (invitation: OrganizationProfileInvitation) => void; - registerTrigger: (id: string) => Ref; + registerTrigger?: (id: string) => Ref; }) { const m = useMessages('invitationsTableTab'); if (isLoading) { @@ -245,7 +238,7 @@ function InvitationsTableBody({ , roles: MembersRoles | null) { const locale = useLocale(); const messages = useMessages('membersTableTab'); const roleNames = useMessages('roles'); const roleAction = usePendingAction({ errorFallback: messages.roleChangeError }); + const members = model.rows.map(({ joinedAt, ...member }) => ({ + ...member, + roleLabel: roleNames[member.role] ?? member.roleLabel, + joinedAtLabel: formatDate(joinedAt, locale), + })); + const searchInputRef = useRef(null); + const inviteButtonRef = useRef(null); + const removalFocus = useListRemovalFocus({ + ids: members.filter(canManageMember).map(member => member.id), + onRemove: model.remove, + fallback: () => inviteButtonRef.current ?? searchInputRef.current, + }); + const remove = useConfirmController({ + onConfirm: member => removalFocus.remove(member.id), + errorFallback: messages.removeError, + }); return { - members: model.rows.map(({ joinedAt, ...member }) => ({ - ...member, - roleLabel: roleNames[member.role] ?? member.roleLabel, - joinedAtLabel: formatDate(joinedAt, locale), - })), + members, roles: roles?.roles.map(role => ({ value: role.key, label: roleNames[role.key] ?? role.name })) ?? [], pageSize: model.pageSize, totalCount: model.totalCount, @@ -35,7 +50,11 @@ export function useMembersTableController(model: ReturnType void roleAction.run(id, () => model.changeRole?.(id, role, roles)) : undefined, - onRemove: model.remove, + searchInputRef, + inviteButtonRef, + registerTrigger: removalFocus.registerTrigger, + remove: model.remove ? remove : undefined, + finalFocus: removalFocus.finalFocus, }; } diff --git a/packages/mosaic/src/features/organization-profile/members-table-tab.types.ts b/packages/mosaic/src/features/organization-profile/members-table-tab.types.ts index 8276ce9ee9b..86729243c8e 100644 --- a/packages/mosaic/src/features/organization-profile/members-table-tab.types.ts +++ b/packages/mosaic/src/features/organization-profile/members-table-tab.types.ts @@ -1,5 +1,8 @@ import type { RoleResource } from '@clerk/shared/types'; -import type { MouseEventHandler } from 'react'; +import type { MouseEventHandler, Ref } from 'react'; + +import type { ConfirmController } from '../../blocks/confirm'; +import type { DialogFocusTarget } from '../../components/dialog'; export type MemberRole = Pick; @@ -37,8 +40,12 @@ export interface MembersTableTabViewProps { roleError?: string; onPageChange: (page: number) => void; onSearchChange: (value: string) => void; + searchInputRef?: Ref; + inviteButtonRef?: Ref; onInvite?: MouseEventHandler; - onRemove?: (id: string) => void | Promise; + registerTrigger?: (id: string) => Ref; + remove?: ConfirmController; + finalFocus?: DialogFocusTarget; onChangeRole?: (id: string, role: string) => void; onBulkAction?: (ids: string[]) => void; } diff --git a/packages/mosaic/src/features/organization-profile/members-table-tab.utils.ts b/packages/mosaic/src/features/organization-profile/members-table-tab.utils.ts new file mode 100644 index 00000000000..d41f8c27c17 --- /dev/null +++ b/packages/mosaic/src/features/organization-profile/members-table-tab.utils.ts @@ -0,0 +1,3 @@ +import type { OrganizationProfileMember } from './members-table-tab.types'; + +export const canManageMember = (member: OrganizationProfileMember) => !member.isCurrentUser && !member.isDeprovisioned; diff --git a/packages/mosaic/src/features/organization-profile/members-table-tab.view.tsx b/packages/mosaic/src/features/organization-profile/members-table-tab.view.tsx index ca31a4a76fc..65905aafdd7 100644 --- a/packages/mosaic/src/features/organization-profile/members-table-tab.view.tsx +++ b/packages/mosaic/src/features/organization-profile/members-table-tab.view.tsx @@ -1,5 +1,5 @@ import * as stylex from '@stylexjs/stylex'; -import { type Ref, useMemo, useRef } from 'react'; +import type { Ref } from 'react'; import { Confirmation } from '../../blocks/confirmation'; import { ActionMenu } from '../../components/action-menu'; @@ -15,22 +15,25 @@ import { Select } from '../../components/select'; import { Spinner } from '../../components/spinner'; import { Table } from '../../components/table'; import { VisuallyHidden } from '../../components/visually-hidden'; -import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; import { useServerDataTable } from '../../hooks/use-server-data-table'; import { fill, useMessages } from '../../localization'; import { mergeStyleProps, themeProps } from '../../props'; import { styles } from './members-table-tab.styles'; import type { MembersTableTabViewProps, OrganizationProfileMember } from './members-table-tab.types'; +import { canManageMember } from './members-table-tab.utils'; import { tableTabStyles } from './table-tab.styles'; const getRowId = (member: OrganizationProfileMember) => member.id; -const canManageMember = (member: OrganizationProfileMember) => !member.isCurrentUser && !member.isDeprovisioned; export function MembersTableTabView({ members, roles, + searchInputRef, + inviteButtonRef, onInvite, - onRemove, + registerTrigger, + remove, + finalFocus, onChangeRole, totalCount, page, @@ -48,14 +51,6 @@ export function MembersTableTabView({ roleError, }: MembersTableTabViewProps) { const m = useMessages('membersTableTab'); - const searchInput = useRef(null); - const inviteButton = useRef(null); - const removalFocus = useListRemovalFocus({ - ids: members.filter(canManageMember).map(getRowId), - onRemove, - fallback: () => inviteButton.current ?? searchInput.current, - }); - const removeDialog = useMemo(() => Confirmation.createHandle(), []); const { table, pagination } = useServerDataTable({ data: members, totalCount, @@ -68,14 +63,23 @@ export function MembersTableTabView({ searchValue, onSearchChange, }); - const columnCount = 3 + Number(Boolean(onRemove)) + Number(Boolean(onBulkAction)); + const columnCount = 3 + Number(Boolean(remove)) + Number(Boolean(onBulkAction)); const query = searchValue.trim(); + const removeAndClearSelection = remove + ? { + ...remove, + confirm: (member: OrganizationProfileMember) => { + table.setRowSelection({}); + remove.confirm(member); + }, + } + : undefined; return ( <>
{onInvite ? (
- {onRemove ? ( + {removeAndClearSelection ? ( fill(m.removeTitle, { name: member.name })} description={m.removeDescription} actionLabel={m.remove} cancelLabel={m.cancel} - onConfirm={member => { - table.setRowSelection({}); - return removalFocus.remove(member.id); - }} - errorFallback={m.removeError} - finalFocus={removalFocus.finalFocus} + finalFocus={finalFocus} /> ) : null} @@ -215,7 +214,7 @@ function MembersTableBody({ onChangeRole?: MembersTableTabViewProps['onChangeRole']; isChangingRole: boolean; onRemoveClick?: (member: OrganizationProfileMember) => void; - registerTrigger: (id: string) => Ref; + registerTrigger?: (id: string) => Ref; }) { const m = useMessages('membersTableTab'); if (isLoading) { @@ -274,7 +273,7 @@ function MembersTableBody({ onChangeRole={onChangeRole} isChangingRole={isChangingRole} onRemoveClick={onRemoveClick ? () => onRemoveClick(row.original) : undefined} - triggerRef={registerTrigger(row.original.id)} + triggerRef={registerTrigger?.(row.original.id)} /> ))} @@ -298,7 +297,7 @@ function MemberRow({ onChangeRole?: MembersTableTabViewProps['onChangeRole']; isChangingRole: boolean; onRemoveClick?: () => void; - triggerRef: Ref; + triggerRef?: Ref; }) { const m = useMessages('membersTableTab'); const manageable = canManageMember(member); diff --git a/packages/mosaic/src/features/organization-profile/requests-table-tab.controller.ts b/packages/mosaic/src/features/organization-profile/requests-table-tab.controller.ts index a9063bd863e..50fc5193c61 100644 --- a/packages/mosaic/src/features/organization-profile/requests-table-tab.controller.ts +++ b/packages/mosaic/src/features/organization-profile/requests-table-tab.controller.ts @@ -1,26 +1,52 @@ +import { useRef } from 'react'; + +import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; import { useLocale } from '../../localization'; import { formatDate } from '../../utils/format-date'; import type { useRequestsTableModel } from './requests-table-tab.model'; +import type { RequestsTableTabViewProps } from './requests-table-tab.types'; -export function useRequestsTableController(model: ReturnType) { +export function useRequestsTableController(model: ReturnType): RequestsTableTabViewProps { const locale = useLocale(); + const requests = model.rows.map(({ id, identifier, name, imageUrl, createdAt }) => ({ + id, + email: identifier, + name: name || undefined, + imageUrl, + requestedAtLabel: formatDate(createdAt, locale), + })); + const inviteButtonRef = useRef(null); + const tableContainerRef = useRef(null); + const ids = requests.map(request => request.id); + const fallback = () => inviteButtonRef.current ?? tableContainerRef.current; + const acceptFocus = useListRemovalFocus({ ids, onRemove: model.accept, fallback }); + const declineFocus = useListRemovalFocus({ ids, onRemove: model.reject, fallback }); + return { - requests: model.rows.map(({ id, identifier, name, imageUrl, createdAt }) => ({ - id, - email: identifier, - name: name || undefined, - imageUrl, - requestedAtLabel: formatDate(createdAt, locale), - })), + requests, pageSize: model.pageSize, totalCount: model.totalCount, page: model.page, isLoading: model.isLoading, isFetching: model.isFetching, isError: model.isError, - onRetry: model.retry, + onRetry: () => void model.retry(), onPageChange: model.fetchPage, - onAccept: model.accept, - onDecline: model.reject, + inviteButtonRef, + tableContainerRef, + registerAcceptTrigger: acceptFocus.registerTrigger, + registerDeclineTrigger: declineFocus.registerTrigger, + onAccept: model.accept + ? async id => { + await acceptFocus.remove(id); + acceptFocus.finalFocus()?.focus(); + } + : undefined, + onDecline: model.reject + ? async id => { + await declineFocus.remove(id); + declineFocus.finalFocus()?.focus(); + } + : undefined, }; } diff --git a/packages/mosaic/src/features/organization-profile/requests-table-tab.types.ts b/packages/mosaic/src/features/organization-profile/requests-table-tab.types.ts index 9a8b7165591..3669d7be0d3 100644 --- a/packages/mosaic/src/features/organization-profile/requests-table-tab.types.ts +++ b/packages/mosaic/src/features/organization-profile/requests-table-tab.types.ts @@ -1,4 +1,4 @@ -import type { MouseEventHandler } from 'react'; +import type { MouseEventHandler, Ref } from 'react'; export interface OrganizationProfileRequest { id: string; @@ -19,7 +19,11 @@ export interface RequestsTableTabViewProps { isError?: boolean; onRetry?: () => void; onPageChange: (page: number) => void; + inviteButtonRef?: Ref; + tableContainerRef?: Ref; onInvite?: MouseEventHandler; + registerAcceptTrigger?: (id: string) => Ref; + registerDeclineTrigger?: (id: string) => Ref; onAccept?: (id: string) => void | Promise; onDecline?: (id: string) => void | Promise; onBulkAction?: (ids: string[]) => void; diff --git a/packages/mosaic/src/features/organization-profile/requests-table-tab.view.tsx b/packages/mosaic/src/features/organization-profile/requests-table-tab.view.tsx index 0c9eca839d2..e527109a9e6 100644 --- a/packages/mosaic/src/features/organization-profile/requests-table-tab.view.tsx +++ b/packages/mosaic/src/features/organization-profile/requests-table-tab.view.tsx @@ -1,5 +1,6 @@ import * as stylex from '@stylexjs/stylex'; -import { useRef, useState } from 'react'; +import type { Ref } from 'react'; +import { useState } from 'react'; import { Avatar } from '../../components/avatar'; import { Button, SubmitButton } from '../../components/button'; @@ -11,7 +12,6 @@ import { Section } from '../../components/section'; import { Spinner } from '../../components/spinner'; import { Table } from '../../components/table'; import { VisuallyHidden } from '../../components/visually-hidden'; -import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../hooks/use-pending-action'; import { useServerDataTable } from '../../hooks/use-server-data-table'; import { fill, useMessages } from '../../localization'; @@ -24,7 +24,11 @@ const getRowId = (request: OrganizationProfileRequest) => request.id; export function RequestsTableTabView({ requests, + inviteButtonRef, + tableContainerRef, onInvite, + registerAcceptTrigger, + registerDeclineTrigger, onAccept, onDecline, totalCount, @@ -38,18 +42,6 @@ export function RequestsTableTabView({ onRetry, }: RequestsTableTabViewProps) { const m = useMessages('requestsTableTab'); - const inviteButton = useRef(null); - const tableContainer = useRef(null); - const acceptFocus = useListRemovalFocus({ - ids: requests.map(getRowId), - onRemove: onAccept, - fallback: () => inviteButton.current ?? tableContainer.current, - }); - const declineFocus = useListRemovalFocus({ - ids: requests.map(getRowId), - onRemove: onDecline, - fallback: () => inviteButton.current ?? tableContainer.current, - }); const { table, pagination } = useServerDataTable({ data: requests, totalCount, @@ -66,7 +58,7 @@ export function RequestsTableTabView({ return (