diff --git a/.changeset/friendly-identifier-lock.md b/.changeset/friendly-identifier-lock.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/friendly-identifier-lock.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.changeset/goofy-parents-trade.md b/.changeset/goofy-parents-trade.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/goofy-parents-trade.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index a8689147e98..285efa27ecf 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -66,6 +66,8 @@ Most problems a feature hits already have a shared answer. Find the row for your | Deciding which part renders `Card.Root` | Only the surface, never a screen or `*Steps`, see [Views](docs/views.md) | | An interaction with an async lifecycle or two values that move together | A machine in the controller file, via `setup()` (`src/machine/`) | | Styles several components share | `src/styles/*.styles.ts` | +| Direct actions and confirmation dialogs that must exclude each other | `usePendingAction.guard` on the confirmation (`src/hooks/`, `src/blocks/confirm/`) | +| A machine-driven step that a lock or caller must await | `waitFor` (`src/machine/wait-for.ts`), inside `usePendingAction.run` | ## Signs you are about to invent a pattern diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index 94f05e37503..ec7dde64fb2 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -45,17 +45,52 @@ It owns the error (see "Errors"): const actions = usePendingAction({ errorFallback: messages.errors.generic }); return { - rows: rows.map(row => (actions.errorKey === row.id ? { ...row, error: actions.error } : row)), - pendingId: actions.pendingKey, + rows: rows.map(row => ({ + ...row, + isPending: actions.pendingKey === row.id, + error: actions.errorKey === row.id ? actions.error : undefined, + })), onRemove: id => void actions.run(id, () => onRemove(id)), }; ``` +When a direct action and a confirmation must not run together, give both +operations one `usePendingAction` owner: + +```ts +const actions = usePendingAction(); +const remove = useConfirmController({ + onConfirm: contact => removalFocus.remove(contact.id), +}); + +return { + contacts: contacts.map(contact => ({ ...contact, isPrimaryPending: actions.pendingKey === contact.id })), + isDisabled: actions.isPending, + remove: actions.guard(remove), +}; +``` + +Keep this coordination in the section controller. Run direct actions with +`actions.run(key, action)` and pass the confirmation through `actions.guard`, which +runs its `confirm` under the same lock. Pass `actions.isPending` to the section +controls' `disabled` props. + +`confirm` resolves `true` when the operation succeeds, or `false` when it fails or +the dialog ignores the call. Its promise follows the operation even if the +dialog closes during cancellation. The shared pending owner waits for that +promise, so closing the dialog does not enable another action before cancellation +settles. Removal keeps its dialog errors, and inline row errors and focus keys +stay unchanged. Put each row's pending state on the row the controller returns, +so views render it without comparing pending keys. + `run` resolves `true` on success, so the caller can close a dialog or move focus. To hold pending through an OAuth redirect, await a delay inside the -action (`user-profile-connected-accounts-section.controller.ts`). When the user -cancels a reverification challenge, `run` resolves `false` without showing an -error and releases the lock. It is not for: +action (`user-profile-connected-accounts-section.controller.ts`). To hold it +through a step a machine drives, run an action that awaits `waitFor` +(`machine/wait-for.ts`) until the machine leaves that state +(`user-profile-add-email.controller.ts`). When the user cancels a +reverification challenge, `run` resolves `false` without showing an error and +releases the lock. It is not for: - a form → `useForm` - a confirm step → `useConfirmController` with `Confirmation` / `Destructive` diff --git a/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts index e6ec9447bfa..0d9d554bcb4 100644 --- a/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts +++ b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts @@ -91,6 +91,45 @@ describe('confirmMachine', () => { }); describe('useConfirmController', () => { + it.each(['resolve', 'reject'])( + 'keeps its completion pending after cancellation until the action can %s', + async outcome => { + const pending = deferred(); + const { result } = renderHook(() => + useConfirmController({ + onConfirm: () => pending.promise, + reverification: { status: 'loading', visible: true, onCancel: vi.fn() }, + }), + ); + const completed = vi.fn(); + act(() => result.current.onOpenChange(true)); + let completion: ReturnType | undefined; + act(() => { + completion = result.current.confirm(); + }); + expect(completion).toBeInstanceOf(Promise); + void completion?.then(completed); + act(() => result.current.onOpenChange(false)); + await act(async () => { + await Promise.resolve(); + }); + expect(result.current.isOpen).toBe(false); + expect(completed).not.toHaveBeenCalled(); + + await act(async () => { + if (outcome === 'reject') { + pending.reject(cancelled); + } else { + pending.resolve(); + } + await completion; + }); + + expect(completed).toHaveBeenCalledWith(outcome === 'resolve'); + expect(result.current.errorMessage).toBeUndefined(); + }, + ); + 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; @@ -107,17 +146,23 @@ describe('useConfirmController', () => { const onConfirm = vi.fn(() => pending.promise); const { result } = renderHook(() => useConfirmController({ onConfirm })); - act(() => result.current.confirm()); + await act(async () => expect(await result.current.confirm()).toBe(false)); expect(onConfirm).not.toHaveBeenCalled(); act(() => result.current.onOpenChange(true)); - act(() => result.current.confirm()); - act(() => result.current.confirm()); + const confirm = result.current.confirm; + let completion: ReturnType | undefined; + let duplicate: ReturnType | undefined; + act(() => { + completion = confirm(); + duplicate = confirm(); + }); expect(onConfirm).toHaveBeenCalledOnce(); + expect(await duplicate).toBe(false); await act(async () => { pending.resolve(); - await pending.promise; + expect(await completion).toBe(true); }); 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 index 9c8b094194e..40205499858 100644 --- a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx +++ b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx @@ -69,7 +69,7 @@ function ConfirmCard({ cancelLabel, isPending: controller.isPending, errorMessage: controller.errorMessage, - confirm: () => controller.confirm(payload), + confirm: () => void controller.confirm(payload), }); return ( diff --git a/packages/mosaic/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts index 6924f103b5b..bb11b314719 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -80,8 +80,8 @@ export interface ConfirmController { /** 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; + /** Runs the action for `payload`. The dialog closes when it resolves and shows why when it rejects. Resolves whether the action succeeded. */ + confirm: (payload: Payload) => Promise; } export interface ConfirmControllerOptions { @@ -136,15 +136,21 @@ export function useConfirmController({ send({ type: 'CANCEL' }); }, confirm: payload => { - if (snapshot.value !== 'confirming') { - return; - } + let completion: Promise | undefined; send({ type: 'CONFIRM', - run: async () => { - await onConfirm(payload); + run: () => { + const operation = (async () => { + await onConfirm(payload); + })(); + completion = operation.then( + () => true, + () => false, + ); + return operation; }, }); + return completion ?? Promise.resolve(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 index 2de465ee6ff..74d87837077 100644 --- a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx +++ b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx @@ -82,6 +82,7 @@ describe('Confirmation', () => { await user.click(confirmButton()); expect(onConfirm).toHaveBeenCalledOnce(); expect(confirmButton()).toHaveAttribute('aria-busy', 'true'); + expect(confirmButton()).toHaveFocus(); run.resolve(); await closed(); diff --git a/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts index 40b9b1f7df8..e0a051d3dcf 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts @@ -84,7 +84,7 @@ export function useUserProfileActiveDevicesController({ registerTrigger: removalFocus.registerTrigger, finalFocus: removalFocus.finalFocus, details, - signOutFromDetails: revoke ? device => details.confirm(device) : undefined, + signOutFromDetails: revoke ? device => void details.confirm(device) : undefined, signOut: revoke ? signOut : undefined, signOutAll: revokeAll ? signOutAll : undefined, // Confirming removes the other devices and the trigger with them — hence the dialog mounted diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/__tests__/user-profile-connected-accounts.connect.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/__tests__/user-profile-connected-accounts.connect.feature.test.tsx index 5a61178dc54..2bc90bef87f 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/__tests__/user-profile-connected-accounts.connect.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/__tests__/user-profile-connected-accounts.connect.feature.test.tsx @@ -158,6 +158,27 @@ describe('connected accounts', () => { request.fail(); }); + it('holds connected account menus while a connection starts', async () => { + await renderSection([google]); + const request = holdRequests('post', '/v1/me/external_accounts'); + const user = userEvent.setup(); + + try { + await user.click(screen.getByRole('button', { name: 'Connect GitHub' })); + await waitFor(() => expect(request.requests).toHaveLength(1)); + const manage = screen.getByRole('button', { name: 'Manage Google' }); + expect(manage).toHaveAttribute('aria-disabled', 'true'); + await user.click(manage); + expect(screen.queryByRole('menu')).toBeNull(); + } finally { + request.fail(); + } + + await waitFor(() => + expect(screen.getByRole('button', { name: 'Manage Google' })).not.toHaveAttribute('aria-disabled', 'true'), + ); + }); + it('verifies the session before connecting a provider, then continues to it', async () => { serveFapi(signedIn([], { verification: { secrets: { password: 'hunter2' } } })); const view = await renderWithClerk(); diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-account-row.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-account-row.view.tsx index 75288f5e092..1c493dd8110 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-account-row.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-account-row.view.tsx @@ -19,13 +19,11 @@ import type { export function UserProfileConnectProviderRowView({ provider, - isPending, isDisabled, onConnect, triggerRef, }: { provider: UserProfileConnectionProvider; - isPending: boolean; isDisabled: boolean; onConnect: (id: string) => void; triggerRef?: Ref; @@ -58,8 +56,8 @@ export function UserProfileConnectProviderRowView({ variant='outline' color='neutral' aria-label={fill(m.connectLabel, { provider: provider.provider })} - isPending={isPending} - disabled={isDisabled && !isPending} + isPending={provider.isPending} + disabled={isDisabled && !provider.isPending} onClick={() => onConnect(provider.id)} > {m.connect} @@ -91,7 +89,7 @@ export function UserProfileConnectedAccountRowView({ }) { const m = useMessages('userProfile').connectedAccounts; const actions: ActionMenuAction[] = []; - if ((account.status === 'reconnect' || account.status === 'error') && onReconnect && !isDisabled) { + if ((account.status === 'reconnect' || account.status === 'error') && onReconnect) { actions.push({ label: account.status === 'reconnect' ? m.reconnect : m.tryAgain, onClick: () => onReconnect(account.id), @@ -131,6 +129,8 @@ export function UserProfileConnectedAccountRowView({ {actions.length > 0 ? ( void; onReconnect: (id: string) => void; sectionRef: Ref; @@ -81,15 +81,17 @@ export function useUserProfileConnectedAccountsController({ accounts: accounts.map(account => actions.errorKey === account.id ? { ...account, reconnectError: actions.error } : account, ), - availableProviders: availableProviders.map(provider => - actions.errorKey === provider.id ? { ...provider, connectError: actions.error } : provider, - ), - pendingId: actions.pendingKey, + availableProviders: availableProviders.map(provider => ({ + ...provider, + isPending: actions.pendingKey === provider.id, + connectError: actions.errorKey === provider.id ? actions.error : undefined, + })), + isDisabled: actions.isPending, onConnect: id => run(id, onConnect), onReconnect: id => run(id, onReconnect), sectionRef, registerTrigger: removalFocus.registerTrigger, - remove: onRemove ? remove : undefined, + remove: onRemove ? actions.guard(remove) : undefined, finalFocus: removalFocus.finalFocus, reconnectReverification, reconnectFinalFocus: () => (actions.lastKey ? removalFocus.trigger(actions.lastKey) : null), 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 bfd25079a0b..5d8a2e0b891 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 @@ -16,6 +16,7 @@ export interface ConnectedAccountProviderDisplay { export interface UserProfileConnectionProvider extends ConnectedAccountProviderDisplay { id: string; connectError?: string; + isPending?: boolean; } export interface UserProfileConnectedAccount extends ConnectedAccountProviderDisplay { @@ -30,7 +31,7 @@ export interface UserProfileConnectedAccountsSectionViewProps { accounts: UserProfileConnectedAccount[]; sectionRef?: Ref; availableProviders?: UserProfileConnectionProvider[]; - pendingId?: string; + isDisabled?: boolean; onConnect?: (id: string) => void; onReconnect?: (id: string) => void; registerTrigger: (id: string) => Ref; 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 0ce9ea67d93..a8fe2660ba4 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 @@ -18,7 +18,7 @@ export function UserProfileConnectedAccountsSectionView({ accounts, sectionRef, availableProviders = [], - pendingId, + isDisabled = false, onConnect, onReconnect, registerTrigger, @@ -31,7 +31,6 @@ export function UserProfileConnectedAccountsSectionView({ }: UserProfileConnectedAccountsSectionViewProps) { const m = useMessages('userProfile').connectedAccounts; const hasRows = accounts.length > 0 || (availableProviders.length > 0 && Boolean(onConnect)); - const isBusy = pendingId !== undefined; return ( <> @@ -52,7 +51,7 @@ export function UserProfileConnectedAccountsSectionView({ key={account.id} account={account} triggerRef={registerTrigger(account.id)} - isDisabled={isBusy} + isDisabled={isDisabled} onReconnect={onReconnect} onRemove={remove ? account => remove.open(account) : undefined} /> @@ -63,8 +62,7 @@ export function UserProfileConnectedAccountsSectionView({ key={provider.id} provider={provider} triggerRef={registerTrigger(provider.id)} - isPending={pendingId === provider.id} - isDisabled={isBusy} + isDisabled={isDisabled} onConnect={onConnect} /> )) diff --git a/packages/mosaic/src/features/user-profile/user-profile-contact-list-row.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-contact-list-row.view.tsx index 84098de9650..f45b3c99a6e 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-contact-list-row.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-contact-list-row.view.tsx @@ -10,7 +10,7 @@ import { Section } from '../../components/section'; import { Plus } from '../../icons/glyphs/plus'; import { fill, useMessages } from '../../localization'; import { truncationStyles } from '../../styles/typography.styles'; -import type { UserProfileContact } from './user-profile-contact.types'; +import type { UserProfileContact, UserProfileContactRow } from './user-profile-contact.types'; import { styles } from './user-profile-profile-panel.styles'; export interface UserProfileContactListRowViewProps { @@ -19,10 +19,10 @@ export interface UserProfileContactListRowViewProps { addAction?: ReactNode; kind: 'email' | 'phone'; label: string; - items: UserProfileContact[]; + items: UserProfileContactRow[]; onAdd?: () => void; onVerify?: (id: string) => void; - pendingVerifyId?: string; + isDisabled?: boolean; onSetPrimary?: (id: string) => void; onRemove?: (item: UserProfileContact) => void; children?: ReactNode; @@ -34,7 +34,7 @@ export function UserProfileContactListRowView({ items, onAdd, onVerify, - pendingVerifyId, + isDisabled = false, onSetPrimary, onRemove, addAction, @@ -65,6 +65,7 @@ export function UserProfileContactListRowView({ size='sm' variant='outline' onClick={onAdd} + disabled={isDisabled} > onVerify(item.id), - status: pendingVerifyId === item.id ? 'pending' : 'idle', + status: item.isVerifyPending ? 'pending' : 'idle', }); } else if (item.isVerified && !item.isDefault && onSetPrimary) { - actions.push({ label: m.setPrimary, onClick: () => onSetPrimary(item.id) }); + actions.push({ + label: m.setPrimary, + onClick: () => onSetPrimary(item.id), + status: item.isPrimaryPending ? 'pending' : 'idle', + }); } if (onRemove) { @@ -120,6 +125,8 @@ export function UserProfileContactListRowView({ diff --git a/packages/mosaic/src/features/user-profile/user-profile-contact.types.ts b/packages/mosaic/src/features/user-profile/user-profile-contact.types.ts index 1f178b0f450..80126c87925 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-contact.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-contact.types.ts @@ -5,6 +5,11 @@ export interface UserProfileContact { isVerified: boolean; } +export interface UserProfileContactRow extends UserProfileContact { + isVerifyPending: boolean; + isPrimaryPending: boolean; +} + export type UserProfileEmail = UserProfileContact; export type UserProfilePhone = UserProfileContact; diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-add-email.controller.test.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-add-email.controller.test.ts index 1f38a4fa3d1..9129cf5d379 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-add-email.controller.test.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-add-email.controller.test.ts @@ -200,7 +200,7 @@ describe('useUserProfileAddEmailController', () => { useUserProfileAddEmailController({ reverification: idleReverification, onCreate }), ); - act(() => result.current.onVerifyEmail(other, email)); + act(() => void result.current.onVerifyEmail(other, email)); expect(email.start).toHaveBeenCalledOnce(); expect(result.current.open).toBe(false); expect(result.current.pendingVerifyId).toBe('email_2'); @@ -227,11 +227,11 @@ describe('useUserProfileAddEmailController', () => { ); expect(result.current.verifyingId).toBeUndefined(); - act(() => result.current.onVerifyEmail(other, email)); + act(() => void result.current.onVerifyEmail(other, email)); expect(result.current.verifyingId).toBe('email_2'); await waitFor(() => expect(result.current.step).toBe('verify')); - act(() => result.current.onVerifyEmail({ id: 'email_3', value: 'person@acme.co' }, email)); + act(() => void result.current.onVerifyEmail({ id: 'email_3', value: 'person@acme.co' }, email)); expect(result.current.verifyingId).toBe('email_2'); expect(result.current.emailAddress).toBe('other@example.com'); @@ -253,7 +253,7 @@ describe('useUserProfileAddEmailController', () => { useUserProfileAddEmailController({ reverification: idleReverification, onCreate: created }), ); - act(() => result.current.onVerifyEmail(other, email)); + act(() => void result.current.onVerifyEmail(other, email)); await waitFor(() => expect(result.current.errorMessage).toBe('Too many requests')); expect(result.current.step).toBe('verify'); expect(result.current.resendSeconds).toBe(0); @@ -279,7 +279,7 @@ describe('useUserProfileAddEmailController', () => { const verified = createDeferredPromise(); const { result } = renderHook(() => useUserProfileAddEmailController({ reverification: idleReverification })); - act(() => result.current.onVerifyEmail(other, verifier(sentLink(verified.promise.then(() => undefined))))); + act(() => void result.current.onVerifyEmail(other, verifier(sentLink(verified.promise.then(() => undefined))))); expect(result.current.step).toBe('link'); expect(result.current.resendSeconds).toBe(60); @@ -297,12 +297,12 @@ describe('useUserProfileAddEmailController', () => { useUserProfileAddEmailController({ reverification: idleReverification }), ); - act(() => result.current.onVerifyEmail(other, email)); + act(() => void result.current.onVerifyEmail(other, email)); await waitFor(() => expect(result.current.step).toBe('link')); act(() => result.current.onOpenChange(false)); expect(cancel).toHaveBeenCalledOnce(); - act(() => result.current.onVerifyEmail(other, email)); + act(() => void result.current.onVerifyEmail(other, email)); await waitFor(() => expect(result.current.step).toBe('link')); unmount(); expect(cancel).toHaveBeenCalledTimes(2); @@ -312,7 +312,7 @@ describe('useUserProfileAddEmailController', () => { const email = verifier(sentLink(Promise.reject(saveError('Link expired')))); const { result } = renderHook(() => useUserProfileAddEmailController({ reverification: idleReverification })); - act(() => result.current.onVerifyEmail(other, email)); + act(() => void result.current.onVerifyEmail(other, email)); await waitFor(() => expect(result.current.errorMessage).toBe('Link expired')); expect(result.current.step).toBe('link'); expect(result.current.open).toBe(true); @@ -323,7 +323,7 @@ describe('useUserProfileAddEmailController', () => { const email = verifier(() => ({ method: 'sso', connect })); const { result } = renderHook(() => useUserProfileAddEmailController({ reverification: idleReverification })); - act(() => result.current.onVerifyEmail({ id: 'email_3', value: 'person@acme.co' }, email)); + act(() => void result.current.onVerifyEmail({ id: 'email_3', value: 'person@acme.co' }, email)); expect(result.current.step).toBe('sso'); expect(result.current.resendSeconds).toBe(0); diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx index b9185a45480..91bda957638 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx @@ -72,6 +72,36 @@ async function enterCode(actor: Actor, code: string) { } describe('the user profile email section', () => { + it('holds the menu and row order while setting an address as primary, then returns focus', async () => { + const other = fapiEmailAddress({ id: 'idn_other', email_address: 'bob@example.com', verification: VERIFIED }); + const { actor } = await renderSection(signedInWithEmails([PRIMARY, other])); + const primary = holdRequests('post', '/v1/me'); + try { + await manageEmail(actor, 'bob@example.com', 'Set as primary'); + await waitFor(() => expect(primary.requests).toHaveLength(1)); + const menu = screen.getByRole('menu'); + const item = within(menu).getByRole('menuitem', { name: 'Set as primary' }); + expect(item).toHaveAttribute('aria-busy', 'true'); + const manage = screen.getByRole('button', { name: 'Manage bob@example.com' }); + expect(manage).toHaveAttribute('aria-disabled', 'true'); + expect(within(menu).getByRole('menuitem', { name: 'Remove email' })).toHaveAttribute('aria-disabled', 'true'); + expect(screen.queryByRole('alertdialog')).toBeNull(); + expect(screen.getByRole('button', { name: 'Add email' })).toBeDisabled(); + expect(await within(item).findByRole('progressbar', { name: 'Set as primary' })).toBeVisible(); + expect(emailsListed()).toEqual(['alice@example.com', 'bob@example.com']); + } finally { + primary.release(); + await waitFor(() => + expect(within(emailRow()).getByText('bob@example.com').parentElement).toHaveTextContent('Primary'), + ); + } + expect(screen.getByRole('button', { name: 'Manage bob@example.com' })).not.toHaveAttribute('aria-disabled', 'true'); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + expect(emailsListed()).toEqual(['alice@example.com', 'bob@example.com']); + expect(screen.getByRole('button', { name: 'Manage bob@example.com' })).toHaveFocus(); + expect(screen.queryByRole('progressbar', { name: 'Set as primary' })).toBeNull(); + }); + it('adds an address and verifies the code it was sent', async () => { const { actor } = await renderSection(signedInWithEmails([])); expect(emailRow()).toHaveTextContent('No email addresses added'); @@ -145,12 +175,18 @@ describe('the user profile email section', () => { expect(screen.getByRole('menuitem', { name: 'Verify' })).toHaveAttribute('aria-busy', 'true'); expect(await screen.findByRole('progressbar', { name: 'Verify' })).toBeVisible(); expect(screen.queryByRole('dialog')).toBeNull(); + expect(screen.getByRole('button', { name: 'Manage alice@example.com' })).toHaveAttribute('aria-disabled', 'true'); + expect(screen.getByRole('button', { name: 'Add email' })).toBeDisabled(); } finally { prepare.release(); } await enterCode(actor, VERIFICATION_CODE); expect(screen.queryByRole('menu')).toBeNull(); + expect(screen.getByRole('button', { name: 'Manage alice@example.com' })).not.toHaveAttribute( + 'aria-disabled', + 'true', + ); await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); }); @@ -360,6 +396,7 @@ describe('email reverification', () => { await manageEmail(actor, 'bob@example.com', 'Set as primary'); expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); + await waitFor(() => expect(screen.queryByRole('menu', { hidden: true })).toBeNull()); await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus()); await actor.keyboard('hunter2{Enter}'); @@ -386,6 +423,7 @@ describe('email reverification', () => { expect(within(emailRow()).queryByRole('alert')).toBeNull(); expect(within(emailRow()).getByRole('button', { name: 'Manage bob@example.com' })).toHaveFocus(); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); await manageEmail(actor, 'bob@example.com', 'Set as primary'); await waitFor(() => expect(fapi.client.sessions[0]?.user?.primary_email_address_id).toBe('idn_second')); }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.controller.ts index be357744086..87048755702 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.controller.ts @@ -7,6 +7,7 @@ import { useErrorText } from '../../../localization'; import { setup } from '../../../machine/setup'; import type { ErrorInvokeEvent } from '../../../machine/types'; import { useMachine } from '../../../machine/use-machine'; +import { waitFor } from '../../../machine/wait-for'; import type { FormError } from '../../../utils/errors'; import { toFormError } from '../../../utils/errors'; import { CODE_LENGTH, CODE_RESEND_SECONDS } from '../../../utils/verification'; @@ -31,7 +32,7 @@ export interface UserProfileAddEmailController extends UserProfileAddEmailDialog handle: DialogHandle; verifyingId: string | undefined; pendingVerifyId: string | undefined; - onVerifyEmail: (email: Pick, verifier: UserProfileEmailVerifier) => void; + onVerifyEmail: (email: Pick, verifier: UserProfileEmailVerifier) => Promise; } interface Context { @@ -266,9 +267,10 @@ export function useUserProfileAddEmailController({ onConnect: () => send({ type: 'CONNECT' }), verifyingId: snapshot.context.verifyingId, pendingVerifyId: state === 'preparing' ? snapshot.context.verifyingId : undefined, - onVerifyEmail: (email, verifier) => { + onVerifyEmail: async (email, verifier) => { codeForm.reset(); send({ type: 'START', id: email.id, emailAddress: email.value, verifier }); + await waitFor(actor, current => current.value !== 'preparing'); }, }; } 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 822bdeca9b9..79abc2f965b 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,14 +1,15 @@ -import { useRef } from 'react'; +import { useRef, useState } from 'react'; import { useConfirmController } from '../../../blocks/confirm'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; +import { applyOrder } from '../../../utils/apply-order'; 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'; export function useUserProfileEmailSectionController({ - emails, + emails: modelEmails, username, onCreateEmail, getEmailVerifier, @@ -18,12 +19,14 @@ export function useUserProfileEmailSectionController({ setPrimaryReverification, createReverification, }: Omit): UserProfileEmailSectionViewProps { + const [order] = useState(() => modelEmails.map(email => email.id)); + const emails = applyOrder(order, modelEmails, email => email.id); const verification = useUserProfileAddEmailController({ username, onCreate: onCreateEmail, reverification: createReverification, }); - const setPrimary = usePendingAction(); + const actions = usePendingAction(); const rowRef = useRef(null); const removalFocus = useListRemovalFocus({ ids: emails.map(email => email.id), @@ -36,23 +39,28 @@ export function useUserProfileEmailSectionController({ }); return { - emails, + emails: emails.map(email => ({ + ...email, + isVerifyPending: verification.pendingVerifyId === email.id, + isPrimaryPending: actions.pendingKey === email.id && !setPrimaryReverification.visible, + })), canAdd: Boolean(onCreateEmail), verification, - error: setPrimary.error, + error: actions.error, + isDisabled: actions.isPending, rowRef, registerTrigger: removalFocus.registerTrigger, - remove: onRemoveEmail ? remove : undefined, + remove: onRemoveEmail ? actions.guard(remove) : undefined, finalFocus: removalFocus.finalFocus, verificationFinalFocus: () => (verification.verifyingId ? removalFocus.trigger(verification.verifyingId) : null), setPrimaryReverification, - setPrimaryFinalFocus: () => (setPrimary.lastKey ? removalFocus.trigger(setPrimary.lastKey) : null), + setPrimaryFinalFocus: () => (actions.lastKey ? removalFocus.trigger(actions.lastKey) : null), onVerify: id => { const email = emails.find(email => email.id === id); if (email) { - verification.onVerifyEmail(email, getEmailVerifier(id)); + void actions.run(id, () => verification.onVerifyEmail(email, getEmailVerifier(id))); } }, - onSetPrimary: id => void setPrimary.run(id, () => onSetPrimaryEmail(id)), + onSetPrimary: id => void actions.run(id, () => onSetPrimaryEmail(id)), }; } 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 1b4985b0644..222554f844b 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 @@ -3,12 +3,13 @@ import type { Ref } from 'react'; import type { ConfirmController } from '../../../blocks/confirm'; import type { DialogFocusTarget } from '../../../components/dialog'; import type { ReverificationController } from '../../reverification'; -import type { UserProfileEmail, UserProfileEmailVerifier } from '../user-profile-contact.types'; +import type { UserProfileContactRow, UserProfileEmail, UserProfileEmailVerifier } from '../user-profile-contact.types'; import type { UserProfileAddEmailController } from './user-profile-add-email.controller'; export interface UserProfileEmailSectionViewProps { - emails: UserProfileEmail[]; + emails: UserProfileContactRow[]; canAdd: boolean; + isDisabled: boolean; verification: UserProfileAddEmailController; error: string | undefined; rowRef: Ref; 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 c110e120037..6941aa480b1 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 @@ -13,6 +13,7 @@ import type { UserProfileEmailSectionViewProps } from './user-profile-email-sect export function UserProfileEmailSectionView({ emails, canAdd, + isDisabled, verification, error, rowRef, @@ -36,6 +37,7 @@ export function UserProfileEmailSectionView({ color='neutral' size='sm' variant='outline' + disabled={isDisabled} /> } > @@ -60,7 +62,7 @@ export function UserProfileEmailSectionView({ onRemove={remove ? email => remove.open(email) : undefined} onSetPrimary={onSetPrimary} onVerify={onVerify} - pendingVerifyId={verification.pendingVerifyId} + isDisabled={isDisabled} > {error} diff --git a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-account-row.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-account-row.view.tsx index 7908a8f2b0c..1bc2d9306be 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-account-row.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-account-row.view.tsx @@ -14,12 +14,10 @@ import type { UserProfileEnterpriseAccount } from './user-profile-enterprise-acc export function UserProfileEnterpriseAccountRowView({ account, onConnect, - isPending, disabled, }: { account: UserProfileEnterpriseAccount; onConnect?: (id: string) => void; - isPending?: boolean; disabled?: boolean; }) { const m = useMessages('userProfile').enterpriseAccounts; @@ -58,7 +56,7 @@ export function UserProfileEnterpriseAccountRowView({ size='sm' variant='outline' aria-label={fill(m.connectProvider, { provider: account.name })} - isPending={isPending} + isPending={account.isPending} pendingLabel={m.connecting} disabled={disabled} onClick={() => onConnect(account.id)} diff --git a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts index d92ec9df897..7e36b725538 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts @@ -18,7 +18,7 @@ export interface UserProfileEnterpriseAccountsControllerOptions { export interface UserProfileEnterpriseAccountsController { accounts: UserProfileEnterpriseAccount[]; connections: UserProfileEnterpriseConnection[]; - pendingId: string | undefined; + isDisabled: boolean; onConnect: (connectionId: string) => void; connectReverification: ReverificationController; } @@ -34,10 +34,12 @@ export function useUserProfileEnterpriseAccountsController({ return { accounts, - connections: connections.map(connection => - actions.errorKey === connection.id ? { ...connection, connectError: actions.error } : connection, - ), - pendingId: actions.pendingKey, + connections: connections.map(connection => ({ + ...connection, + isPending: actions.pendingKey === connection.id, + connectError: actions.errorKey === connection.id ? actions.error : undefined, + })), + isDisabled: actions.isPending, onConnect: id => void actions.run(id, async () => { if ((await onConnect(id)) === 'redirecting') { diff --git a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.types.ts index 9f43d1bf1af..3e09e948d21 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.types.ts @@ -4,6 +4,7 @@ export interface UserProfileEnterpriseConnection { iconUrl?: string; monochromeIcon?: boolean; connectError?: string; + isPending?: boolean; } export interface UserProfileEnterpriseAccount extends UserProfileEnterpriseConnection { diff --git a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.view.tsx index ca9ba722029..c2144b7382c 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.view.tsx @@ -11,7 +11,7 @@ import type { export interface UserProfileEnterpriseAccountsSectionViewProps { accounts: UserProfileEnterpriseAccount[]; connections?: UserProfileEnterpriseConnection[]; - pendingId?: string; + isDisabled?: boolean; onConnect?: (id: string) => void; connectReverification?: ReverificationController; } @@ -19,7 +19,7 @@ export interface UserProfileEnterpriseAccountsSectionViewProps { export function UserProfileEnterpriseAccountsSectionView({ accounts, connections = [], - pendingId, + isDisabled = false, onConnect, connectReverification, }: UserProfileEnterpriseAccountsSectionViewProps) { @@ -49,8 +49,7 @@ export function UserProfileEnterpriseAccountsSectionView({ key={connection.id} account={connection} onConnect={onConnect} - isPending={pendingId === connection.id} - disabled={pendingId !== undefined} + disabled={isDisabled} /> )) : null} diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx index f146797e6cf..61781b09b0f 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/__tests__/user-profile-mfa-management.feature.test.tsx @@ -367,6 +367,41 @@ describe('User profile MFA management', () => { } }); + it('blocks backup-code regeneration while the default SMS method is changing', async () => { + const first = fapiPhoneNumber({ ...phone, reserved_for_second_factor: true, default_second_factor: true }); + const second = fapiPhoneNumber({ + ...phone, + id: 'phone_2', + phone_number: '+15555550202', + reserved_for_second_factor: true, + }); + const fapi = await renderMfa( + fapiUser({ id: 'user_1', phone_numbers: [first, second], backup_code_enabled: true, two_factor_enabled: true }), + ); + const held = holdRequests('post', '/v1/me/phone_numbers/phone_2'); + const user = userEvent.setup(); + try { + await openMenu(user, screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0202' })); + await user.click(screen.getByRole('menuitem', { name: 'Set as default' })); + await waitFor(() => expect(held.requests).toHaveLength(1)); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + const manage = screen.getByRole('button', { name: 'Manage Backup codes' }); + expect(manage).toHaveAttribute('aria-disabled', 'true'); + await user.click(manage); + expect(screen.queryByRole('menu')).toBeNull(); + expect(fapi.mfa.backupCodeCreations).toBe(0); + expect(screen.getByRole('button', { name: 'Add verification method' })).toBeDisabled(); + } finally { + held.release(); + } + const manage = screen.getByRole('button', { name: 'Manage Backup codes' }); + await waitFor(() => expect(manage).not.toHaveAttribute('aria-disabled', 'true')); + await openMenu(user, manage); + await user.click(screen.getByRole('menuitem', { name: 'Regenerate' })); + await waitFor(() => expect(screen.getByText('CODE0100')).toBeVisible()); + expect(fapi.mfa.backupCodeCreations).toBe(1); + }); + it('returns failed regeneration to the section and retries from the menu', async () => { const fapi = await renderMfa( fapiUser({ id: 'user_1', totp_enabled: true, backup_code_enabled: true, two_factor_enabled: true }), @@ -421,6 +456,7 @@ describe('User profile MFA management', () => { expect(screen.getByRole('menuitem', { name: 'Regenerate' })).toHaveAttribute('aria-busy', 'true'); expect(await screen.findByRole('progressbar', { name: 'Regenerate' })).toBeVisible(); expect(screen.queryByRole('dialog')).toBeNull(); + expect(screen.getByRole('button', { name: 'Manage Authenticator app' })).toHaveAttribute('aria-disabled', 'true'); } finally { generate.release(); } @@ -428,6 +464,10 @@ describe('User profile MFA management', () => { const list = await screen.findByRole('list', { name: 'Backup codes' }); expect(within(list).getAllByRole('listitem')).toHaveLength(10); await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + expect(screen.getByRole('button', { name: 'Manage Authenticator app', hidden: true })).not.toHaveAttribute( + 'aria-disabled', + 'true', + ); }); it('holds the Regenerate menu item until session verification opens', async () => { @@ -865,6 +905,12 @@ describe('User profile MFA management', () => { expect(screen.getAllByText('SMS verification')[0]?.closest('li')).toHaveTextContent('+1 (555) 555-0202'), ); expect(screen.getAllByText('SMS verification')[0]?.closest('li')).toHaveTextContent('Default'); + await waitFor(() => + expect(screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0101' })).not.toHaveAttribute( + 'aria-disabled', + 'true', + ), + ); await user.click(screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0101' })); await waitFor(() => expect(screen.getByRole('menuitem', { name: 'Set as default' })).toBeVisible()); }); @@ -908,6 +954,12 @@ describe('User profile MFA management', () => { expect(fapi.mfa.phoneUpdates).toContainEqual({ id: 'phone_2', default: true, reserved: undefined }), ); expect(attempts).toBe(2); + await waitFor(() => + expect(screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0101' })).not.toHaveAttribute( + 'aria-disabled', + 'true', + ), + ); await user.click(screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0101' })); await waitFor(() => expect(screen.getByRole('menuitem', { name: 'Set as default' })).toBeVisible()); }); @@ -916,7 +968,7 @@ describe('User profile MFA management', () => { { phase: 'mutation', method: 'post', path: '/v1/me/phone_numbers/phone_2', pendingPhone: '+1 (555) 555-0202' }, { phase: 'refresh', method: 'get', path: '/v1/me', pendingPhone: '+1 (555) 555-0101' }, ] as const)( - 'keeps method menus available but hides default changes during $phase', + 'disables method menus while changing the default during $phase', async ({ method, path, pendingPhone }) => { const first = fapiPhoneNumber({ ...phone, reserved_for_second_factor: true, default_second_factor: true }); const second = fapiPhoneNumber({ @@ -934,16 +986,21 @@ describe('User profile MFA management', () => { await waitFor(() => expect(held.requests).toHaveLength(1)); try { const pendingAction = screen.getByRole('button', { name: `Manage SMS verification ${pendingPhone}` }); - await openMenu(user, pendingAction); - await waitFor(() => expect(screen.getByRole('menuitem', { name: 'Remove method' })).toBeVisible()); - expect(screen.queryByRole('menuitem', { name: 'Set as default' })).toBeNull(); - await user.click(await screen.findByRole('menuitem', { name: 'Remove method' })); + expect(pendingAction).toHaveAttribute('aria-disabled', 'true'); + await user.click(pendingAction); + expect(screen.queryByRole('menu')).toBeNull(); expect(screen.queryByRole('alertdialog')).toBeNull(); } finally { held.release(); } await waitFor(() => expect(screen.getByText('+1 (555) 555-0202').closest('li')).toHaveTextContent('Default')); await user.keyboard('{Escape}'); + await waitFor(() => + expect(screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0101' })).not.toHaveAttribute( + 'aria-disabled', + 'true', + ), + ); await user.click(screen.getByRole('button', { name: 'Manage SMS verification +1 (555) 555-0101' })); await waitFor(() => expect(screen.getByRole('menuitem', { name: 'Set as default' })).toBeVisible()); }, diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-row.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-row.view.tsx index cffe0b4ebdf..10a6d93c07b 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-row.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-row.view.tsx @@ -11,12 +11,14 @@ import type { UserProfileMfaMethod, UserProfileMfaSectionViewProps } from './use export function UserProfileMfaRowView({ method, + isDisabled, triggerRef, onRemove, onSetDefault, regenerate, }: Pick & { method: UserProfileMfaMethod; + isDisabled?: boolean; triggerRef?: Ref; onRemove?: () => void; onSetDefault?: (id: string) => void; @@ -56,6 +58,8 @@ export function UserProfileMfaRowView({ {actions.length > 0 ? ( sectionRef.current?.querySelector('button:not([disabled])') ?? sectionRef.current, }); + const localPendingAction = usePendingAction(); + const actions = pendingAction ?? localPendingAction; const remove = useConfirmController({ onConfirm: method => removalFocus.remove(method.id), reverification: removeReverification, }); const [pickerOpen, setPickerOpen] = useState(false); - const defaultAction = usePendingAction({ errorFallback: m.setDefaultError }); const setDefault = (id: string) => { const method = methods.find(method => method.id === id); if (!onSetDefault || method?.type !== 'sms' || !method.canSetDefault || method.isDefault) { return; } - void defaultAction.run(id, () => onSetDefault(id)); + void actions.run(id, () => onSetDefault(id), { errorFallback: m.setDefaultError }); }; return { sectionRef, + isDisabled: actions.isPending, pickerOpen, onPickerOpenChange: setPickerOpen, addMethod: onAdd @@ -49,16 +52,13 @@ export function useUserProfileMfaSectionLeafController({ : undefined, registerTrigger: removalFocus.registerTrigger, openRemoval: (method: UserProfileMfaMethod) => { - if (defaultAction.isPending) { - return; - } - defaultAction.reset(); + actions.reset(); remove.open(method); }, - remove: onRemove ? remove : undefined, + remove: onRemove ? actions.guard(remove) : undefined, finalRemovalFocus: removalFocus.finalFocus, - setDefaultFinalFocus: () => (defaultAction.lastKey ? removalFocus.trigger(defaultAction.lastKey) : null), - defaultError: defaultAction.error, - setDefault: onSetDefault && !defaultAction.isPending ? setDefault : undefined, + setDefaultFinalFocus: () => (actions.lastKey ? removalFocus.trigger(actions.lastKey) : null), + defaultError: actions.error, + setDefault: onSetDefault ? setDefault : undefined, }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts index 9602f10ca77..3d8f54cd463 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.controller.ts @@ -1,6 +1,7 @@ import { isReverificationCancelledError } from '@clerk/shared/error'; import { useEffect } from 'react'; +import { usePendingAction } from '../../../hooks/use-pending-action'; import { type ErrorDescription, fill, type MosaicMessages, useErrorText, useMessages } from '../../../localization'; import { setup } from '../../../machine/setup'; import type { DoneInvokeEvent } from '../../../machine/types'; @@ -355,9 +356,9 @@ function createBackupCodeActions( const generateBackupCodes = () => { const generate = model.generateBackupCodes; if (!generate) { - return; + return Promise.resolve(); } - void run('generate', async () => { + return run('generate', async () => { const codes = await generate(); return { kind: 'backup', codes }; }).catch(() => undefined); @@ -504,6 +505,7 @@ function createSetupProps( } export function useUserProfileMfaController(model: ReadyModel) { + const pendingAction = usePendingAction(); const errorText = useErrorText(); const backupCodeMessages = useMessages('userProfile').mfa.backupCodes; const [{ context, value }, send] = useMachine(mfaMachine); @@ -557,7 +559,7 @@ export function useUserProfileMfaController(model: ReadyModel) { prepareAuthenticator(); } } else if (method === 'backup-codes') { - generateBackupCodes(); + void generateBackupCodes(); } else { send({ type: 'SELECT_SMS', phoneId: model.phones[0]?.id ?? '', hasPhones: model.phones.length > 0 }); } @@ -574,7 +576,7 @@ export function useUserProfileMfaController(model: ReadyModel) { submitSms, resendSms, verifyAuthenticator, - generateBackupCodes, + generateBackupCodes: () => void generateBackupCodes(), copy, download, print, @@ -585,11 +587,12 @@ export function useUserProfileMfaController(model: ReadyModel) { ); const sectionProps: UserProfileMfaSectionViewProps = { + pendingAction, methods: [...model.methods], addableMethods: model.addableMethods, regenerate: model.generateBackupCodes ? { - onClick: generateBackupCodes, + onClick: () => void pendingAction.run('regenerate', generateBackupCodes), status: context.pending === 'generate' && !dialogOpen ? 'pending' : 'idle', error: flow.kind === 'closed' && flow.error ? errorText(flow.error) : undefined, } @@ -607,7 +610,7 @@ export function useUserProfileMfaController(model: ReadyModel) { dialogOpen, showAddTrigger, showAddControl: showAddTrigger || dialogOpen, - isPending: isBusy, + isPending: isBusy || pendingAction.isPending, reverifying, onDialogOpenChange: (open: boolean) => { if (!open && !canClose) { diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.view.tsx index e4609cf442b..de7d2006951 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section/user-profile-mfa-section.view.tsx @@ -4,6 +4,7 @@ import type { ReactNode, Ref } from 'react'; import { Confirmation } from '../../../blocks/confirmation'; import { ReverificationDialog } from '../../../blocks/reverification'; import { Section } from '../../../components/section'; +import type { PendingAction } from '../../../hooks/use-pending-action'; import { fill, type MosaicMessages, useMessages } from '../../../localization'; import type { ReverificationController } from '../../reverification'; import { UserProfileSecurityList } from '../user-profile-security-list'; @@ -17,6 +18,7 @@ export type { UserProfileMfaAddableMethod, UserProfileMfaMethod } from './user-p export interface UserProfileMfaSectionViewProps { methods: UserProfileMfaMethod[]; + pendingAction?: PendingAction; addableMethods?: readonly UserProfileMfaAddableMethod[]; addButtonRef?: Ref; addControl?: ReactNode; @@ -30,6 +32,7 @@ export interface UserProfileMfaSectionViewProps { export function UserProfileMfaSectionView({ methods, + pendingAction, addableMethods, addButtonRef, addControl, @@ -43,6 +46,7 @@ export function UserProfileMfaSectionView({ const m = useMessages('userProfile').mfa; const controller = useUserProfileMfaSectionLeafController({ methods, + pendingAction, onAdd, onRemove, onSetDefault, @@ -58,6 +62,7 @@ export function UserProfileMfaSectionView({ (controller.addMethod && addableMethods?.length ? ( @@ -77,6 +82,7 @@ export function UserProfileMfaSectionView({ controller.openRemoval(method) : undefined} onSetDefault={controller.setDefault} diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-add-phone.controller.test.ts b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-add-phone.controller.test.ts index 3340d462079..0c8257bbdea 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-add-phone.controller.test.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-add-phone.controller.test.ts @@ -43,12 +43,12 @@ describe('useUserProfileAddPhoneController', () => { const verifier = { sendCode, verifyCode: () => Promise.resolve() }; expect(result.current.verifyingId).toBeUndefined(); - act(() => result.current.onVerifyPhone({ id: 'phone_2', value: '+18015550101' }, verifier)); + act(() => void result.current.onVerifyPhone({ id: 'phone_2', value: '+18015550101' }, verifier)); expect(result.current.verifyingId).toBe('phone_2'); await act(async () => vi.advanceTimersByTimeAsync(0)); expect(result.current.step).toBe('verify'); - act(() => result.current.onVerifyPhone({ id: 'phone_3', value: '+18015550102' }, verifier)); + act(() => void result.current.onVerifyPhone({ id: 'phone_3', value: '+18015550102' }, verifier)); expect(result.current.verifyingId).toBe('phone_2'); expect(result.current.phoneNumber).toBe('+18015550101'); 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 4abe0f4d86b..f714e2c17c7 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 @@ -95,6 +95,35 @@ async function enterCode(actor: Actor, code: string) { } describe('the user profile phone section', () => { + it('holds the menu and row order while setting a number as primary', async () => { + const { actor } = await renderSection(signedIn([HOME, WORK])); + const primary = holdRequests('post', '/v1/me'); + try { + await manage(actor, WORK_LABEL, 'Set as primary'); + await waitFor(() => expect(primary.requests).toHaveLength(1)); + const menu = screen.getByRole('menu'); + const item = within(menu).getByRole('menuitem', { name: 'Set as primary' }); + expect(item).toHaveAttribute('aria-busy', 'true'); + const manageWork = screen.getByRole('button', { name: `Manage ${WORK_LABEL}` }); + expect(manageWork).toHaveAttribute('aria-disabled', 'true'); + expect(within(menu).getByRole('menuitem', { name: 'Remove phone number' })).toHaveAttribute( + 'aria-disabled', + 'true', + ); + expect(screen.queryByRole('alertdialog')).toBeNull(); + expect(screen.getByRole('button', { name: 'Add phone number' })).toBeDisabled(); + expect(await within(item).findByRole('progressbar', { name: 'Set as primary' })).toBeVisible(); + expect(listed()).toEqual([HOME_LABEL, WORK_LABEL]); + } finally { + primary.release(); + await waitFor(() => expect(within(row()).getByText(WORK_LABEL).parentElement).toHaveTextContent('Primary')); + } + expect(screen.getByRole('button', { name: `Manage ${WORK_LABEL}` })).not.toHaveAttribute('aria-disabled', 'true'); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + expect(listed()).toEqual([HOME_LABEL, WORK_LABEL]); + expect(screen.queryByRole('progressbar', { name: 'Set as primary' })).toBeNull(); + }); + it('lists the primary number first and marks it', async () => { await renderSection(signedIn([HOME, WORK])); @@ -184,6 +213,8 @@ describe('the user profile phone section', () => { expect(screen.getByRole('menuitem', { name: 'Verify phone number' })).toHaveAttribute('aria-busy', 'true'); expect(await screen.findByRole('progressbar', { name: 'Verify phone number' })).toBeVisible(); expect(screen.queryByRole('dialog')).toBeNull(); + expect(screen.getByRole('button', { name: `Manage ${HOME_LABEL}` })).toHaveAttribute('aria-disabled', 'true'); + expect(screen.getByRole('button', { name: 'Add phone number' })).toBeDisabled(); } finally { prepare.release(); } @@ -191,6 +222,7 @@ describe('the user profile phone section', () => { await waitFor(() => expect(screen.getByRole('button', { name: 'Close' })).toHaveFocus()); await enterCode(actor, VERIFICATION_CODE); expect(screen.queryByRole('menu')).toBeNull(); + expect(screen.getByRole('button', { name: `Manage ${HOME_LABEL}` })).not.toHaveAttribute('aria-disabled', 'true'); await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); }); @@ -199,8 +231,8 @@ describe('the user profile phone section', () => { await manage(actor, WORK_LABEL, 'Set as primary'); - await waitFor(() => expect(listed()).toEqual([WORK_LABEL, HOME_LABEL])); - expect(within(row()).getByText(WORK_LABEL).parentElement).toHaveTextContent('Primary'); + await waitFor(() => expect(within(row()).getByText(WORK_LABEL).parentElement).toHaveTextContent('Primary')); + expect(listed()).toEqual([HOME_LABEL, WORK_LABEL]); }); it('removes a number once the removal is confirmed', async () => { @@ -474,6 +506,7 @@ describe('phone reverification', () => { await manage(actor, WORK_LABEL, 'Set as primary'); expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); + await waitFor(() => expect(screen.queryByRole('menu', { hidden: true })).toBeNull()); await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus()); await actor.keyboard('hunter2{Enter}'); @@ -498,6 +531,7 @@ describe('phone reverification', () => { expect(within(row()).queryByRole('alert')).toBeNull(); expect(within(row()).getByRole('button', { name: `Manage ${WORK_LABEL}` })).toHaveFocus(); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); await manage(actor, WORK_LABEL, 'Set as primary'); await waitFor(() => expect(fapi.client.sessions[0]?.user?.primary_phone_number_id).toBe('idn_work')); }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-add-phone.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-add-phone.controller.ts index 522cd2ddbe4..c8a11fe5f93 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-add-phone.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-add-phone.controller.ts @@ -7,6 +7,7 @@ import { useErrorText } from '../../../localization'; import { setup } from '../../../machine/setup'; import type { ErrorInvokeEvent } from '../../../machine/types'; import { useMachine } from '../../../machine/use-machine'; +import { waitFor } from '../../../machine/wait-for'; import type { FormError } from '../../../utils/errors'; import { toFormError } from '../../../utils/errors'; import { CODE_RESEND_SECONDS } from '../../../utils/verification'; @@ -25,7 +26,7 @@ export interface UserProfileAddPhoneController extends UserProfileAddPhoneDialog handle: DialogHandle; verifyingId: string | undefined; pendingVerifyId: string | undefined; - onVerifyPhone: (phone: Pick, verifier: UserProfilePhoneVerifier) => void; + onVerifyPhone: (phone: Pick, verifier: UserProfilePhoneVerifier) => Promise; } interface Context { @@ -199,9 +200,10 @@ export function useUserProfileAddPhoneController({ }, verifyingId: snapshot.context.verifyingId, pendingVerifyId: state === 'preparing' ? snapshot.context.verifyingId : undefined, - onVerifyPhone: (phone, verifier) => { + onVerifyPhone: async (phone, verifier) => { codeForm.reset(); send({ type: 'START', id: phone.id, phoneNumber: phone.value, verifier }); + await waitFor(actor, current => current.value !== 'preparing'); }, }; } 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 9419a425010..f3ef40941c2 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,15 +1,16 @@ import { stringToFormattedPhoneString } from '@clerk/shared/phone'; -import { useRef } from 'react'; +import { useRef, useState } from 'react'; import { useConfirmController } from '../../../blocks/confirm'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; +import { applyOrder } from '../../../utils/apply-order'; 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'; export function useUserProfilePhoneSectionController({ - phones, + phones: modelPhones, defaultPhoneCountry, onCreatePhone, getPhoneVerifier, @@ -19,11 +20,13 @@ export function useUserProfilePhoneSectionController({ setPrimaryReverification, createReverification, }: Omit): UserProfilePhoneSectionViewProps { + const [order] = useState(() => modelPhones.map(phone => phone.id)); + const phones = applyOrder(order, modelPhones, phone => phone.id); const verification = useUserProfileAddPhoneController({ onCreate: onCreatePhone, reverification: createReverification, }); - const setPrimary = usePendingAction(); + const actions = usePendingAction(); const rowRef = useRef(null); const removalFocus = useListRemovalFocus({ ids: phones.map(phone => phone.id), @@ -36,24 +39,30 @@ export function useUserProfilePhoneSectionController({ }); return { - phones: phones.map(phone => ({ ...phone, value: stringToFormattedPhoneString(phone.value) })), + phones: phones.map(phone => ({ + ...phone, + value: stringToFormattedPhoneString(phone.value), + isVerifyPending: verification.pendingVerifyId === phone.id, + isPrimaryPending: actions.pendingKey === phone.id && !setPrimaryReverification.visible, + })), defaultPhoneCountry, canAdd: Boolean(onCreatePhone), verification, - error: setPrimary.error, + error: actions.error, + isDisabled: actions.isPending, rowRef, registerTrigger: removalFocus.registerTrigger, - remove: onRemovePhone ? remove : undefined, + remove: onRemovePhone ? actions.guard(remove) : undefined, finalFocus: removalFocus.finalFocus, verificationFinalFocus: () => (verification.verifyingId ? removalFocus.trigger(verification.verifyingId) : null), setPrimaryReverification, - setPrimaryFinalFocus: () => (setPrimary.lastKey ? removalFocus.trigger(setPrimary.lastKey) : null), + setPrimaryFinalFocus: () => (actions.lastKey ? removalFocus.trigger(actions.lastKey) : null), onVerify: id => { const phone = phones.find(phone => phone.id === id); if (phone) { - verification.onVerifyPhone(phone, getPhoneVerifier(id)); + void actions.run(id, () => verification.onVerifyPhone(phone, getPhoneVerifier(id))); } }, - onSetPrimary: id => void setPrimary.run(id, () => onSetPrimaryPhone(id)), + onSetPrimary: id => void actions.run(id, () => onSetPrimaryPhone(id)), }; } 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 b9a659e3f4c..a15b5130d1e 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 @@ -4,13 +4,14 @@ import type { ConfirmController } from '../../../blocks/confirm'; import type { DialogFocusTarget } from '../../../components/dialog'; import type { CountryIso } from '../../../components/phone-input'; import type { ReverificationController } from '../../reverification'; -import type { UserProfilePhone, UserProfilePhoneVerifier } from '../user-profile-contact.types'; +import type { UserProfileContactRow, UserProfilePhone, UserProfilePhoneVerifier } from '../user-profile-contact.types'; import type { UserProfileAddPhoneController } from './user-profile-add-phone.controller'; export interface UserProfilePhoneSectionViewProps { - phones: UserProfilePhone[]; + phones: UserProfileContactRow[]; defaultPhoneCountry?: CountryIso; canAdd: boolean; + isDisabled: boolean; verification: UserProfileAddPhoneController; error: string | undefined; rowRef: Ref; 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 f5c0d37718b..f40fdcf72b6 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 @@ -14,6 +14,7 @@ export function UserProfilePhoneSectionView({ phones, defaultPhoneCountry, canAdd, + isDisabled, verification, error, rowRef, @@ -37,6 +38,7 @@ export function UserProfilePhoneSectionView({ color='neutral' size='sm' variant='outline' + disabled={isDisabled} /> } > @@ -61,7 +63,7 @@ export function UserProfilePhoneSectionView({ onRemove={remove ? phone => remove.open(phone) : undefined} onSetPrimary={onSetPrimary} onVerify={onVerify} - pendingVerifyId={verification.pendingVerifyId} + isDisabled={isDisabled} > {error} diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.controller.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.controller.test.tsx index 70fd2af2da2..01c5924a71d 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.controller.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.controller.test.tsx @@ -2,6 +2,7 @@ import { act, renderHook } from '@testing-library/react'; import { describe, expect, it, vi } from 'vitest'; import { deferred } from '../../../../__tests__/async'; +import type { ReverificationController } from '../../../reverification'; import { useUserProfileWeb3WalletsController } from '../user-profile-web3-wallets-section.controller'; import type { ReadyWeb3WalletsModel, @@ -14,6 +15,12 @@ const providers = [ { id: 'web3_solana_signature', provider: 'Solana', walletPicker: 'solana' }, ] satisfies UserProfileWeb3Provider[]; +const idleReverification = { status: 'idle', visible: false } satisfies ReverificationController; + +function pendingRowId(controller: ReturnType) { + return [...controller.wallets, ...controller.availableProviders].find(row => row.isPending)?.id; +} + describe('Web3 connection controller', () => { it('does not move an address-phase connection to a different provider with the same address', async () => { const pending = deferred(); @@ -42,7 +49,7 @@ describe('Web3 connection controller', () => { act(() => { void result.current.onConnect('web3_metamask_signature'); }); - expect(result.current.pendingId).toBe('web3_metamask_signature'); + expect(pendingRowId(result.current)).toBe('web3_metamask_signature'); await act(async () => { pending.reject(new Error('Rejected')); await pending.promise.catch(() => undefined); @@ -69,6 +76,7 @@ describe('Web3 connection controller', () => { verify: vi.fn(), setPrimary, remove: vi.fn(), + setPrimaryReverification: idleReverification, }), ); @@ -79,7 +87,7 @@ describe('Web3 connection controller', () => { act(() => { void result.current.onSetPrimary('other'); }); - expect(result.current.pendingId).toBe('other'); + expect(pendingRowId(result.current)).toBe('other'); await act(async () => { primary.reject(new Error('Primary rejected')); await primary.promise.catch(() => undefined); @@ -137,7 +145,7 @@ describe('Web3 connection controller', () => { act(() => { void result.current.onConnect('web3_metamask_signature'); }); - expect(result.current.pendingId).toBe('web3_metamask_signature'); + expect(pendingRowId(result.current)).toBe('web3_metamask_signature'); await act(async () => { pending.resolve(); await pending.promise; @@ -172,7 +180,7 @@ describe('Web3 connection controller', () => { void result.current.onSetPrimary('other'); void result.current.onConnect('web3_metamask_signature'); }); - expect(result.current.pendingId).toBe('target'); + expect(pendingRowId(result.current)).toBe('target'); expect(setPrimary).not.toHaveBeenCalled(); await act(async () => { pending.reject(new Error('Retry rejected')); @@ -229,7 +237,7 @@ describe('Web3 connection controller', () => { address: provider.walletPicker === 'solana' ? address : address.toLowerCase(), }); }); - expect(result.current.pendingId).toBe('target'); + expect(pendingRowId(result.current)).toBe('target'); act(() => reportTarget?.({ kind: 'wallet', id: 'target' })); @@ -237,7 +245,7 @@ describe('Web3 connection controller', () => { signature.reject(new Error('Signature rejected')); await signature.promise.catch(() => undefined); }); - expect(result.current.pendingId).toBeUndefined(); + expect(pendingRowId(result.current)).toBeUndefined(); expect(result.current.wallets[1]?.verifyError).toBe('Something went wrong. Please try again.'); expect(result.current.wallets[0]?.verifyError).toBeUndefined(); }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx index 7bf2d1ef0e7..b711cc2ec6a 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx @@ -464,7 +464,7 @@ describe('Web3 wallets', () => { await waitFor(() => expect(screen.getByText('Primary')).toBeInTheDocument()); }); - it('sets a verified wallet as primary through Clerk', async () => { + it('holds the wallet menu while setting primary and updates the badge in place', async () => { const fapi = await renderWeb3({ web3_wallets: [ fapiWeb3Wallet({ id: 'wallet_1', web3_wallet: '0x1234567890abcdef' }), @@ -475,13 +475,32 @@ describe('Web3 wallets', () => { const user = userEvent.setup(); await waitFor(() => expect(screen.getAllByRole('button', { name: 'Manage MetaMask' })).toHaveLength(2)); + expect(screen.getAllByRole('button', { name: 'Manage MetaMask' })[1].closest('.cl-section-row')).toContainElement( + screen.getByTitle('0xabcdef1234567890'), + ); await openMenu(user, screen.getAllByRole('button', { name: 'Manage MetaMask' })[0]); expect(screen.queryByRole('menuitem', { name: 'Set as primary' })).toBeNull(); await user.keyboard('{Escape}'); await openMenu(user, screen.getAllByRole('button', { name: 'Manage MetaMask' })[1]); - await user.click(screen.getByRole('menuitem', { name: 'Set as primary' })); + const primary = holdRequests('post', '/v1/me'); + try { + await user.click(screen.getByRole('menuitem', { name: 'Set as primary' })); + await waitFor(() => expect(primary.requests).toHaveLength(1)); + const menu = screen.getByRole('menu'); + const item = within(menu).getByRole('menuitem', { name: 'Set as primary' }); + expect(item).toHaveAttribute('aria-busy', 'true'); + expect(within(menu).getByRole('menuitem', { name: 'Remove wallet' })).toHaveAttribute('aria-disabled', 'true'); + expect(await within(item).findByRole('progressbar', { name: 'Set as primary' })).toBeVisible(); + expect(screen.getByRole('button', { name: 'Connect Solana' })).toBeDisabled(); + } finally { + primary.release(); + } await waitFor(() => expect(fapi.client.sessions[0]?.user.primary_web3_wallet_id).toBe('wallet_2')); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + expect(screen.getAllByRole('button', { name: 'Manage MetaMask' })[1].closest('.cl-section-row')).toContainElement( + screen.getByTitle('0xabcdef1234567890'), + ); const selectedRow = screen.getByTitle('0xabcdef1234567890').closest('.cl-section-row'); expect(selectedRow).not.toBeNull(); if (selectedRow) { @@ -582,6 +601,7 @@ describe('Web3 wallets', () => { await openMenu(user, secondManage()); await user.click(screen.getByRole('menuitem', { name: 'Set as primary' })); expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); + await waitFor(() => expect(screen.queryByRole('menu', { hidden: true })).toBeNull()); await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus()); await user.keyboard('hunter2{Enter}'); @@ -604,6 +624,7 @@ describe('Web3 wallets', () => { expect(screen.queryByRole('alert')).toBeNull(); await waitFor(() => expect(secondManage()).toHaveFocus()); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); await openMenu(user, secondManage()); await user.click(screen.getByRole('menuitem', { name: 'Set as primary' })); await waitFor(() => expect(fapi.client.sessions[0]?.user.primary_web3_wallet_id).toBe('wallet_2')); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx index 9b0618176aa..aa4ba4bc86b 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx @@ -17,7 +17,6 @@ import type { UserProfileWeb3Provider, UserProfileWeb3Wallet } from './user-prof export function UserProfileWeb3WalletRowView({ wallet, triggerRef, - isPending = false, isDisabled = false, onConnect, onVerify, @@ -26,7 +25,6 @@ export function UserProfileWeb3WalletRowView({ }: { wallet: UserProfileWeb3Wallet | UserProfileWeb3Provider; triggerRef?: Ref; - isPending?: boolean; isDisabled?: boolean; onConnect?: (id: string) => void; onVerify?: (id: string) => void; @@ -35,6 +33,7 @@ export function UserProfileWeb3WalletRowView({ }) { const m = useMessages('userProfile').web3Wallets; const linkedWallet = 'address' in wallet ? wallet : undefined; + const isPending = wallet.isPending ?? false; const address = linkedWallet?.address; const shortAddress = address && (address.length <= 10 ? address : `${address.slice(0, 6)}...${address.slice(-4)}`); const actions: ActionMenuAction[] = []; @@ -43,7 +42,11 @@ export function UserProfileWeb3WalletRowView({ actions.push({ label: m.verify, onClick: () => onVerify(wallet.id) }); } if (linkedWallet && !linkedWallet.isPrimary && linkedWallet.isVerified && onSetPrimary) { - actions.push({ label: m.setPrimary, onClick: () => onSetPrimary(wallet.id) }); + actions.push({ + label: m.setPrimary, + onClick: () => onSetPrimary(wallet.id), + status: linkedWallet.isPrimaryPending ? 'pending' : 'idle', + }); } if (linkedWallet && onRemove && linkedWallet.canRemove !== false) { actions.push({ label: m.remove, color: 'negative', onClick: () => onRemove(linkedWallet) }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts index 9d242b0b1ae..e5d7fff0b05 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts @@ -5,6 +5,7 @@ import { usePanelTitle } from '../../../components/panel'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; import { useMessages } from '../../../localization'; +import { applyOrder } from '../../../utils/apply-order'; import type { ReadyWeb3WalletsModel, UserProfileWeb3Provider, @@ -23,7 +24,7 @@ type WalletAction = | { kind: 'verify' | 'primary'; walletId: string }; export function useUserProfileWeb3WalletsController({ - wallets, + wallets: modelWallets, availableProviders, connect, verify, @@ -45,6 +46,8 @@ export function useUserProfileWeb3WalletsController({ | 'createReverification' >) { const messages = useMessages('userProfile').web3Wallets; + const [order] = useState(() => modelWallets.map(wallet => wallet.id)); + const wallets = applyOrder(order, modelWallets, wallet => wallet.id); const action = usePendingAction({ errorFallback: messages.errors.generic }); const [picker, setPicker] = useState({ open: false }); const [owner, setOwner] = useState(); @@ -58,7 +61,6 @@ export function useUserProfileWeb3WalletsController({ sectionRef.current ?? panelTitle(), }); - // TODO: Share the action lock during removal while preserving errors for the confirmation dialog. const removeWallet = useConfirmController({ onConfirm: wallet => removalFocus.remove(wallet.id), reverification: removeReverification, @@ -150,18 +152,26 @@ export function useUserProfileWeb3WalletsController({ const actionFinalFocus = () => (ownerId ? removalFocus.trigger(ownerId) : null); return { - wallets: wallets.map(wallet => - errorId === wallet.id + wallets: wallets.map(wallet => { + const isPending = pendingId === wallet.id; + const row = { + ...wallet, + isPending, + isPrimaryPending: isPending && owner?.kind === 'primary' && !setPrimaryReverification.visible, + }; + return errorId === wallet.id ? owner?.kind === 'primary' - ? { ...wallet, primaryError: action.error } - : { ...wallet, verifyError: action.error } - : wallet, - ), - availableProviders: availableProviders.map(provider => - errorId === provider.id && (!picker.open || picker.kind !== 'connect' || picker.provider.id !== provider.id) - ? { ...provider, connectError: action.error } - : provider, - ), + ? { ...row, primaryError: action.error } + : { ...row, verifyError: action.error } + : row; + }), + availableProviders: availableProviders.map(provider => { + const row = { ...provider, isPending: pendingId === provider.id }; + return errorId === provider.id && + (!picker.open || picker.kind !== 'connect' || picker.provider.id !== provider.id) + ? { ...row, connectError: action.error } + : row; + }), error: !picker.open && errorId && @@ -169,10 +179,10 @@ export function useUserProfileWeb3WalletsController({ !availableProviders.some(provider => provider.id === errorId) ? action.error : undefined, - pendingId, + isDisabled: action.isPending, sectionRef, registerTrigger: removalFocus.registerTrigger, - remove: remove ? removeWallet : undefined, + remove: remove ? action.guard(removeWallet) : undefined, finalFocus: removalFocus.finalFocus, setPrimaryReverification, createReverification, diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.tsx index 38f77348d3a..99a1e7d83b2 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.tsx @@ -34,7 +34,7 @@ function Web3Wallets({ model }: { model: ReadyWeb3WalletsModel }) { wallets={controller.wallets} sectionRef={controller.sectionRef} availableProviders={controller.availableProviders} - pendingId={controller.pendingId} + isDisabled={controller.isDisabled} error={controller.error} onConnect={id => { void controller.onConnect(id); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts index a0ce8fec689..5cfc8fab839 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts @@ -13,6 +13,7 @@ export interface UserProfileWeb3Provider { iconUrl?: string; monochromeIcon?: boolean; connectError?: string; + isPending?: boolean; } export interface UserProfileWeb3Wallet { @@ -30,13 +31,15 @@ export interface UserProfileWeb3Wallet { canRemove?: boolean; primaryError?: string; verifyError?: string; + isPending?: boolean; + isPrimaryPending?: boolean; } export interface UserProfileWeb3WalletsSectionViewProps { wallets: UserProfileWeb3Wallet[]; sectionRef?: Ref; availableProviders?: UserProfileWeb3Provider[]; - pendingId?: string; + isDisabled?: boolean; error?: string; onConnect?: (id: string) => void; onVerify?: (id: string) => void; diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx index aeab97de06c..a400c7b0329 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx @@ -10,7 +10,7 @@ export function UserProfileWeb3WalletsSectionView({ wallets, sectionRef, availableProviders = [], - pendingId, + isDisabled = false, error, onConnect, onVerify, @@ -25,7 +25,6 @@ export function UserProfileWeb3WalletsSectionView({ }: UserProfileWeb3WalletsSectionViewProps) { const m = useMessages('userProfile').web3Wallets; const hasRows = wallets.length > 0 || (availableProviders.length > 0 && Boolean(onConnect)) || Boolean(error); - const isBusy = pendingId !== undefined; return ( <> @@ -46,8 +45,7 @@ export function UserProfileWeb3WalletsSectionView({ key={wallet.id} wallet={wallet} triggerRef={registerTrigger(wallet.id)} - isPending={pendingId === wallet.id} - isDisabled={isBusy} + isDisabled={isDisabled} onVerify={onVerify} onSetPrimary={onSetPrimary} onRemove={remove ? wallet => remove.open(wallet) : undefined} @@ -59,8 +57,7 @@ export function UserProfileWeb3WalletsSectionView({ key={provider.id} wallet={provider} triggerRef={registerTrigger(provider.id)} - isPending={pendingId === provider.id} - isDisabled={isBusy} + isDisabled={isDisabled} onConnect={onConnect} /> )) diff --git a/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx b/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx index b2e39244861..9179197a568 100644 --- a/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx +++ b/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx @@ -5,13 +5,24 @@ import { describe, expect, it, vi } from 'vitest'; import { deferred } from '../../__tests__/async'; import { clerkApiError } from '../../__tests__/clerk-errors'; +import { useConfirmController } from '../../blocks/confirm'; import { MosaicProvider } from '../../mosaic-provider'; +import { SaveError } from '../../utils/errors'; import { usePendingAction } from '../use-pending-action'; function blocked() { return clerkApiError('action_blocked', 'Raw server sentence.'); } +function useGuardedConfirm(onConfirm: () => Promise) { + const actions = usePendingAction(); + const remove = useConfirmController({ + onConfirm, + reverification: { status: 'loading', visible: true, onCancel: vi.fn() }, + }); + return { actions, remove: actions.guard(remove) }; +} + describe('usePendingAction', () => { it('reports success and leaves no error', async () => { const { result } = renderHook(() => usePendingAction()); @@ -264,3 +275,125 @@ describe('usePendingAction', () => { expect(result.current.errorKey).toBeUndefined(); }); }); + +describe('usePendingAction guard', () => { + it('refuses a guarded confirm while a run holds the lock, leaving the dialog unchanged', async () => { + const pending = deferred(); + const onConfirm = vi.fn(() => Promise.resolve()); + const { result } = renderHook(() => useGuardedConfirm(onConfirm)); + act(() => result.current.remove.onOpenChange(true)); + const owners = result.current; + + act(() => { + void owners.actions.run('row', () => pending.promise); + void owners.remove.confirm(); + }); + + expect(onConfirm).not.toHaveBeenCalled(); + expect(result.current.remove.isOpen).toBe(true); + expect(result.current.remove.isPending).toBe(false); + expect(result.current.actions.isPending).toBe(true); + await act(() => pending.resolve()); + expect(result.current.actions.isPending).toBe(false); + await act(() => result.current.remove.confirm()); + expect(onConfirm).toHaveBeenCalledOnce(); + expect(result.current.remove.isOpen).toBe(false); + }); + + it('refuses a run while a guarded confirm holds the lock, leaving its key and error unset', async () => { + const pending = deferred(); + const direct = vi.fn(); + const { result } = renderHook(() => useGuardedConfirm(() => pending.promise)); + act(() => result.current.remove.onOpenChange(true)); + const owners = result.current; + + await act(async () => { + void owners.remove.confirm(); + expect(await owners.actions.run('row', direct)).toBe(false); + }); + + expect(direct).not.toHaveBeenCalled(); + expect(result.current.actions.lastKey).toBeUndefined(); + expect(result.current.actions.error).toBeUndefined(); + expect(result.current.actions.isPending).toBe(true); + await act(() => pending.resolve()); + await act(async () => expect(await owners.actions.run('row', direct)).toBe(true)); + expect(direct).toHaveBeenCalledOnce(); + }); + + it('keeps the dialog error when a run refuses its retry', async () => { + const pending = deferred(); + const failure = new SaveError({ global: { code: 'action_blocked', message: 'Confirm refused.' } }); + const onConfirm = vi.fn(() => Promise.reject(failure)); + const { result } = renderHook(() => useGuardedConfirm(onConfirm)); + act(() => result.current.remove.onOpenChange(true)); + await act(() => result.current.remove.confirm()); + const error = result.current.remove.errorMessage; + expect(error).toBeDefined(); + expect(result.current.remove.isOpen).toBe(true); + expect(result.current.actions.error).toBeUndefined(); + + act(() => { + void result.current.actions.run('row', () => pending.promise); + void result.current.remove.confirm(); + }); + + expect(onConfirm).toHaveBeenCalledOnce(); + expect(result.current.remove.errorMessage).toBe(error); + expect(result.current.remove.isOpen).toBe(true); + expect(result.current.remove.isPending).toBe(false); + await act(() => pending.resolve()); + }); + + it('keeps the last key and clears the run error while a guarded confirm runs', async () => { + const pending = deferred(); + const failure = new SaveError({ global: { code: 'action_blocked', message: 'Run refused.' } }); + const { result } = renderHook(() => useGuardedConfirm(() => pending.promise)); + await act(() => result.current.actions.run('row', () => Promise.reject(failure))); + expect(result.current.actions.error).toBeDefined(); + + act(() => result.current.remove.onOpenChange(true)); + act(() => { + void result.current.remove.confirm(); + }); + + expect(result.current.actions.isPending).toBe(true); + expect(result.current.actions.pendingKey).toBeUndefined(); + expect(result.current.actions.lastKey).toBe('row'); + expect(result.current.actions.errorKey).toBeUndefined(); + expect(result.current.actions.error).toBeUndefined(); + await act(() => pending.resolve()); + expect(result.current.actions.isPending).toBe(false); + expect(result.current.actions.lastKey).toBe('row'); + }); + + it.each(['resolve', 'reject'])( + 'holds the lock after the dialog is cancelled until its action promises %s', + async outcome => { + const pending = deferred(); + const direct = vi.fn(); + const { result } = renderHook(() => useGuardedConfirm(() => pending.promise)); + act(() => result.current.remove.onOpenChange(true)); + act(() => { + void result.current.remove.confirm(); + }); + act(() => result.current.remove.onOpenChange(false)); + expect(result.current.remove.isOpen).toBe(false); + await act(async () => expect(await result.current.actions.run('row', direct)).toBe(false)); + expect(direct).not.toHaveBeenCalled(); + expect(result.current.actions.isPending).toBe(true); + expect(result.current.actions.pendingKey).toBeUndefined(); + + await act(() => { + if (outcome === 'reject') { + pending.reject(new Error('Late confirm failure')); + } else { + pending.resolve(); + } + }); + expect(result.current.actions.isPending).toBe(false); + expect(result.current.remove.errorMessage).toBeUndefined(); + await act(async () => expect(await result.current.actions.run('row', direct)).toBe(true)); + }, + ); +}); diff --git a/packages/mosaic/src/hooks/use-list-removal-focus.ts b/packages/mosaic/src/hooks/use-list-removal-focus.ts index 73621b38222..38a4604ede1 100644 --- a/packages/mosaic/src/hooks/use-list-removal-focus.ts +++ b/packages/mosaic/src/hooks/use-list-removal-focus.ts @@ -1,3 +1,4 @@ +import type { RefCallback } from 'react'; import { useRef } from 'react'; export function useListRemovalFocus({ @@ -10,14 +11,23 @@ export function useListRemovalFocus({ fallback: () => HTMLElement | null; }) { const triggers = useRef(new Map()); + const triggerRefs = useRef(new Map>()); const removed = useRef<{ id: string; index: number } | undefined>(undefined); - const registerTrigger = (id: string) => (element: HTMLButtonElement | null) => { - if (element) { - triggers.current.set(id, element); - } else { - triggers.current.delete(id); + const registerTrigger = (id: string) => { + const existing = triggerRefs.current.get(id); + if (existing) { + return existing; } + const ref = (element: HTMLButtonElement | null) => { + if (element) { + triggers.current.set(id, element); + } else { + triggers.current.delete(id); + } + }; + triggerRefs.current.set(id, ref); + return ref; }; const remove = async (id: string) => { diff --git a/packages/mosaic/src/hooks/use-pending-action.ts b/packages/mosaic/src/hooks/use-pending-action.ts index bfeb88f3274..e620d828510 100644 --- a/packages/mosaic/src/hooks/use-pending-action.ts +++ b/packages/mosaic/src/hooks/use-pending-action.ts @@ -10,8 +10,15 @@ export interface PendingActionOptions { errorFallback?: string; } +export interface PendingConfirm { + confirm: (payload: TPayload) => Promise; +} + export interface PendingAction { run: (key: TKey, action: () => Promise | void, options?: PendingActionOptions) => Promise; + guard: >( + confirm: TConfirm & PendingConfirm, + ) => TConfirm; pendingKey: TKey | undefined; lastKey: TKey | undefined; isPending: boolean; @@ -26,46 +33,64 @@ interface PendingActionFailure { errorFallback: string | undefined; } +type Pending = { kind: 'inline'; key: TKey } | { kind: 'external' }; + /** - * Runs inline actions, such as row buttons, under one lock and tracks which key is pending, which key ran last, and which key last failed. - * `run` resolves `true` on success and `false` on failure, ignores calls while any run is in flight, and clears the last error when it starts. + * Runs inline actions and guarded confirmations under one lock and tracks which inline key is pending, ran last, or last failed. + * `run` resolves `true` on success and `false` on failure, ignores calls while any action is in flight, and clears the last error when any action starts. */ export function usePendingAction({ errorFallback, }: PendingActionOptions = {}): PendingAction { const errorText = useErrorText(); - const [pendingKey, setPendingKey] = useState(); + const running = useRef(false); + const [pending, setPending] = useState>(); const [lastKey, setLastKey] = useState(); const [failure, setFailure] = useState>(); - const running = useRef(false); - const run = async (key: TKey, action: () => Promise | void, options: PendingActionOptions = {}) => { + const claim = (next: Pending, action: () => T | Promise): Promise | undefined => { if (running.current) { - return false; + return undefined; } running.current = true; - setPendingKey(key); - setLastKey(key); + setPending(next); setFailure(undefined); - try { - await action(); - return true; - } catch (cause) { - if (!isReverificationCancelledError(cause)) { - setFailure({ key, error: toLocalizableError(cause), errorFallback: options.errorFallback }); + return (async () => { + try { + return await action(); + } finally { + running.current = false; + setPending(undefined); } - return false; - } finally { - running.current = false; - setPendingKey(undefined); - } + })(); + }; + + const run = async (key: TKey, action: () => Promise | void, options: PendingActionOptions = {}) => { + return ( + claim({ kind: 'inline', key }, async () => { + setLastKey(key); + try { + await action(); + return true; + } catch (cause) { + if (!isReverificationCancelledError(cause)) { + setFailure({ key, error: toLocalizableError(cause), errorFallback: options.errorFallback }); + } + return false; + } + }) ?? false + ); }; return { run, - pendingKey, + guard: confirm => ({ + ...confirm, + confirm: async payload => (await claim({ kind: 'external' }, () => confirm.confirm(payload))) ?? false, + }), + pendingKey: pending?.kind === 'inline' ? pending.key : undefined, lastKey, - isPending: pendingKey !== undefined, + isPending: pending !== undefined, error: failure ? errorText(failure.error, failure.errorFallback ?? errorFallback) : undefined, errorKey: failure?.key, reset: () => setFailure(undefined), diff --git a/packages/mosaic/src/machine/README.md b/packages/mosaic/src/machine/README.md index 6bad81cb237..3fbff7c06c2 100644 --- a/packages/mosaic/src/machine/README.md +++ b/packages/mosaic/src/machine/README.md @@ -222,11 +222,12 @@ You've already seen `states`, `on`, `context`, `assign`, and `invoke`. The rest: ### API at a glance -The exports live in `create-machine.ts`, `setup.ts`, `assign.ts`, `create-actor.ts` and `use-machine.ts`; their JSDoc is the reference. Which React hook to reach for: +The exports live in `create-machine.ts`, `setup.ts`, `assign.ts`, `create-actor.ts`, `use-machine.ts` and `wait-for.ts`; their JSDoc is the reference. Which React hook to reach for: - `useMachine(machine)` when the component owns the actor for its lifetime. - `useActor(actor)` when the actor is shared or passed in, including a `mockActor` that renders a step directly. - `useSelector(actor, selector)` when a component needs one slice and should re-render only when that slice changes. +- `waitFor(actor, predicate)` when a caller needs a promise for a machine-driven step, such as holding a `usePendingAction` lock until the machine leaves a sending state. ### Two behaviors worth knowing diff --git a/packages/mosaic/src/machine/__tests__/wait-for.test.ts b/packages/mosaic/src/machine/__tests__/wait-for.test.ts new file mode 100644 index 00000000000..ba4f99d1e42 --- /dev/null +++ b/packages/mosaic/src/machine/__tests__/wait-for.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from 'vitest'; + +import { deferred, tick } from '../../__tests__/async'; +import { createActor } from '../create-actor'; +import { createMachine } from '../create-machine'; +import { waitFor } from '../wait-for'; + +function startSending() { + const gate = deferred(); + const machine = createMachine, { type: 'SEND' }>({ + initial: 'idle', + states: { + idle: { on: { SEND: 'sending' } }, + sending: { invoke: { src: () => gate.promise, onDone: 'sent' } }, + sent: {}, + }, + }); + const actor = createActor(machine); + actor.start(); + return { actor, gate }; +} + +describe('waitFor', () => { + it('resolves with the current snapshot when it already matches', async () => { + const { actor } = startSending(); + + await expect(waitFor(actor, snapshot => snapshot.value === 'idle')).resolves.toMatchObject({ value: 'idle' }); + }); + + it('resolves once the actor reaches a matching snapshot', async () => { + const { actor, gate } = startSending(); + actor.send({ type: 'SEND' }); + let settled = false; + const waiting = waitFor(actor, snapshot => snapshot.value !== 'sending').then(snapshot => { + settled = true; + return snapshot; + }); + + await tick(); + expect(settled).toBe(false); + + gate.resolve(); + await expect(waiting).resolves.toMatchObject({ value: 'sent' }); + }); +}); diff --git a/packages/mosaic/src/machine/wait-for.ts b/packages/mosaic/src/machine/wait-for.ts new file mode 100644 index 00000000000..26099d794ab --- /dev/null +++ b/packages/mosaic/src/machine/wait-for.ts @@ -0,0 +1,24 @@ +import type { Actor, EventObject, Snapshot } from './types'; + +/** + * Resolves with the first snapshot that matches `predicate`, checking the current one first. + * Use it to await a machine-driven step, such as `actions.run(key, () => waitFor(actor, s => s.value !== 'sending'))`. + */ +export function waitFor( + actor: Pick, 'getSnapshot' | 'subscribe'>, + predicate: (snapshot: Snapshot) => boolean, +): Promise> { + return new Promise(resolve => { + const current = actor.getSnapshot(); + if (predicate(current)) { + resolve(current); + return; + } + const unsubscribe = actor.subscribe(snapshot => { + if (predicate(snapshot)) { + unsubscribe(); + resolve(snapshot); + } + }); + }); +} diff --git a/packages/swingset/src/stories/fixtures/user-profile-enterprise-accounts.ts b/packages/swingset/src/stories/fixtures/user-profile-enterprise-accounts.ts index 9316a74f1d1..f05eecd8016 100644 --- a/packages/swingset/src/stories/fixtures/user-profile-enterprise-accounts.ts +++ b/packages/swingset/src/stories/fixtures/user-profile-enterprise-accounts.ts @@ -50,8 +50,8 @@ export function useEnterpriseAccountsFixture({ return { accounts: linkedAccounts, - connections: availableConnections, - pendingId, + connections: availableConnections.map(item => ({ ...item, isPending: item.id === pendingId })), + isDisabled: pendingId !== undefined, onConnect: (id: string) => { const connection = availableConnections.find(item => item.id === id); if (!connection || pendingId) {