From 448d16ed57df7a763d0c57d7e787af7d224801d3 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 15:54:56 -0600 Subject: [PATCH 01/12] test(mosaic): cover conflicting contact primary actions --- ...ser-profile-email-section.feature.test.tsx | 25 +++++++++++++++++++ ...ser-profile-phone-section.feature.test.tsx | 22 ++++++++++++++++ 2 files changed, 47 insertions(+) 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 d28b064404a..75b27675d32 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 @@ -68,6 +68,31 @@ async function enterCode(actor: Actor, code: string) { } describe('the user profile email section', () => { + it('closes the menu and blocks conflicting actions while setting an address as primary', 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)); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + const manage = screen.getByRole('button', { name: 'Manage bob@example.com' }); + expect(manage).toHaveAttribute('aria-disabled', 'true'); + await actor.click(manage); + expect(screen.queryByRole('menu')).toBeNull(); + expect(screen.queryByRole('alertdialog')).toBeNull(); + expect(screen.getByRole('button', { name: 'Add email' })).toBeDisabled(); + expect(await screen.findByRole('status', { name: 'Set as primary' })).toBeVisible(); + } 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'); + expect(screen.queryByRole('status', { 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'); 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 b796c5bf17d..cc86a87e320 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 @@ -92,6 +92,28 @@ async function enterCode(actor: Actor, code: string) { } describe('the user profile phone section', () => { + it('closes the menu and blocks conflicting actions 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)); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + const manageWork = screen.getByRole('button', { name: `Manage ${WORK_LABEL}` }); + expect(manageWork).toHaveAttribute('aria-disabled', 'true'); + await actor.click(manageWork); + expect(screen.queryByRole('menu')).toBeNull(); + expect(screen.queryByRole('alertdialog')).toBeNull(); + expect(screen.getByRole('button', { name: 'Add phone number' })).toBeDisabled(); + expect(await screen.findByRole('status', { name: 'Set as primary' })).toBeVisible(); + } 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'); + expect(screen.queryByRole('status', { name: 'Set as primary' })).toBeNull(); + }); + it('lists the primary number first and marks it', async () => { await renderSection(signedIn([HOME, WORK])); From 7f39b53f1b5279ff3acd0c815a4903576ddaf609 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 16:14:05 -0600 Subject: [PATCH 02/12] fix(mosaic): coordinate identifier mutations across action owners --- .changeset/friendly-identifier-lock.md | 2 + packages/mosaic/AGENTS.md | 1 + packages/mosaic/docs/controllers.md | 31 ++++ .../src/blocks/confirm/confirm-shell.tsx | 2 + .../src/blocks/confirm/confirm.controller.ts | 22 ++- .../__tests__/confirmation.feature.test.tsx | 1 + .../src/blocks/confirmation/confirmation.tsx | 3 +- .../src/blocks/destructive/destructive.tsx | 3 +- .../user-profile-contact-list-row.view.tsx | 19 ++- ...ser-profile-email-section.feature.test.tsx | 4 +- .../user-profile-email-section.controller.ts | 7 +- .../user-profile-email-section.types.ts | 2 + .../user-profile-email-section.view.tsx | 5 + ...ser-profile-phone-section.feature.test.tsx | 4 +- .../user-profile-phone-section.controller.ts | 7 +- .../user-profile-phone-section.types.ts | 2 + .../user-profile-phone-section.view.tsx | 5 + .../user-profile-web3-wallet-row.view.tsx | 12 +- ...profile-web3-wallets-section.controller.ts | 8 +- ...user-profile-web3-wallets-section.types.ts | 2 + ...user-profile-web3-wallets-section.view.tsx | 7 +- .../hooks/__tests__/use-action-lock.test.tsx | 139 ++++++++++++++++++ packages/mosaic/src/hooks/use-action-lock.ts | 29 ++++ .../src/hooks/use-list-removal-focus.ts | 20 ++- .../mosaic/src/hooks/use-pending-action.ts | 51 ++++--- 25 files changed, 342 insertions(+), 46 deletions(-) create mode 100644 .changeset/friendly-identifier-lock.md create mode 100644 packages/mosaic/src/hooks/__tests__/use-action-lock.test.tsx create mode 100644 packages/mosaic/src/hooks/use-action-lock.ts 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/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index 0ef6ef8a5da..f94ebb1b3b5 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -34,6 +34,7 @@ Most problems a feature hits already have a shared answer. Find the row for your | Problem | Use | | ------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------- | | An async action from a button or row: pending, an error, one lock | `usePendingAction` (`src/hooks/`) | +| Direct actions and confirmation dialogs that must exclude each other | `useActionLock` injected into `usePendingAction` and `useConfirmController` (`src/hooks/`, `src/blocks/confirm/`) | | A form: values, validation, submit, field errors | `useForm` (`src/components/form/`), with `validateAsync` for a server-side check | | A `
` element, without the browser's validation popup | `Form` (`src/primitives/form/`) | | A model save whose Clerk errors should land on form fields | `save` (`src/utils/errors.ts`) in the model, or reject with `FormSubmitError` | diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index d2aca72f306..247a4f21012 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -51,6 +51,37 @@ return { }; ``` +When a direct action and a confirmation must share admission, create one +`useActionLock` (`src/hooks/use-action-lock.ts`) and inject it into both owners: + +```ts +const actionLock = useActionLock(); +const primary = usePendingAction({ actionLock }); +const remove = useConfirmController({ + actionLock, + onConfirm: contact => removalFocus.remove(contact.id), +}); +``` + +The lock admits synchronously, so two calls in one event cannot both start. +Its `tryRun` returns `undefined` without invoking busy work, or a promise that +preserves the accepted action's result or rejection. It stays held until the +actual action settles, even if a dialog closes and stops observing the action. +Opening a dialog does not acquire the lock. The lock owns no errors or row IDs. +`usePendingAction` keeps its inline errors and boolean result; refused runs +return `false` without changing metadata. `useConfirmController` keeps removal +errors in its dialog; refusal leaves the dialog and its error unchanged. +Its `canConfirm` projection disables only the confirmation submit control. +Admission still runs through the lock, not the rendered projection. Without an +injected lock, each owner retains its existing independent behavior. + +`actionLock` is a hook option, not a per-run option. Project its `isPending` to +the controls in the shared section. For contact primary changes, close the menu +on selection by leaving `ActionMenu.status` undefined, disable Add and keep +Manage disabled but focusable, and show a delayed row spinner beside the value. +Use one `useSpinDelay` outside the row map with `minDuration: 0`, so feedback ends +with the request. The Primary badge replaces the spinner after success. + `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 diff --git a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx index 9c8b094194e..6c2aff082e6 100644 --- a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx +++ b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx @@ -34,6 +34,7 @@ export interface ConfirmStepProps { actionLabel: string; cancelLabel: string; isPending: boolean; + canConfirm: boolean; errorMessage: string | undefined; confirm: () => void; } @@ -68,6 +69,7 @@ function ConfirmCard({ actionLabel: resolve(actionLabel, payload), cancelLabel, isPending: controller.isPending, + canConfirm: controller.canConfirm, errorMessage: controller.errorMessage, confirm: () => controller.confirm(payload), }); diff --git a/packages/mosaic/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts index 6924f103b5b..18c70af44b1 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -4,6 +4,7 @@ import { useMemo } from 'react'; import type { DialogHandle } from '../../components/dialog'; import { Dialog } from '../../components/dialog'; import type { ReverificationController } from '../../features/reverification'; +import type { ActionLock } from '../../hooks/use-action-lock'; import type { ErrorDescription } from '../../localization'; import { useErrorText } from '../../localization'; import { setup } from '../../machine/setup'; @@ -73,6 +74,7 @@ export interface ConfirmController { isPending: boolean; /** Whether the dialog can close right now. False while the action is past the point of cancelling */ canClose: boolean; + canConfirm: boolean; errorMessage: string | undefined; /** Which card the dialog shows: the confirm step, or the reverification the action was interrupted by. */ step: ConfirmStep; @@ -85,6 +87,7 @@ export interface ConfirmController { } export interface ConfirmControllerOptions { + actionLock?: ActionLock; /** The action the dialog confirms. Resolve to close; reject to keep the dialog open showing why. */ onConfirm: (payload: Payload) => Promise | void; /** The flow `onConfirm` runs under, when the action may need step-up verification. */ @@ -107,6 +110,7 @@ export function useConfirmController({ onConfirm, reverification, errorFallback, + actionLock, }: ConfirmControllerOptions): ConfirmController { const handle = useMemo(() => Dialog.createHandle(), []); const [snapshot, send] = useMachine(confirmMachine); @@ -120,6 +124,7 @@ export function useConfirmController({ isOpen: snapshot.value !== 'idle', isPending, canClose, + canConfirm: snapshot.value === 'confirming' && !actionLock?.isPending, errorMessage: error ? errorText(error, errorFallback) : undefined, step: isPending && reverification?.visible ? 'verify' : 'confirm', reverification, @@ -139,12 +144,17 @@ export function useConfirmController({ if (snapshot.value !== 'confirming') { return; } - send({ - type: 'CONFIRM', - run: async () => { - await onConfirm(payload); - }, - }); + const run = async () => { + await onConfirm(payload); + }; + if (actionLock) { + const accepted = actionLock.tryRun(run); + if (accepted) { + send({ type: 'CONFIRM', run: () => accepted }); + } + return; + } + send({ type: 'CONFIRM', run }); }, }; } 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/blocks/confirmation/confirmation.tsx b/packages/mosaic/src/blocks/confirmation/confirmation.tsx index 02dd5b79491..962590eae06 100644 --- a/packages/mosaic/src/blocks/confirmation/confirmation.tsx +++ b/packages/mosaic/src/blocks/confirmation/confirmation.tsx @@ -39,7 +39,7 @@ export interface ConfirmationProps extends ConfirmBlockProps({ color = 'negative', ...props }: ConfirmationProps) { return ( - {({ title, description, actionLabel, cancelLabel, isPending, errorMessage, confirm }) => ( + {({ title, description, actionLabel, cancelLabel, isPending, canConfirm, errorMessage, confirm }) => ( <> {title} @@ -67,6 +67,7 @@ export function Confirmation({ color = 'negative', ...props }: C fullWidth color={color} isPending={isPending} + disabled={!canConfirm && !isPending} onClick={confirm} > {actionLabel} diff --git a/packages/mosaic/src/blocks/destructive/destructive.tsx b/packages/mosaic/src/blocks/destructive/destructive.tsx index 494a2d96fad..839ce134100 100644 --- a/packages/mosaic/src/blocks/destructive/destructive.tsx +++ b/packages/mosaic/src/blocks/destructive/destructive.tsx @@ -32,6 +32,7 @@ function DestructiveStep({ actionLabel, cancelLabel, isPending, + canConfirm, errorMessage, confirm, fieldLabel, @@ -98,7 +99,7 @@ function DestructiveStep({ fullWidth color='negative' isPending={isPending} - disabled={!isConfirmed} + disabled={!isConfirmed || !canConfirm} focusableWhenDisabled > {actionLabel} 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 46e05dfc421..81251d5cdae 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 @@ -7,6 +7,8 @@ import { Badge } from '../../components/badge'; import { Button } from '../../components/button'; import { Icon } from '../../components/icon'; import { Section } from '../../components/section'; +import { Spinner } from '../../components/spinner'; +import { useSpinDelay } from '../../hooks/use-spin-delay'; import { Plus } from '../../icons/glyphs/plus'; import { fill, useMessages } from '../../localization'; import { truncationStyles } from '../../styles/typography.styles'; @@ -23,6 +25,8 @@ export interface UserProfileContactListRowViewProps { onAdd?: () => void; onVerify?: (id: string) => void; pendingVerifyId?: string; + pendingPrimaryId?: string; + isDisabled?: boolean; onSetPrimary?: (id: string) => void; onRemove?: (item: UserProfileContact) => void; children?: ReactNode; @@ -35,6 +39,8 @@ export function UserProfileContactListRowView({ onAdd, onVerify, pendingVerifyId, + pendingPrimaryId, + isDisabled = false, onSetPrimary, onRemove, addAction, @@ -43,6 +49,7 @@ export function UserProfileContactListRowView({ children, }: UserProfileContactListRowViewProps) { const m = useMessages('userProfileContact'); + const shownPrimaryId = useSpinDelay(pendingPrimaryId ?? null, { minDuration: 0 }); const emptyDescription = m[kind].empty; return ( @@ -64,6 +71,7 @@ export function UserProfileContactListRowView({ size='sm' variant='outline' onClick={onAdd} + disabled={isDisabled} > {item.value} - {item.isDefault ? {m.primary} : null} + {item.isDefault ? ( + {m.primary} + ) : shownPrimaryId === item.id ? ( + + ) : null} {item.isVerified ? null : {m.unverified}} @@ -119,6 +134,8 @@ export function UserProfileContactListRowView({ 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 75b27675d32..ade3e344757 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 @@ -82,7 +82,7 @@ describe('the user profile email section', () => { expect(screen.queryByRole('menu')).toBeNull(); expect(screen.queryByRole('alertdialog')).toBeNull(); expect(screen.getByRole('button', { name: 'Add email' })).toBeDisabled(); - expect(await screen.findByRole('status', { name: 'Set as primary' })).toBeVisible(); + expect(await screen.findByRole('progressbar', { name: 'Set as primary' })).toBeVisible(); } finally { primary.release(); await waitFor(() => @@ -90,7 +90,7 @@ describe('the user profile email section', () => { ); } expect(screen.getByRole('button', { name: 'Manage bob@example.com' })).not.toHaveAttribute('aria-disabled', 'true'); - expect(screen.queryByRole('status', { name: 'Set as primary' })).toBeNull(); + expect(screen.queryByRole('progressbar', { name: 'Set as primary' })).toBeNull(); }); it('adds an address and verifies the code it was sent', async () => { 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..6f0bd477d76 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,6 +1,7 @@ import { useRef } from 'react'; import { useConfirmController } from '../../../blocks/confirm'; +import { useActionLock } from '../../../hooks/use-action-lock'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; import type { UserProfileEmail } from '../user-profile-contact.types'; @@ -23,7 +24,8 @@ export function useUserProfileEmailSectionController({ onCreate: onCreateEmail, reverification: createReverification, }); - const setPrimary = usePendingAction(); + const actionLock = useActionLock(); + const setPrimary = usePendingAction({ actionLock }); const rowRef = useRef(null); const removalFocus = useListRemovalFocus({ ids: emails.map(email => email.id), @@ -31,6 +33,7 @@ export function useUserProfileEmailSectionController({ fallback: () => rowRef.current?.querySelector('button:not([disabled])') ?? rowRef.current, }); const remove = useConfirmController({ + actionLock, onConfirm: email => removalFocus.remove(email.id), reverification: removeReverification, }); @@ -40,6 +43,8 @@ export function useUserProfileEmailSectionController({ canAdd: Boolean(onCreateEmail), verification, error: setPrimary.error, + isDisabled: actionLock.isPending, + pendingPrimaryId: setPrimary.pendingKey, rowRef, registerTrigger: removalFocus.registerTrigger, remove: onRemoveEmail ? remove : undefined, 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..88b7e698fe5 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 @@ -9,6 +9,8 @@ import type { UserProfileAddEmailController } from './user-profile-add-email.con export interface UserProfileEmailSectionViewProps { emails: UserProfileEmail[]; canAdd: boolean; + isDisabled: boolean; + pendingPrimaryId: string | undefined; 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 2e3de1fd1ce..a64231c60a4 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,8 @@ import type { UserProfileEmailSectionViewProps } from './user-profile-email-sect export function UserProfileEmailSectionView({ emails, canAdd, + isDisabled, + pendingPrimaryId, verification, error, rowRef, @@ -35,6 +37,7 @@ export function UserProfileEmailSectionView({ color='neutral' size='sm' variant='outline' + disabled={isDisabled} /> } > @@ -59,6 +62,8 @@ export function UserProfileEmailSectionView({ onRemove={remove ? email => remove.open(email) : undefined} onSetPrimary={onSetPrimary} onVerify={onVerify} + isDisabled={isDisabled} + pendingPrimaryId={pendingPrimaryId} pendingVerifyId={verification.pendingVerifyId} > {error} 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 cc86a87e320..7a5e711fae7 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 @@ -105,13 +105,13 @@ describe('the user profile phone section', () => { expect(screen.queryByRole('menu')).toBeNull(); expect(screen.queryByRole('alertdialog')).toBeNull(); expect(screen.getByRole('button', { name: 'Add phone number' })).toBeDisabled(); - expect(await screen.findByRole('status', { name: 'Set as primary' })).toBeVisible(); + expect(await screen.findByRole('progressbar', { name: 'Set as primary' })).toBeVisible(); } 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'); - expect(screen.queryByRole('status', { name: 'Set as primary' })).toBeNull(); + expect(screen.queryByRole('progressbar', { name: 'Set as primary' })).toBeNull(); }); it('lists the primary number first and marks it', async () => { 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..08cf24cbc8b 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 @@ -2,6 +2,7 @@ import { stringToFormattedPhoneString } from '@clerk/shared/phone'; import { useRef } from 'react'; import { useConfirmController } from '../../../blocks/confirm'; +import { useActionLock } from '../../../hooks/use-action-lock'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; import type { UserProfilePhone } from '../user-profile-contact.types'; @@ -23,7 +24,8 @@ export function useUserProfilePhoneSectionController({ onCreate: onCreatePhone, reverification: createReverification, }); - const setPrimary = usePendingAction(); + const actionLock = useActionLock(); + const setPrimary = usePendingAction({ actionLock }); const rowRef = useRef(null); const removalFocus = useListRemovalFocus({ ids: phones.map(phone => phone.id), @@ -31,6 +33,7 @@ export function useUserProfilePhoneSectionController({ fallback: () => rowRef.current?.querySelector('button:not([disabled])') ?? rowRef.current, }); const remove = useConfirmController({ + actionLock, onConfirm: phone => removalFocus.remove(phone.id), reverification: removeReverification, }); @@ -41,6 +44,8 @@ export function useUserProfilePhoneSectionController({ canAdd: Boolean(onCreatePhone), verification, error: setPrimary.error, + isDisabled: actionLock.isPending, + pendingPrimaryId: setPrimary.pendingKey, rowRef, registerTrigger: removalFocus.registerTrigger, remove: onRemovePhone ? remove : undefined, 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..d4abc3d5a6b 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 @@ -11,6 +11,8 @@ export interface UserProfilePhoneSectionViewProps { phones: UserProfilePhone[]; defaultPhoneCountry?: CountryIso; canAdd: boolean; + isDisabled: boolean; + pendingPrimaryId: string | undefined; 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 ffcfe7bf5dc..8e48ea107fb 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,8 @@ export function UserProfilePhoneSectionView({ phones, defaultPhoneCountry, canAdd, + isDisabled, + pendingPrimaryId, verification, error, rowRef, @@ -36,6 +38,7 @@ export function UserProfilePhoneSectionView({ color='neutral' size='sm' variant='outline' + disabled={isDisabled} /> } > @@ -60,6 +63,8 @@ export function UserProfilePhoneSectionView({ onRemove={remove ? phone => remove.open(phone) : undefined} onSetPrimary={onSetPrimary} onVerify={onVerify} + isDisabled={isDisabled} + pendingPrimaryId={pendingPrimaryId} pendingVerifyId={verification.pendingVerifyId} > {error} 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 e788761c532..86618a6285e 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 @@ -7,6 +7,7 @@ import { Badge } from '../../../components/badge'; import { SubmitButton } from '../../../components/button'; import { Icon } from '../../../components/icon'; import { Section } from '../../../components/section'; +import { Spinner } from '../../../components/spinner'; import { ArrowUpRight } from '../../../icons/glyphs/arrow-up-right'; import { fill, useMessages } from '../../../localization'; import { truncationStyles } from '../../../styles/typography.styles'; @@ -18,6 +19,7 @@ export function UserProfileWeb3WalletRowView({ wallet, triggerRef, isPending = false, + isPrimaryPending = false, isDisabled = false, onConnect, onVerify, @@ -27,6 +29,7 @@ export function UserProfileWeb3WalletRowView({ wallet: UserProfileWeb3Wallet | UserProfileWeb3Provider; triggerRef?: Ref; isPending?: boolean; + isPrimaryPending?: boolean; isDisabled?: boolean; onConnect?: (id: string) => void; onVerify?: (id: string) => void; @@ -68,7 +71,14 @@ export function UserProfileWeb3WalletRowView({ > {wallet.provider || shortAddress} - {linkedWallet?.isPrimary ? {m.primary} : null} + {linkedWallet?.isPrimary ? ( + {m.primary} + ) : isPrimaryPending ? ( + + ) : null} {linkedWallet && !linkedWallet.isVerified ? ( {isPending ? m.verifying : m.unverified} ) : null} 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 4fba28cb105..6daca8a1e94 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 @@ -2,6 +2,7 @@ import { useRef, useState } from 'react'; import { useConfirmController } from '../../../blocks/confirm'; import { usePanelTitle } from '../../../components/panel'; +import { useActionLock } from '../../../hooks/use-action-lock'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; import { useMessages } from '../../../localization'; @@ -45,7 +46,8 @@ export function useUserProfileWeb3WalletsController({ | 'createReverification' >) { const messages = useMessages('userProfileWeb3Wallets'); - const action = usePendingAction({ errorFallback: messages.errors.generic }); + const actionLock = useActionLock(); + const action = usePendingAction({ actionLock, errorFallback: messages.errors.generic }); const [picker, setPicker] = useState({ open: false }); const [owner, setOwner] = useState(); const sectionRef = useRef(null); @@ -58,8 +60,8 @@ export function useUserProfileWeb3WalletsController({ sectionRef.current ?? panelTitle(), }); - // TODO: Share the action lock during removal while preserving errors for the confirmation dialog. const removeWallet = useConfirmController({ + actionLock, onConfirm: wallet => removalFocus.remove(wallet.id), reverification: removeReverification, }); @@ -170,6 +172,8 @@ export function useUserProfileWeb3WalletsController({ ? action.error : undefined, pendingId, + pendingPrimaryId: owner?.kind === 'primary' ? pendingId : undefined, + isDisabled: actionLock.isPending, sectionRef, registerTrigger: removalFocus.registerTrigger, remove: remove ? removeWallet : undefined, 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..87c625e2391 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 @@ -37,6 +37,8 @@ export interface UserProfileWeb3WalletsSectionViewProps { sectionRef?: Ref; availableProviders?: UserProfileWeb3Provider[]; pendingId?: string; + pendingPrimaryId?: 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 bd259076163..f537fde2b57 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 @@ -1,6 +1,7 @@ import { Confirmation } from '../../../blocks/confirmation'; import { ReverificationDialog } from '../../../blocks/reverification'; import { Section } from '../../../components/section'; +import { useSpinDelay } from '../../../hooks/use-spin-delay'; import { fill, useMessages } from '../../../localization'; import { truncateWithEndVisible } from '../../../utils/truncate-text-with-end-visible'; import { UserProfileWeb3WalletRowView } from './user-profile-web3-wallet-row.view'; @@ -11,6 +12,8 @@ export function UserProfileWeb3WalletsSectionView({ sectionRef, availableProviders = [], pendingId, + pendingPrimaryId, + isDisabled = false, error, onConnect, onVerify, @@ -25,7 +28,8 @@ export function UserProfileWeb3WalletsSectionView({ }: UserProfileWeb3WalletsSectionViewProps) { const m = useMessages('userProfileWeb3Wallets'); const hasRows = wallets.length > 0 || (availableProviders.length > 0 && Boolean(onConnect)) || Boolean(error); - const isBusy = pendingId !== undefined; + const isBusy = isDisabled || pendingId !== undefined; + const shownPrimaryId = useSpinDelay(pendingPrimaryId ?? null, { minDuration: 0 }); return ( <> @@ -47,6 +51,7 @@ export function UserProfileWeb3WalletsSectionView({ wallet={wallet} triggerRef={registerTrigger(wallet.id)} isPending={pendingId === wallet.id} + isPrimaryPending={shownPrimaryId === wallet.id} isDisabled={isBusy} onVerify={onVerify} onSetPrimary={onSetPrimary} diff --git a/packages/mosaic/src/hooks/__tests__/use-action-lock.test.tsx b/packages/mosaic/src/hooks/__tests__/use-action-lock.test.tsx new file mode 100644 index 00000000000..e573e5af535 --- /dev/null +++ b/packages/mosaic/src/hooks/__tests__/use-action-lock.test.tsx @@ -0,0 +1,139 @@ +import { act, renderHook } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import { deferred } from '../../__tests__/async'; +import { useConfirmController } from '../../blocks/confirm'; +import { SaveError } from '../../utils/errors'; +import { useActionLock } from '../use-action-lock'; +import { usePendingAction } from '../use-pending-action'; + +function useOwners(onConfirm: () => Promise) { + const actionLock = useActionLock(); + const primary = usePendingAction({ actionLock }); + const remove = useConfirmController({ + actionLock, + onConfirm, + reverification: { status: 'loading', visible: true, onCancel: vi.fn() }, + }); + return { actionLock, primary, remove }; +} + +describe('shared action admission', () => { + it('refuses removal in the same stack as primary without changing the dialog', async () => { + const pending = deferred(); + const onConfirm = vi.fn(() => Promise.resolve()); + const { result } = renderHook(() => useOwners(onConfirm)); + act(() => result.current.remove.onOpenChange(true)); + const owners = result.current; + + act(() => { + void owners.primary.run('primary', () => pending.promise); + owners.remove.confirm(); + }); + + expect(onConfirm).not.toHaveBeenCalled(); + expect(result.current.remove.isOpen).toBe(true); + expect(result.current.remove.isPending).toBe(false); + expect(result.current.remove.canConfirm).toBe(false); + await act(() => pending.resolve()); + expect(result.current.remove.canConfirm).toBe(true); + await act(() => result.current.remove.confirm()); + expect(onConfirm).toHaveBeenCalledOnce(); + expect(result.current.remove.isOpen).toBe(false); + }); + + it('refuses primary in the same stack as removal without changing primary metadata', async () => { + const pending = deferred(); + const primary = vi.fn(); + const { result } = renderHook(() => useOwners(() => pending.promise)); + act(() => result.current.remove.onOpenChange(true)); + const owners = result.current; + + await act(async () => { + owners.remove.confirm(); + expect(await owners.primary.run('primary', primary)).toBe(false); + }); + + expect(primary).not.toHaveBeenCalled(); + expect(result.current.primary.lastKey).toBeUndefined(); + expect(result.current.primary.error).toBeUndefined(); + expect(result.current.actionLock.isPending).toBe(true); + await act(() => pending.resolve()); + await act(async () => expect(await owners.primary.run('primary', primary)).toBe(true)); + expect(primary).toHaveBeenCalledOnce(); + }); + + it('keeps a rejected removal error when another owner makes confirmation busy', async () => { + const pending = deferred(); + const failure = new SaveError({ global: { code: 'action_blocked', message: 'Removal refused.' } }); + const onConfirm = vi.fn(() => Promise.reject(failure)); + const { result } = renderHook(() => useOwners(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.primary.error).toBeUndefined(); + + act(() => { + void result.current.primary.run('primary', () => pending.promise); + 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('holds admission after cancelling observation until the underlying removal settles', async () => { + const pending = deferred(); + const primary = vi.fn(); + const { result } = renderHook(() => useOwners(() => pending.promise)); + act(() => result.current.remove.onOpenChange(true)); + act(() => result.current.remove.confirm()); + act(() => result.current.remove.onOpenChange(false)); + expect(result.current.remove.isOpen).toBe(false); + await act(async () => expect(await result.current.primary.run('primary', primary)).toBe(false)); + expect(primary).not.toHaveBeenCalled(); + expect(result.current.actionLock.isPending).toBe(true); + + await act(() => pending.reject(new Error('Late removal failure'))); + expect(result.current.actionLock.isPending).toBe(false); + expect(result.current.remove.errorMessage).toBeUndefined(); + await act(async () => expect(await result.current.primary.run('primary', primary)).toBe(true)); + }); +}); + +describe('useActionLock', () => { + it('returns the accepted result and refuses reentrant work synchronously', async () => { + const { result } = renderHook(() => useActionLock()); + const other = vi.fn(); + await act(async () => { + const accepted = result.current.tryRun(() => { + expect(result.current.tryRun(other)).toBeUndefined(); + return 42; + }); + expect(await accepted).toBe(42); + }); + expect(other).not.toHaveBeenCalled(); + expect(result.current.isPending).toBe(false); + }); + + it.each(['throw', 'reject'])('preserves the original %s and releases admission', async mode => { + const { result } = renderHook(() => useActionLock()); + const failure = new Error('Original failure'); + await act(async () => { + const accepted = result.current.tryRun(() => { + if (mode === 'throw') { + throw failure; + } + return Promise.reject(failure); + }); + await expect(accepted).rejects.toBe(failure); + }); + expect(result.current.isPending).toBe(false); + await act(async () => expect(await result.current.tryRun(() => 'retry')).toBe('retry')); + }); +}); diff --git a/packages/mosaic/src/hooks/use-action-lock.ts b/packages/mosaic/src/hooks/use-action-lock.ts new file mode 100644 index 00000000000..e112d93ad84 --- /dev/null +++ b/packages/mosaic/src/hooks/use-action-lock.ts @@ -0,0 +1,29 @@ +import { useRef, useState } from 'react'; + +export interface ActionLock { + isPending: boolean; + tryRun: (action: () => T | Promise) => Promise | undefined; +} + +export function useActionLock(): ActionLock { + const running = useRef(false); + const [isPending, setIsPending] = useState(false); + + const tryRun = (action: () => T | Promise): Promise | undefined => { + if (running.current) { + return undefined; + } + running.current = true; + setIsPending(true); + return (async () => { + try { + return await action(); + } finally { + running.current = false; + setIsPending(false); + } + })(); + }; + + return { isPending, tryRun }; +} 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..b898cdf274c 100644 --- a/packages/mosaic/src/hooks/use-pending-action.ts +++ b/packages/mosaic/src/hooks/use-pending-action.ts @@ -1,15 +1,21 @@ import { isReverificationCancelledError } from '@clerk/shared/error'; -import { useRef, useState } from 'react'; +import { useState } from 'react'; import type { ErrorDescription } from '../localization'; import { useErrorText } from '../localization'; import { toLocalizableError } from '../utils/errors'; +import type { ActionLock } from './use-action-lock'; +import { useActionLock } from './use-action-lock'; export interface PendingActionOptions { /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ errorFallback?: string; } +export interface PendingActionHookOptions extends PendingActionOptions { + actionLock?: ActionLock; +} + export interface PendingAction { run: (key: TKey, action: () => Promise | void, options?: PendingActionOptions) => Promise; pendingKey: TKey | undefined; @@ -32,33 +38,34 @@ interface PendingActionFailure { */ export function usePendingAction({ errorFallback, -}: PendingActionOptions = {}): PendingAction { + actionLock, +}: PendingActionHookOptions = {}): PendingAction { const errorText = useErrorText(); const [pendingKey, setPendingKey] = useState(); const [lastKey, setLastKey] = useState(); const [failure, setFailure] = useState>(); - const running = useRef(false); + const localLock = useActionLock(); + const lock = actionLock ?? localLock; const run = async (key: TKey, action: () => Promise | void, options: PendingActionOptions = {}) => { - if (running.current) { - return false; - } - running.current = true; - setPendingKey(key); - setLastKey(key); - setFailure(undefined); - try { - await action(); - return true; - } catch (cause) { - if (!isReverificationCancelledError(cause)) { - setFailure({ key, error: toLocalizableError(cause), errorFallback: options.errorFallback }); - } - return false; - } finally { - running.current = false; - setPendingKey(undefined); - } + return ( + lock.tryRun(async () => { + setPendingKey(key); + setLastKey(key); + setFailure(undefined); + try { + await action(); + return true; + } catch (cause) { + if (!isReverificationCancelledError(cause)) { + setFailure({ key, error: toLocalizableError(cause), errorFallback: options.errorFallback }); + } + return false; + } finally { + setPendingKey(undefined); + } + }) ?? false + ); }; return { From b5642a60db9c527c3f84e08d9bf60e57fb87d33a Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 20:03:39 -0600 Subject: [PATCH 03/12] fix(mosaic): keep primary actions pending without reordering rows --- .../user-profile-contact.utils.test.ts | 12 +++++--- .../user-profile-contact-list-row.view.tsx | 18 ++++-------- .../user-profile-contact.utils.ts | 13 ++++----- ...ser-profile-email-section.feature.test.tsx | 18 ++++++++---- ...ser-profile-phone-section.feature.test.tsx | 24 +++++++++++----- ...file-web3-wallets-section.feature.test.tsx | 28 ++++++++++++++++--- ...profile-web3-wallets-section.model.test.ts | 17 +++++++++-- .../user-profile-web3-wallet-row.view.tsx | 16 ++++------- ...user-profile-web3-wallets-section.model.ts | 10 +++---- .../user-profile-web3-wallets-section.tsx | 2 ++ ...user-profile-web3-wallets-section.view.tsx | 4 +-- 11 files changed, 100 insertions(+), 62 deletions(-) diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-contact.utils.test.ts b/packages/mosaic/src/features/user-profile/__tests__/user-profile-contact.utils.test.ts index 3c2604a96dc..b70ad875de4 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-contact.utils.test.ts +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-contact.utils.test.ts @@ -9,26 +9,30 @@ function contact(id: string, status: Contact['verification']['status'], expireAt } describe('toContacts', () => { - it('orders primary first, then verified, then pending by expiry, then never started', () => { + it('orders verified, then pending by expiry, then never started regardless of primary', () => { const items = [ contact('idn_unstarted', null), contact('idn_pending_late', 'unverified', new Date(2026, 0, 2)), contact('idn_verified_b', 'verified'), - contact('idn_primary', 'verified'), + contact('idn_verified_z', 'verified'), contact('idn_pending_soon', 'unverified', new Date(2026, 0, 1)), contact('idn_verified_a', 'verified'), ]; - const contacts = toContacts(items, 'idn_primary', item => item.id); + const contacts = toContacts(items, 'idn_verified_z', item => item.id); + const changedPrimary = toContacts(items, 'idn_verified_b', item => item.id); expect(contacts.map(item => item.id)).toEqual([ - 'idn_primary', 'idn_verified_a', 'idn_verified_b', + 'idn_verified_z', 'idn_pending_soon', 'idn_pending_late', 'idn_unstarted', ]); + expect(changedPrimary.map(item => item.id)).toEqual(contacts.map(item => item.id)); + expect(changedPrimary.find(item => item.id === 'idn_verified_b')?.isDefault).toBe(true); + expect(changedPrimary.find(item => item.id === 'idn_verified_z')?.isDefault).toBe(false); }); it('marks which contact is the primary one and which are verified', () => { 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 81251d5cdae..a55e508de6f 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 @@ -7,8 +7,6 @@ import { Badge } from '../../components/badge'; import { Button } from '../../components/button'; import { Icon } from '../../components/icon'; import { Section } from '../../components/section'; -import { Spinner } from '../../components/spinner'; -import { useSpinDelay } from '../../hooks/use-spin-delay'; import { Plus } from '../../icons/glyphs/plus'; import { fill, useMessages } from '../../localization'; import { truncationStyles } from '../../styles/typography.styles'; @@ -49,7 +47,6 @@ export function UserProfileContactListRowView({ children, }: UserProfileContactListRowViewProps) { const m = useMessages('userProfileContact'); - const shownPrimaryId = useSpinDelay(pendingPrimaryId ?? null, { minDuration: 0 }); const emptyDescription = m[kind].empty; return ( @@ -102,7 +99,11 @@ export function UserProfileContactListRowView({ status: pendingVerifyId === item.id ? '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: pendingPrimaryId === item.id ? 'pending' : 'idle', + }); } if (onRemove) { @@ -118,14 +119,7 @@ export function UserProfileContactListRowView({ {item.value} - {item.isDefault ? ( - {m.primary} - ) : shownPrimaryId === item.id ? ( - - ) : null} + {item.isDefault ? {m.primary} : null} {item.isVerified ? null : {m.unverified}} diff --git a/packages/mosaic/src/features/user-profile/user-profile-contact.utils.ts b/packages/mosaic/src/features/user-profile/user-profile-contact.utils.ts index 56b2f78d083..39c6a9613c0 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-contact.utils.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-contact.utils.ts @@ -30,11 +30,10 @@ export function byId(items: T[], id: string, kind: str type Identification = { id: string; verification: Pick }; -export function sortByVerification(items: T[], primaryId: string | null): T[] { - const others = items.filter(item => item.id !== primaryId); - const verified = others.filter(item => item.verification.status === 'verified'); - const pending = others.filter(item => item.verification.status && item.verification.status !== 'verified'); - const unstarted = others.filter(item => !item.verification.status); +export function sortByVerification(items: T[]): T[] { + const verified = items.filter(item => item.verification.status === 'verified'); + const pending = items.filter(item => item.verification.status && item.verification.status !== 'verified'); + const unstarted = items.filter(item => !item.verification.status); verified.sort((a, b) => a.id.localeCompare(b.id)); pending.sort((a, b) => { @@ -43,7 +42,7 @@ export function sortByVerification(items: T[], primary return aExpireAt && bExpireAt ? aExpireAt.getTime() - bExpireAt.getTime() : 0; }); - return [...items.filter(item => item.id === primaryId), ...verified, ...pending, ...unstarted]; + return [...verified, ...pending, ...unstarted]; } export function toContacts( @@ -51,7 +50,7 @@ export function toContacts( primaryId: string | null, value: (item: T) => string, ): UserProfileContact[] { - return sortByVerification(items, primaryId).map(item => ({ + return sortByVerification(items).map(item => ({ id: item.id, value: value(item), isDefault: item.id === primaryId, 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 ade3e344757..0e41f723cf4 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 @@ -68,21 +68,24 @@ async function enterCode(actor: Actor, code: string) { } describe('the user profile email section', () => { - it('closes the menu and blocks conflicting actions while setting an address as primary', async () => { + it('holds the menu and row order while setting an address as primary', async () => { const other = fapiEmailAddress({ id: 'idn_other', email_address: 'bob@example.com', verification: VERIFIED }); const { actor } = await renderSection(signedInWithEmails([PRIMARY, other])); + const initialOrder = emailsListed(); const primary = holdRequests('post', '/v1/me'); try { await manageEmail(actor, 'bob@example.com', 'Set as primary'); await waitFor(() => expect(primary.requests).toHaveLength(1)); - await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + 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'); - await actor.click(manage); - expect(screen.queryByRole('menu')).toBeNull(); + 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 screen.findByRole('progressbar', { name: 'Set as primary' })).toBeVisible(); + expect(await within(item).findByRole('progressbar', { name: 'Set as primary' })).toBeVisible(); + expect(emailsListed()).toEqual(initialOrder); } finally { primary.release(); await waitFor(() => @@ -90,6 +93,8 @@ describe('the user profile email section', () => { ); } expect(screen.getByRole('button', { name: 'Manage bob@example.com' })).not.toHaveAttribute('aria-disabled', 'true'); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + expect(emailsListed()).toEqual(initialOrder); expect(screen.queryByRole('progressbar', { name: 'Set as primary' })).toBeNull(); }); @@ -365,6 +370,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')); }); @@ -478,7 +484,7 @@ describe('email reverification', () => { await screen.findByRole('dialog', { name: 'Verify your email' }); await enterCode(actor, VERIFICATION_CODE); await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); - expect(emailsListed()).toEqual(['alice@example.com', 'new@example.com']); + expect(emailsListed()).toEqual(['new@example.com', 'alice@example.com']); }); it('closes quietly when confirmation is dismissed and starts clean on reopen', async () => { 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 7a5e711fae7..a0e56983b35 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 @@ -92,29 +92,37 @@ async function enterCode(actor: Actor, code: string) { } describe('the user profile phone section', () => { - it('closes the menu and blocks conflicting actions while setting a number as primary', async () => { + it('holds the menu and row order while setting a number as primary', async () => { const { actor } = await renderSection(signedIn([HOME, WORK])); + const initialOrder = listed(); const primary = holdRequests('post', '/v1/me'); try { await manage(actor, WORK_LABEL, 'Set as primary'); await waitFor(() => expect(primary.requests).toHaveLength(1)); - await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + 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'); - await actor.click(manageWork); - expect(screen.queryByRole('menu')).toBeNull(); + 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 screen.findByRole('progressbar', { name: 'Set as primary' })).toBeVisible(); + expect(await within(item).findByRole('progressbar', { name: 'Set as primary' })).toBeVisible(); + expect(listed()).toEqual(initialOrder); } 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(initialOrder); expect(screen.queryByRole('progressbar', { name: 'Set as primary' })).toBeNull(); }); - it('lists the primary number first and marks it', async () => { + it('lists verified numbers in order and marks the primary', async () => { await renderSection(signedIn([HOME, WORK])); expect(listed()).toEqual([HOME_LABEL, WORK_LABEL]); @@ -203,7 +211,8 @@ describe('the user profile phone section', () => { await manage(actor, WORK_LABEL, 'Set as primary'); - await waitFor(() => expect(listed()).toEqual([WORK_LABEL, HOME_LABEL])); + await waitFor(() => expect(within(row()).getByText(WORK_LABEL).parentElement).toHaveTextContent('Primary')); + expect(listed()).toEqual([HOME_LABEL, WORK_LABEL]); expect(within(row()).getByText(WORK_LABEL).parentElement).toHaveTextContent('Primary'); }); @@ -502,6 +511,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-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 cb9fc8857d2..4f120f3ad34 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) { @@ -604,6 +623,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')); @@ -735,14 +755,14 @@ describe('Web3 wallets', () => { primary_web3_wallet_id: 'wallet_2', }); const user = userEvent.setup(); - const first = screen.getAllByRole('button', { name: 'Manage MetaMask' })[1]; + const first = screen.getAllByRole('button', { name: 'Manage MetaMask' })[0]; first.focus(); await user.keyboard('{Enter}'); await user.click(await screen.findByRole('menuitem', { name: 'Remove wallet' })); await user.keyboard('{Escape}'); await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(2); - await openMenu(user, screen.getAllByRole('button', { name: 'Manage MetaMask' })[0]); + await openMenu(user, screen.getAllByRole('button', { name: 'Manage MetaMask' })[1]); await user.click(screen.getByRole('menuitem', { name: 'Remove wallet' })); expect(screen.getByRole('alertdialog')).toHaveTextContent('0xabcd...7890'); await user.click(screen.getByRole('button', { name: 'Remove' })); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.model.test.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.model.test.ts index b35b021cd21..362bdc035d7 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.model.test.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.model.test.ts @@ -122,7 +122,7 @@ describe('Web3 wallet projection', () => { } }); - it('WEB3-04 orders primary, verified, then unverified wallets without mutating the input', () => { + it('WEB3-04 orders verified, then unverified wallets without moving the primary or mutating the input', () => { const wallets = Object.freeze([ { ...metamask, @@ -134,7 +134,7 @@ describe('Web3 wallet projection', () => { { ...metamask, id: 'primary', - verification: { ...metamask.verification, status: 'unverified' as const }, + verification: { ...metamask.verification, status: null }, }, { ...metamask, @@ -148,17 +148,28 @@ describe('Web3 wallet projection', () => { enabledStrategies: ['web3_metamask_signature'], allowCreation: false, }); + const changedPrimary = projectWeb3Wallets({ + wallets, + primaryId: 'unverified_later', + enabledStrategies: ['web3_metamask_signature'], + allowCreation: false, + }); expect(result.status).toBe('ready'); if (result.status === 'ready') { expect(result.wallets.map(wallet => wallet.id)).toEqual([ - 'primary', 'verified_a', 'verified_z', 'unverified_earlier', 'unverified_later', + 'primary', ]); expect(result.availableProviders).toEqual([]); } + if (result.status === 'ready' && changedPrimary.status === 'ready') { + expect(changedPrimary.wallets.map(wallet => wallet.id)).toEqual(result.wallets.map(wallet => wallet.id)); + expect(changedPrimary.wallets.find(wallet => wallet.id === 'unverified_later')?.isPrimary).toBe(true); + expect(changedPrimary.wallets.find(wallet => wallet.id === 'primary')?.isPrimary).toBe(false); + } expect(wallets.map(wallet => wallet.id)).toEqual([ 'unverified_later', 'verified_z', 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 86618a6285e..553f747bf05 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 @@ -7,7 +7,6 @@ import { Badge } from '../../../components/badge'; import { SubmitButton } from '../../../components/button'; import { Icon } from '../../../components/icon'; import { Section } from '../../../components/section'; -import { Spinner } from '../../../components/spinner'; import { ArrowUpRight } from '../../../icons/glyphs/arrow-up-right'; import { fill, useMessages } from '../../../localization'; import { truncationStyles } from '../../../styles/typography.styles'; @@ -46,7 +45,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: isPrimaryPending ? 'pending' : 'idle', + }); } if (linkedWallet && onRemove && linkedWallet.canRemove !== false) { actions.push({ label: m.remove, color: 'negative', onClick: () => onRemove(linkedWallet) }); @@ -71,14 +74,7 @@ export function UserProfileWeb3WalletRowView({ > {wallet.provider || shortAddress} - {linkedWallet?.isPrimary ? ( - {m.primary} - ) : isPrimaryPending ? ( - - ) : null} + {linkedWallet?.isPrimary ? {m.primary} : null} {linkedWallet && !linkedWallet.isVerified ? ( {isPending ? m.verifying : m.unverified} ) : null} diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts index 9da99dda1c5..965a41e7bfe 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts @@ -45,13 +45,11 @@ export function projectWeb3Wallets({ wallet.verification.strategy === 'admin' || WEB3_PROVIDERS.some(provider => provider.strategy === wallet.verification.strategy), ); - const primary = knownWallets.filter(wallet => wallet.id === primaryId); - const remaining = knownWallets.filter(wallet => wallet.id !== primaryId); - const verified = remaining.filter(wallet => wallet.verification.status === 'verified'); - const unverified = remaining.filter( + const verified = knownWallets.filter(wallet => wallet.verification.status === 'verified'); + const unverified = knownWallets.filter( wallet => wallet.verification.status !== null && wallet.verification.status !== 'verified', ); - const withoutStatus = remaining.filter(wallet => wallet.verification.status === null); + const withoutStatus = knownWallets.filter(wallet => wallet.verification.status === null); verified.sort((first, second) => first.id.localeCompare(second.id)); unverified.sort((first, second) => { @@ -60,7 +58,7 @@ export function projectWeb3Wallets({ return firstExpiry && secondExpiry ? firstExpiry.getTime() - secondExpiry.getTime() : 0; }); - const sorted = [...primary, ...verified, ...unverified, ...withoutStatus]; + const sorted = [...verified, ...unverified, ...withoutStatus]; const connectedStrategies = new Set(wallets.map(wallet => wallet.verification.strategy)); 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..9a9eb2cab3d 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 @@ -35,6 +35,8 @@ function Web3Wallets({ model }: { model: ReadyWeb3WalletsModel }) { sectionRef={controller.sectionRef} availableProviders={controller.availableProviders} pendingId={controller.pendingId} + pendingPrimaryId={controller.pendingPrimaryId} + 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.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx index f537fde2b57..aa2b3582492 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 @@ -1,7 +1,6 @@ import { Confirmation } from '../../../blocks/confirmation'; import { ReverificationDialog } from '../../../blocks/reverification'; import { Section } from '../../../components/section'; -import { useSpinDelay } from '../../../hooks/use-spin-delay'; import { fill, useMessages } from '../../../localization'; import { truncateWithEndVisible } from '../../../utils/truncate-text-with-end-visible'; import { UserProfileWeb3WalletRowView } from './user-profile-web3-wallet-row.view'; @@ -29,7 +28,6 @@ export function UserProfileWeb3WalletsSectionView({ const m = useMessages('userProfileWeb3Wallets'); const hasRows = wallets.length > 0 || (availableProviders.length > 0 && Boolean(onConnect)) || Boolean(error); const isBusy = isDisabled || pendingId !== undefined; - const shownPrimaryId = useSpinDelay(pendingPrimaryId ?? null, { minDuration: 0 }); return ( <> @@ -51,7 +49,7 @@ export function UserProfileWeb3WalletsSectionView({ wallet={wallet} triggerRef={registerTrigger(wallet.id)} isPending={pendingId === wallet.id} - isPrimaryPending={shownPrimaryId === wallet.id} + isPrimaryPending={pendingPrimaryId === wallet.id} isDisabled={isBusy} onVerify={onVerify} onSetPrimary={onSetPrimary} From 29ba31938b0c2dd428b3cc5e3117a8744db8bccb Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 20:20:27 -0600 Subject: [PATCH 04/12] refactor(mosaic): pass confirmation disabled state directly --- packages/mosaic/docs/controllers.md | 4 ++-- .../src/blocks/confirm/confirm-shell.tsx | 2 -- .../src/blocks/confirm/confirm.controller.ts | 2 -- .../__tests__/confirmation.feature.test.tsx | 20 ++++++++++++++++++- .../src/blocks/confirmation/confirmation.tsx | 7 ++++--- .../src/blocks/destructive/destructive.tsx | 3 +-- .../user-profile-email-section.view.tsx | 1 + .../user-profile-phone-section.view.tsx | 1 + ...user-profile-web3-wallets-section.view.tsx | 1 + .../hooks/__tests__/use-action-lock.test.tsx | 4 ++-- 10 files changed, 31 insertions(+), 14 deletions(-) diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index 247a4f21012..890e0c1a4a3 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -71,8 +71,8 @@ Opening a dialog does not acquire the lock. The lock owns no errors or row IDs. `usePendingAction` keeps its inline errors and boolean result; refused runs return `false` without changing metadata. `useConfirmController` keeps removal errors in its dialog; refusal leaves the dialog and its error unchanged. -Its `canConfirm` projection disables only the confirmation submit control. -Admission still runs through the lock, not the rendered projection. Without an +Pass the section's pending state to `Confirmation.disabled` to disable its submit +control while another action runs. Admission still runs through the lock. Without an injected lock, each owner retains its existing independent behavior. `actionLock` is a hook option, not a per-run option. Project its `isPending` to diff --git a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx index 6c2aff082e6..9c8b094194e 100644 --- a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx +++ b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx @@ -34,7 +34,6 @@ export interface ConfirmStepProps { actionLabel: string; cancelLabel: string; isPending: boolean; - canConfirm: boolean; errorMessage: string | undefined; confirm: () => void; } @@ -69,7 +68,6 @@ function ConfirmCard({ actionLabel: resolve(actionLabel, payload), cancelLabel, isPending: controller.isPending, - canConfirm: controller.canConfirm, errorMessage: controller.errorMessage, confirm: () => controller.confirm(payload), }); diff --git a/packages/mosaic/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts index 18c70af44b1..faf04107a81 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -74,7 +74,6 @@ export interface ConfirmController { isPending: boolean; /** Whether the dialog can close right now. False while the action is past the point of cancelling */ canClose: boolean; - canConfirm: boolean; errorMessage: string | undefined; /** Which card the dialog shows: the confirm step, or the reverification the action was interrupted by. */ step: ConfirmStep; @@ -124,7 +123,6 @@ export function useConfirmController({ isOpen: snapshot.value !== 'idle', isPending, canClose, - canConfirm: snapshot.value === 'confirming' && !actionLock?.isPending, errorMessage: error ? errorText(error, errorFallback) : undefined, step: isPending && reverification?.visible ? 'verify' : 'confirm', reverification, 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 74d87837077..b256db420ed 100644 --- a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx +++ b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx @@ -38,7 +38,11 @@ function Host({ async function renderAccount( options: ConfirmControllerOptions, - { localization, description }: { localization?: MosaicLocalization; description?: ReactNode } = {}, + { + localization, + description, + disabled, + }: { localization?: MosaicLocalization; description?: ReactNode; disabled?: boolean } = {}, ) { const ref: { current: ConfirmController | undefined } = { current: undefined }; serveFapi({ client: fapiClient([session]) }); @@ -53,6 +57,7 @@ async function renderAccount( title={TITLE} description={description ?? 'Google will be removed from this account.'} actionLabel='Remove' + disabled={disabled} /> ); }} @@ -70,6 +75,19 @@ async function renderAccount( const confirmButton = () => screen.getByRole('button', { name: 'Remove' }); describe('Confirmation', () => { + it('disables confirmation while leaving Cancel usable', async () => { + const onConfirm = vi.fn(() => Promise.resolve()); + const { user, open, closed } = await renderAccount({ onConfirm }, { disabled: true }); + await open(); + + expect(confirmButton()).toBeDisabled(); + await user.click(confirmButton()); + expect(onConfirm).not.toHaveBeenCalled(); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + await closed(); + }); + it('opens from its trigger, confirms, closes once the action lands, and hands focus back', async () => { const run = deferred(); const onConfirm = vi.fn(() => run.promise); diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.tsx b/packages/mosaic/src/blocks/confirmation/confirmation.tsx index 962590eae06..0387ddec10d 100644 --- a/packages/mosaic/src/blocks/confirmation/confirmation.tsx +++ b/packages/mosaic/src/blocks/confirmation/confirmation.tsx @@ -10,6 +10,7 @@ export type ConfirmationColor = 'negative' | 'primary'; export interface ConfirmationProps extends ConfirmBlockProps { /** The weight the confirming button carries. An action that can be undone takes `primary` (default: `negative`) */ color?: ConfirmationColor; + disabled?: boolean; } /** @@ -36,10 +37,10 @@ export interface ConfirmationProps extends ConfirmBlockProps */ -export function Confirmation({ color = 'negative', ...props }: ConfirmationProps) { +export function Confirmation({ color = 'negative', disabled, ...props }: ConfirmationProps) { return ( - {({ title, description, actionLabel, cancelLabel, isPending, canConfirm, errorMessage, confirm }) => ( + {({ title, description, actionLabel, cancelLabel, isPending, errorMessage, confirm }) => ( <> {title} @@ -67,7 +68,7 @@ export function Confirmation({ color = 'negative', ...props }: C fullWidth color={color} isPending={isPending} - disabled={!canConfirm && !isPending} + disabled={disabled && !isPending} onClick={confirm} > {actionLabel} diff --git a/packages/mosaic/src/blocks/destructive/destructive.tsx b/packages/mosaic/src/blocks/destructive/destructive.tsx index 839ce134100..494a2d96fad 100644 --- a/packages/mosaic/src/blocks/destructive/destructive.tsx +++ b/packages/mosaic/src/blocks/destructive/destructive.tsx @@ -32,7 +32,6 @@ function DestructiveStep({ actionLabel, cancelLabel, isPending, - canConfirm, errorMessage, confirm, fieldLabel, @@ -99,7 +98,7 @@ function DestructiveStep({ fullWidth color='negative' isPending={isPending} - disabled={!isConfirmed || !canConfirm} + disabled={!isConfirmed} focusableWhenDisabled > {actionLabel} 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 a64231c60a4..6cee98a1799 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 @@ -79,6 +79,7 @@ export function UserProfileEmailSectionView({ {remove ? ( fill(email.isVerified ? m.email.removeDialog.verifiedDescription : m.email.removeDialog.description, { 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 8e48ea107fb..3acdf8c8bc1 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 @@ -81,6 +81,7 @@ export function UserProfilePhoneSectionView({ {remove ? ( fill(phone.isVerified ? m.phone.removeDialog.verifiedDescription : m.phone.removeDialog.description, { 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 aa2b3582492..fdd044c14d8 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 @@ -76,6 +76,7 @@ export function UserProfileWeb3WalletsSectionView({ {remove ? ( fill(wallet.isVerified ? m.removeDialog.verifiedDescription : m.removeDialog.description, { diff --git a/packages/mosaic/src/hooks/__tests__/use-action-lock.test.tsx b/packages/mosaic/src/hooks/__tests__/use-action-lock.test.tsx index e573e5af535..4d589c1d23a 100644 --- a/packages/mosaic/src/hooks/__tests__/use-action-lock.test.tsx +++ b/packages/mosaic/src/hooks/__tests__/use-action-lock.test.tsx @@ -34,9 +34,9 @@ describe('shared action admission', () => { expect(onConfirm).not.toHaveBeenCalled(); expect(result.current.remove.isOpen).toBe(true); expect(result.current.remove.isPending).toBe(false); - expect(result.current.remove.canConfirm).toBe(false); + expect(result.current.actionLock.isPending).toBe(true); await act(() => pending.resolve()); - expect(result.current.remove.canConfirm).toBe(true); + expect(result.current.actionLock.isPending).toBe(false); await act(() => result.current.remove.confirm()); expect(onConfirm).toHaveBeenCalledOnce(); expect(result.current.remove.isOpen).toBe(false); From aad2c1b4ac93f23e8fe182dc94ed03dc91f81055 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 20:23:02 -0600 Subject: [PATCH 05/12] docs(mosaic): trim shared action lock guidance --- packages/mosaic/docs/controllers.md | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index 890e0c1a4a3..8207d6e496d 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -51,7 +51,7 @@ return { }; ``` -When a direct action and a confirmation must share admission, create one +When a direct action and a confirmation must not run together, create one `useActionLock` (`src/hooks/use-action-lock.ts`) and inject it into both owners: ```ts @@ -63,24 +63,10 @@ const remove = useConfirmController({ }); ``` -The lock admits synchronously, so two calls in one event cannot both start. -Its `tryRun` returns `undefined` without invoking busy work, or a promise that -preserves the accepted action's result or rejection. It stays held until the -actual action settles, even if a dialog closes and stops observing the action. -Opening a dialog does not acquire the lock. The lock owns no errors or row IDs. -`usePendingAction` keeps its inline errors and boolean result; refused runs -return `false` without changing metadata. `useConfirmController` keeps removal -errors in its dialog; refusal leaves the dialog and its error unchanged. -Pass the section's pending state to `Confirmation.disabled` to disable its submit -control while another action runs. Admission still runs through the lock. Without an -injected lock, each owner retains its existing independent behavior. - -`actionLock` is a hook option, not a per-run option. Project its `isPending` to -the controls in the shared section. For contact primary changes, close the menu -on selection by leaving `ActionMenu.status` undefined, disable Add and keep -Manage disabled but focusable, and show a delayed row spinner beside the value. -Use one `useSpinDelay` outside the row map with `minDuration: 0`, so feedback ends -with the request. The Primary badge replaces the spinner after success. +Pass `actionLock.isPending` to the section controls' `disabled` props, including +`Confirmation.disabled`. The lock prevents overlapping requests even before React +renders the disabled controls, and stays held until the action settles. Each owner +keeps its existing error handling. `actionLock` is a hook option, not a per-run option. `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 From 625bbc3a6b2ebb36eee71cd9fa7a48efa6290eb0 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 11:24:36 -0600 Subject: [PATCH 06/12] refactor(mosaic): share pending ownership for identifier actions --- packages/mosaic/AGENTS.md | 54 ++++++------ packages/mosaic/docs/controllers.md | 20 +++-- .../src/blocks/confirm/confirm.controller.ts | 9 +- .../user-profile-email-section.controller.ts | 16 ++-- .../user-profile-phone-section.controller.ts | 16 ++-- ...profile-web3-wallets-section.controller.ts | 8 +- ...tsx => use-pending-action-shared.test.tsx} | 84 ++++++++++++------- packages/mosaic/src/hooks/use-action-lock.ts | 29 ------- .../mosaic/src/hooks/use-pending-action.ts | 49 ++++++----- 9 files changed, 143 insertions(+), 142 deletions(-) rename packages/mosaic/src/hooks/__tests__/{use-action-lock.test.tsx => use-pending-action-shared.test.tsx} (58%) delete mode 100644 packages/mosaic/src/hooks/use-action-lock.ts diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index f94ebb1b3b5..88430d393f2 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -31,33 +31,33 @@ A flow lives in `src/features//`. The **model** (`*.model.ts(x)`) is th Most problems a feature hits already have a shared answer. Find the row for your problem and use it. Do not write a local version, even a small one. -| Problem | Use | -| ------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------- | -| An async action from a button or row: pending, an error, one lock | `usePendingAction` (`src/hooks/`) | -| Direct actions and confirmation dialogs that must exclude each other | `useActionLock` injected into `usePendingAction` and `useConfirmController` (`src/hooks/`, `src/blocks/confirm/`) | -| A form: values, validation, submit, field errors | `useForm` (`src/components/form/`), with `validateAsync` for a server-side check | -| A `` element, without the browser's validation popup | `Form` (`src/primitives/form/`) | -| A model save whose Clerk errors should land on form fields | `save` (`src/utils/errors.ts`) in the model, or reject with `FormSubmitError` | -| A confirm step before an action | `useConfirmController` (`src/blocks/confirm/`) with `Confirmation` (`src/blocks/confirmation/`) | -| A delete that needs the user to type a phrase | `useConfirmController` (`src/blocks/confirm/`) with `Destructive` (`src/blocks/destructive/`) | -| An action that may need step-up verification | `useReverificationFlow` (`src/features/reverification/`) in the model, passed to `useConfirmController` | -| A direct action that needs step-up verification | `useReverificationFlow` in the model, with `ReverificationDialog` (`src/blocks/reverification/`) in the view | -| Showing any error to the user | One of the owners above. They call `toLocalizableError` and resolve copy with `useErrorText` | -| Moving focus after a list row is removed | `useListRemovalFocus` (`src/hooks/`) | -| A debounced, abortable async check as the user types | `useDebouncedAsync` (`src/hooks/`) | -| A spinner that should not flash or flicker | `useSpinDelay` (`src/hooks/`) | -| A placeholder for content that is still loading | `Skeleton` (`src/components/skeleton/`), see [Skeletons](docs/skeletons.md) | -| A date shown to the user | `formatDate` (`src/utils/format-date.ts`) | -| A value turned into kebab case, e.g. for a `data-*` attribute | `kebabCase` (`src/utils/kebab-case.ts`) | -| Current time for display labels | `useNow` (`src/hooks/`) | -| Whether enterprise SSO permits adding another identification | `allowsIdentificationCreation` (`src/utils/allows-identification-creation.ts`) | -| An inline message row that animates open and closed | `useHeldMessage`, `useMessageHeight`, `FeedbackBody` (`src/utils/feedback.tsx`) + `src/styles/feedback.styles.ts` | -| Data read from a Clerk resource that should be cached across mounts | A query hook in `src/queries/`, called from the model (`src/queries/use-user-sessions-query.ts`) | -| An item changed in place in an infinite list, kept until the next reload | `replaceInPages` (`src/utils/infinite-pages.ts`), passed to the list's `setData` | -| Siblings that slide when an item is added or removed | `useLayoutAnimation` (`src/primitives/hooks/`) | -| An input that reformats as the user types without moving the caret | `useFormattedInput` (`src/primitives/hooks/`) | -| 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` | +| Problem | Use | +| ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------- | +| An async action from a button or row: pending, an error, one lock | `usePendingAction` (`src/hooks/`) | +| Direct actions and confirmation dialogs that must exclude each other | One `usePendingAction` owner, with `runExternal` passed as `useConfirmController`'s `runAction` (`src/hooks/`, `src/blocks/confirm/`) | +| A form: values, validation, submit, field errors | `useForm` (`src/components/form/`), with `validateAsync` for a server-side check | +| A `` element, without the browser's validation popup | `Form` (`src/primitives/form/`) | +| A model save whose Clerk errors should land on form fields | `save` (`src/utils/errors.ts`) in the model, or reject with `FormSubmitError` | +| A confirm step before an action | `useConfirmController` (`src/blocks/confirm/`) with `Confirmation` (`src/blocks/confirmation/`) | +| A delete that needs the user to type a phrase | `useConfirmController` (`src/blocks/confirm/`) with `Destructive` (`src/blocks/destructive/`) | +| An action that may need step-up verification | `useReverificationFlow` (`src/features/reverification/`) in the model, passed to `useConfirmController` | +| A direct action that needs step-up verification | `useReverificationFlow` in the model, with `ReverificationDialog` (`src/blocks/reverification/`) in the view | +| Showing any error to the user | One of the owners above. They call `toLocalizableError` and resolve copy with `useErrorText` | +| Moving focus after a list row is removed | `useListRemovalFocus` (`src/hooks/`) | +| A debounced, abortable async check as the user types | `useDebouncedAsync` (`src/hooks/`) | +| A spinner that should not flash or flicker | `useSpinDelay` (`src/hooks/`) | +| A placeholder for content that is still loading | `Skeleton` (`src/components/skeleton/`), see [Skeletons](docs/skeletons.md) | +| A date shown to the user | `formatDate` (`src/utils/format-date.ts`) | +| A value turned into kebab case, e.g. for a `data-*` attribute | `kebabCase` (`src/utils/kebab-case.ts`) | +| Current time for display labels | `useNow` (`src/hooks/`) | +| Whether enterprise SSO permits adding another identification | `allowsIdentificationCreation` (`src/utils/allows-identification-creation.ts`) | +| An inline message row that animates open and closed | `useHeldMessage`, `useMessageHeight`, `FeedbackBody` (`src/utils/feedback.tsx`) + `src/styles/feedback.styles.ts` | +| Data read from a Clerk resource that should be cached across mounts | A query hook in `src/queries/`, called from the model (`src/queries/use-user-sessions-query.ts`) | +| An item changed in place in an infinite list, kept until the next reload | `replaceInPages` (`src/utils/infinite-pages.ts`), passed to the list's `setData` | +| Siblings that slide when an item is added or removed | `useLayoutAnimation` (`src/primitives/hooks/`) | +| An input that reformats as the user types without moving the caret | `useFormattedInput` (`src/primitives/hooks/`) | +| 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` | ## Signs you are about to invent a pattern diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index 8207d6e496d..3df07e24d44 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -51,22 +51,24 @@ return { }; ``` -When a direct action and a confirmation must not run together, create one -`useActionLock` (`src/hooks/use-action-lock.ts`) and inject it into both owners: +When a direct action and a confirmation must not run together, give both +operations one `usePendingAction` owner: ```ts -const actionLock = useActionLock(); -const primary = usePendingAction({ actionLock }); +const actions = usePendingAction(); const remove = useConfirmController({ - actionLock, + runAction: actions.runExternal, onConfirm: contact => removalFocus.remove(contact.id), }); ``` -Pass `actionLock.isPending` to the section controls' `disabled` props, including -`Confirmation.disabled`. The lock prevents overlapping requests even before React -renders the disabled controls, and stays held until the action settles. Each owner -keeps its existing error handling. `actionLock` is a hook option, not a per-run option. +Run direct actions with `actions.run(key, action)`. Pass `actions.isPending` to +the section controls' `disabled` props, including `Confirmation.disabled`. +`runExternal` returns the accepted promise, or `undefined` while another action +runs. It leaves row keys and inline errors unchanged and preserves rejections +for the confirmation dialog to handle. Both paths block overlapping calls before +React renders and stay pending until the accepted promise settles. Closing a +verification dialog does not release pending while cancellation is still settling. `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 diff --git a/packages/mosaic/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts index faf04107a81..4559f4d436c 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -4,7 +4,6 @@ import { useMemo } from 'react'; import type { DialogHandle } from '../../components/dialog'; import { Dialog } from '../../components/dialog'; import type { ReverificationController } from '../../features/reverification'; -import type { ActionLock } from '../../hooks/use-action-lock'; import type { ErrorDescription } from '../../localization'; import { useErrorText } from '../../localization'; import { setup } from '../../machine/setup'; @@ -86,7 +85,7 @@ export interface ConfirmController { } export interface ConfirmControllerOptions { - actionLock?: ActionLock; + runAction?: (action: () => Promise) => Promise | undefined; /** The action the dialog confirms. Resolve to close; reject to keep the dialog open showing why. */ onConfirm: (payload: Payload) => Promise | void; /** The flow `onConfirm` runs under, when the action may need step-up verification. */ @@ -109,7 +108,7 @@ export function useConfirmController({ onConfirm, reverification, errorFallback, - actionLock, + runAction, }: ConfirmControllerOptions): ConfirmController { const handle = useMemo(() => Dialog.createHandle(), []); const [snapshot, send] = useMachine(confirmMachine); @@ -145,8 +144,8 @@ export function useConfirmController({ const run = async () => { await onConfirm(payload); }; - if (actionLock) { - const accepted = actionLock.tryRun(run); + if (runAction) { + const accepted = runAction(run); if (accepted) { send({ type: 'CONFIRM', run: () => accepted }); } 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 6f0bd477d76..c6ba700f473 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,7 +1,6 @@ import { useRef } from 'react'; import { useConfirmController } from '../../../blocks/confirm'; -import { useActionLock } from '../../../hooks/use-action-lock'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; import type { UserProfileEmail } from '../user-profile-contact.types'; @@ -24,8 +23,7 @@ export function useUserProfileEmailSectionController({ onCreate: onCreateEmail, reverification: createReverification, }); - const actionLock = useActionLock(); - const setPrimary = usePendingAction({ actionLock }); + const actions = usePendingAction(); const rowRef = useRef(null); const removalFocus = useListRemovalFocus({ ids: emails.map(email => email.id), @@ -33,7 +31,7 @@ export function useUserProfileEmailSectionController({ fallback: () => rowRef.current?.querySelector('button:not([disabled])') ?? rowRef.current, }); const remove = useConfirmController({ - actionLock, + runAction: actions.runExternal, onConfirm: email => removalFocus.remove(email.id), reverification: removeReverification, }); @@ -42,22 +40,22 @@ export function useUserProfileEmailSectionController({ emails, canAdd: Boolean(onCreateEmail), verification, - error: setPrimary.error, - isDisabled: actionLock.isPending, - pendingPrimaryId: setPrimary.pendingKey, + error: actions.error, + isDisabled: actions.isPending, + pendingPrimaryId: actions.pendingKey, rowRef, registerTrigger: removalFocus.registerTrigger, remove: onRemoveEmail ? 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)); } }, - 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-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 08cf24cbc8b..d2faa559b9c 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 @@ -2,7 +2,6 @@ import { stringToFormattedPhoneString } from '@clerk/shared/phone'; import { useRef } from 'react'; import { useConfirmController } from '../../../blocks/confirm'; -import { useActionLock } from '../../../hooks/use-action-lock'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; import type { UserProfilePhone } from '../user-profile-contact.types'; @@ -24,8 +23,7 @@ export function useUserProfilePhoneSectionController({ onCreate: onCreatePhone, reverification: createReverification, }); - const actionLock = useActionLock(); - const setPrimary = usePendingAction({ actionLock }); + const actions = usePendingAction(); const rowRef = useRef(null); const removalFocus = useListRemovalFocus({ ids: phones.map(phone => phone.id), @@ -33,7 +31,7 @@ export function useUserProfilePhoneSectionController({ fallback: () => rowRef.current?.querySelector('button:not([disabled])') ?? rowRef.current, }); const remove = useConfirmController({ - actionLock, + runAction: actions.runExternal, onConfirm: phone => removalFocus.remove(phone.id), reverification: removeReverification, }); @@ -43,22 +41,22 @@ export function useUserProfilePhoneSectionController({ defaultPhoneCountry, canAdd: Boolean(onCreatePhone), verification, - error: setPrimary.error, - isDisabled: actionLock.isPending, - pendingPrimaryId: setPrimary.pendingKey, + error: actions.error, + isDisabled: actions.isPending, + pendingPrimaryId: actions.pendingKey, rowRef, registerTrigger: removalFocus.registerTrigger, remove: onRemovePhone ? 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)); } }, - 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-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 6daca8a1e94..26aa625bf14 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 @@ -2,7 +2,6 @@ import { useRef, useState } from 'react'; import { useConfirmController } from '../../../blocks/confirm'; import { usePanelTitle } from '../../../components/panel'; -import { useActionLock } from '../../../hooks/use-action-lock'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; import { useMessages } from '../../../localization'; @@ -46,8 +45,7 @@ export function useUserProfileWeb3WalletsController({ | 'createReverification' >) { const messages = useMessages('userProfileWeb3Wallets'); - const actionLock = useActionLock(); - const action = usePendingAction({ actionLock, errorFallback: messages.errors.generic }); + const action = usePendingAction({ errorFallback: messages.errors.generic }); const [picker, setPicker] = useState({ open: false }); const [owner, setOwner] = useState(); const sectionRef = useRef(null); @@ -61,7 +59,7 @@ export function useUserProfileWeb3WalletsController({ panelTitle(), }); const removeWallet = useConfirmController({ - actionLock, + runAction: action.runExternal, onConfirm: wallet => removalFocus.remove(wallet.id), reverification: removeReverification, }); @@ -173,7 +171,7 @@ export function useUserProfileWeb3WalletsController({ : undefined, pendingId, pendingPrimaryId: owner?.kind === 'primary' ? pendingId : undefined, - isDisabled: actionLock.isPending, + isDisabled: action.isPending, sectionRef, registerTrigger: removalFocus.registerTrigger, remove: remove ? removeWallet : undefined, diff --git a/packages/mosaic/src/hooks/__tests__/use-action-lock.test.tsx b/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx similarity index 58% rename from packages/mosaic/src/hooks/__tests__/use-action-lock.test.tsx rename to packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx index 4d589c1d23a..53f5f1433cd 100644 --- a/packages/mosaic/src/hooks/__tests__/use-action-lock.test.tsx +++ b/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx @@ -4,18 +4,16 @@ import { describe, expect, it, vi } from 'vitest'; import { deferred } from '../../__tests__/async'; import { useConfirmController } from '../../blocks/confirm'; import { SaveError } from '../../utils/errors'; -import { useActionLock } from '../use-action-lock'; import { usePendingAction } from '../use-pending-action'; function useOwners(onConfirm: () => Promise) { - const actionLock = useActionLock(); - const primary = usePendingAction({ actionLock }); + const actions = usePendingAction(); const remove = useConfirmController({ - actionLock, + runAction: actions.runExternal, onConfirm, reverification: { status: 'loading', visible: true, onCancel: vi.fn() }, }); - return { actionLock, primary, remove }; + return { actions, remove }; } describe('shared action admission', () => { @@ -27,16 +25,16 @@ describe('shared action admission', () => { const owners = result.current; act(() => { - void owners.primary.run('primary', () => pending.promise); + void owners.actions.run('primary', () => pending.promise); owners.remove.confirm(); }); expect(onConfirm).not.toHaveBeenCalled(); expect(result.current.remove.isOpen).toBe(true); expect(result.current.remove.isPending).toBe(false); - expect(result.current.actionLock.isPending).toBe(true); + expect(result.current.actions.isPending).toBe(true); await act(() => pending.resolve()); - expect(result.current.actionLock.isPending).toBe(false); + expect(result.current.actions.isPending).toBe(false); await act(() => result.current.remove.confirm()); expect(onConfirm).toHaveBeenCalledOnce(); expect(result.current.remove.isOpen).toBe(false); @@ -51,19 +49,19 @@ describe('shared action admission', () => { await act(async () => { owners.remove.confirm(); - expect(await owners.primary.run('primary', primary)).toBe(false); + expect(await owners.actions.run('primary', primary)).toBe(false); }); expect(primary).not.toHaveBeenCalled(); - expect(result.current.primary.lastKey).toBeUndefined(); - expect(result.current.primary.error).toBeUndefined(); - expect(result.current.actionLock.isPending).toBe(true); + 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.primary.run('primary', primary)).toBe(true)); + await act(async () => expect(await owners.actions.run('primary', primary)).toBe(true)); expect(primary).toHaveBeenCalledOnce(); }); - it('keeps a rejected removal error when another owner makes confirmation busy', async () => { + it('keeps a rejected removal error while primary makes confirmation busy', async () => { const pending = deferred(); const failure = new SaveError({ global: { code: 'action_blocked', message: 'Removal refused.' } }); const onConfirm = vi.fn(() => Promise.reject(failure)); @@ -73,10 +71,10 @@ describe('shared action admission', () => { const error = result.current.remove.errorMessage; expect(error).toBeDefined(); expect(result.current.remove.isOpen).toBe(true); - expect(result.current.primary.error).toBeUndefined(); + expect(result.current.actions.error).toBeUndefined(); act(() => { - void result.current.primary.run('primary', () => pending.promise); + void result.current.actions.run('primary', () => pending.promise); result.current.remove.confirm(); }); @@ -87,7 +85,28 @@ describe('shared action admission', () => { await act(() => pending.resolve()); }); - it('holds admission after cancelling observation until the underlying removal settles', async () => { + it('preserves the primary error and focus key while removal runs', async () => { + const pending = deferred(); + const failure = new SaveError({ global: { code: 'action_blocked', message: 'Primary refused.' } }); + const { result } = renderHook(() => useOwners(() => pending.promise)); + await act(() => result.current.actions.run('email', () => Promise.reject(failure))); + const error = result.current.actions.error; + expect(error).toBeDefined(); + + act(() => result.current.remove.onOpenChange(true)); + act(() => result.current.remove.confirm()); + + expect(result.current.actions.isPending).toBe(true); + expect(result.current.actions.pendingKey).toBeUndefined(); + expect(result.current.actions.lastKey).toBe('email'); + expect(result.current.actions.errorKey).toBe('email'); + expect(result.current.actions.error).toBe(error); + await act(() => pending.resolve()); + expect(result.current.actions.isPending).toBe(false); + expect(result.current.actions.error).toBe(error); + }); + + it.each(['resolve', 'reject'])('holds pending after cancellation until removal promises %s', async outcome => { const pending = deferred(); const primary = vi.fn(); const { result } = renderHook(() => useOwners(() => pending.promise)); @@ -95,24 +114,31 @@ describe('shared action admission', () => { act(() => result.current.remove.confirm()); act(() => result.current.remove.onOpenChange(false)); expect(result.current.remove.isOpen).toBe(false); - await act(async () => expect(await result.current.primary.run('primary', primary)).toBe(false)); + await act(async () => expect(await result.current.actions.run('primary', primary)).toBe(false)); expect(primary).not.toHaveBeenCalled(); - expect(result.current.actionLock.isPending).toBe(true); + expect(result.current.actions.isPending).toBe(true); + expect(result.current.actions.pendingKey).toBeUndefined(); - await act(() => pending.reject(new Error('Late removal failure'))); - expect(result.current.actionLock.isPending).toBe(false); + await act(() => { + if (outcome === 'reject') { + pending.reject(new Error('Late removal failure')); + } else { + pending.resolve(); + } + }); + expect(result.current.actions.isPending).toBe(false); expect(result.current.remove.errorMessage).toBeUndefined(); - await act(async () => expect(await result.current.primary.run('primary', primary)).toBe(true)); + await act(async () => expect(await result.current.actions.run('primary', primary)).toBe(true)); }); }); -describe('useActionLock', () => { +describe('external pending actions', () => { it('returns the accepted result and refuses reentrant work synchronously', async () => { - const { result } = renderHook(() => useActionLock()); + const { result } = renderHook(() => usePendingAction()); const other = vi.fn(); await act(async () => { - const accepted = result.current.tryRun(() => { - expect(result.current.tryRun(other)).toBeUndefined(); + const accepted = result.current.runExternal(() => { + expect(result.current.runExternal(other)).toBeUndefined(); return 42; }); expect(await accepted).toBe(42); @@ -122,10 +148,10 @@ describe('useActionLock', () => { }); it.each(['throw', 'reject'])('preserves the original %s and releases admission', async mode => { - const { result } = renderHook(() => useActionLock()); + const { result } = renderHook(() => usePendingAction()); const failure = new Error('Original failure'); await act(async () => { - const accepted = result.current.tryRun(() => { + const accepted = result.current.runExternal(() => { if (mode === 'throw') { throw failure; } @@ -134,6 +160,6 @@ describe('useActionLock', () => { await expect(accepted).rejects.toBe(failure); }); expect(result.current.isPending).toBe(false); - await act(async () => expect(await result.current.tryRun(() => 'retry')).toBe('retry')); + await act(async () => expect(await result.current.runExternal(() => 'retry')).toBe('retry')); }); }); diff --git a/packages/mosaic/src/hooks/use-action-lock.ts b/packages/mosaic/src/hooks/use-action-lock.ts deleted file mode 100644 index e112d93ad84..00000000000 --- a/packages/mosaic/src/hooks/use-action-lock.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { useRef, useState } from 'react'; - -export interface ActionLock { - isPending: boolean; - tryRun: (action: () => T | Promise) => Promise | undefined; -} - -export function useActionLock(): ActionLock { - const running = useRef(false); - const [isPending, setIsPending] = useState(false); - - const tryRun = (action: () => T | Promise): Promise | undefined => { - if (running.current) { - return undefined; - } - running.current = true; - setIsPending(true); - return (async () => { - try { - return await action(); - } finally { - running.current = false; - setIsPending(false); - } - })(); - }; - - return { isPending, tryRun }; -} diff --git a/packages/mosaic/src/hooks/use-pending-action.ts b/packages/mosaic/src/hooks/use-pending-action.ts index b898cdf274c..e659bbec8e8 100644 --- a/packages/mosaic/src/hooks/use-pending-action.ts +++ b/packages/mosaic/src/hooks/use-pending-action.ts @@ -1,23 +1,18 @@ import { isReverificationCancelledError } from '@clerk/shared/error'; -import { useState } from 'react'; +import { useRef, useState } from 'react'; import type { ErrorDescription } from '../localization'; import { useErrorText } from '../localization'; import { toLocalizableError } from '../utils/errors'; -import type { ActionLock } from './use-action-lock'; -import { useActionLock } from './use-action-lock'; export interface PendingActionOptions { /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ errorFallback?: string; } -export interface PendingActionHookOptions extends PendingActionOptions { - actionLock?: ActionLock; -} - export interface PendingAction { run: (key: TKey, action: () => Promise | void, options?: PendingActionOptions) => Promise; + runExternal: (action: () => T | Promise) => Promise | undefined; pendingKey: TKey | undefined; lastKey: TKey | undefined; isPending: boolean; @@ -32,25 +27,40 @@ 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 and external actions 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 it starts. */ export function usePendingAction({ errorFallback, - actionLock, -}: PendingActionHookOptions = {}): PendingAction { +}: 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 localLock = useActionLock(); - const lock = actionLock ?? localLock; + + const claim = (next: Pending, action: () => T | Promise): Promise | undefined => { + if (running.current) { + return undefined; + } + running.current = true; + setPending(next); + return (async () => { + try { + return await action(); + } finally { + running.current = false; + setPending(undefined); + } + })(); + }; const run = async (key: TKey, action: () => Promise | void, options: PendingActionOptions = {}) => { return ( - lock.tryRun(async () => { - setPendingKey(key); + claim({ kind: 'inline', key }, async () => { setLastKey(key); setFailure(undefined); try { @@ -61,8 +71,6 @@ export function usePendingAction({ setFailure({ key, error: toLocalizableError(cause), errorFallback: options.errorFallback }); } return false; - } finally { - setPendingKey(undefined); } }) ?? false ); @@ -70,9 +78,10 @@ export function usePendingAction({ return { run, - pendingKey, + runExternal: action => claim({ kind: 'external' }, action), + 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), From badabc5e8bfd1892ff3637d1579473edb7e963d3 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 11:54:20 -0600 Subject: [PATCH 07/12] refactor(mosaic): coordinate identifier actions in section controllers --- packages/mosaic/AGENTS.md | 54 +++++++++---------- packages/mosaic/docs/controllers.md | 31 +++++++---- .../__tests__/confirm.controller.test.ts | 53 ++++++++++++++++-- .../src/blocks/confirm/confirm-shell.tsx | 2 +- .../src/blocks/confirm/confirm.controller.ts | 35 ++++++------ ...ofile-active-devices-section.controller.ts | 2 +- .../user-profile-email-section.controller.ts | 8 ++- .../user-profile-phone-section.controller.ts | 8 ++- ...profile-web3-wallets-section.controller.ts | 9 +++- .../use-pending-action-shared.test.tsx | 23 +++++--- 10 files changed, 152 insertions(+), 73 deletions(-) diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index 88430d393f2..24b0251e1a2 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -31,33 +31,33 @@ A flow lives in `src/features//`. The **model** (`*.model.ts(x)`) is th Most problems a feature hits already have a shared answer. Find the row for your problem and use it. Do not write a local version, even a small one. -| Problem | Use | -| ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------- | -| An async action from a button or row: pending, an error, one lock | `usePendingAction` (`src/hooks/`) | -| Direct actions and confirmation dialogs that must exclude each other | One `usePendingAction` owner, with `runExternal` passed as `useConfirmController`'s `runAction` (`src/hooks/`, `src/blocks/confirm/`) | -| A form: values, validation, submit, field errors | `useForm` (`src/components/form/`), with `validateAsync` for a server-side check | -| A `` element, without the browser's validation popup | `Form` (`src/primitives/form/`) | -| A model save whose Clerk errors should land on form fields | `save` (`src/utils/errors.ts`) in the model, or reject with `FormSubmitError` | -| A confirm step before an action | `useConfirmController` (`src/blocks/confirm/`) with `Confirmation` (`src/blocks/confirmation/`) | -| A delete that needs the user to type a phrase | `useConfirmController` (`src/blocks/confirm/`) with `Destructive` (`src/blocks/destructive/`) | -| An action that may need step-up verification | `useReverificationFlow` (`src/features/reverification/`) in the model, passed to `useConfirmController` | -| A direct action that needs step-up verification | `useReverificationFlow` in the model, with `ReverificationDialog` (`src/blocks/reverification/`) in the view | -| Showing any error to the user | One of the owners above. They call `toLocalizableError` and resolve copy with `useErrorText` | -| Moving focus after a list row is removed | `useListRemovalFocus` (`src/hooks/`) | -| A debounced, abortable async check as the user types | `useDebouncedAsync` (`src/hooks/`) | -| A spinner that should not flash or flicker | `useSpinDelay` (`src/hooks/`) | -| A placeholder for content that is still loading | `Skeleton` (`src/components/skeleton/`), see [Skeletons](docs/skeletons.md) | -| A date shown to the user | `formatDate` (`src/utils/format-date.ts`) | -| A value turned into kebab case, e.g. for a `data-*` attribute | `kebabCase` (`src/utils/kebab-case.ts`) | -| Current time for display labels | `useNow` (`src/hooks/`) | -| Whether enterprise SSO permits adding another identification | `allowsIdentificationCreation` (`src/utils/allows-identification-creation.ts`) | -| An inline message row that animates open and closed | `useHeldMessage`, `useMessageHeight`, `FeedbackBody` (`src/utils/feedback.tsx`) + `src/styles/feedback.styles.ts` | -| Data read from a Clerk resource that should be cached across mounts | A query hook in `src/queries/`, called from the model (`src/queries/use-user-sessions-query.ts`) | -| An item changed in place in an infinite list, kept until the next reload | `replaceInPages` (`src/utils/infinite-pages.ts`), passed to the list's `setData` | -| Siblings that slide when an item is added or removed | `useLayoutAnimation` (`src/primitives/hooks/`) | -| An input that reformats as the user types without moving the caret | `useFormattedInput` (`src/primitives/hooks/`) | -| 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` | +| Problem | Use | +| ------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------- | +| An async action from a button or row: pending, an error, one lock | `usePendingAction` (`src/hooks/`) | +| Direct actions and confirmation dialogs that must exclude each other | Section controller wraps `confirm` with `usePendingAction.runExternal` (`src/hooks/`, `src/blocks/confirm/`) | +| A form: values, validation, submit, field errors | `useForm` (`src/components/form/`), with `validateAsync` for a server-side check | +| A `` element, without the browser's validation popup | `Form` (`src/primitives/form/`) | +| A model save whose Clerk errors should land on form fields | `save` (`src/utils/errors.ts`) in the model, or reject with `FormSubmitError` | +| A confirm step before an action | `useConfirmController` (`src/blocks/confirm/`) with `Confirmation` (`src/blocks/confirmation/`) | +| A delete that needs the user to type a phrase | `useConfirmController` (`src/blocks/confirm/`) with `Destructive` (`src/blocks/destructive/`) | +| An action that may need step-up verification | `useReverificationFlow` (`src/features/reverification/`) in the model, passed to `useConfirmController` | +| A direct action that needs step-up verification | `useReverificationFlow` in the model, with `ReverificationDialog` (`src/blocks/reverification/`) in the view | +| Showing any error to the user | One of the owners above. They call `toLocalizableError` and resolve copy with `useErrorText` | +| Moving focus after a list row is removed | `useListRemovalFocus` (`src/hooks/`) | +| A debounced, abortable async check as the user types | `useDebouncedAsync` (`src/hooks/`) | +| A spinner that should not flash or flicker | `useSpinDelay` (`src/hooks/`) | +| A placeholder for content that is still loading | `Skeleton` (`src/components/skeleton/`), see [Skeletons](docs/skeletons.md) | +| A date shown to the user | `formatDate` (`src/utils/format-date.ts`) | +| A value turned into kebab case, e.g. for a `data-*` attribute | `kebabCase` (`src/utils/kebab-case.ts`) | +| Current time for display labels | `useNow` (`src/hooks/`) | +| Whether enterprise SSO permits adding another identification | `allowsIdentificationCreation` (`src/utils/allows-identification-creation.ts`) | +| An inline message row that animates open and closed | `useHeldMessage`, `useMessageHeight`, `FeedbackBody` (`src/utils/feedback.tsx`) + `src/styles/feedback.styles.ts` | +| Data read from a Clerk resource that should be cached across mounts | A query hook in `src/queries/`, called from the model (`src/queries/use-user-sessions-query.ts`) | +| An item changed in place in an infinite list, kept until the next reload | `replaceInPages` (`src/utils/infinite-pages.ts`), passed to the list's `setData` | +| Siblings that slide when an item is added or removed | `useLayoutAnimation` (`src/primitives/hooks/`) | +| An input that reformats as the user types without moving the caret | `useFormattedInput` (`src/primitives/hooks/`) | +| 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` | ## Signs you are about to invent a pattern diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index 3df07e24d44..32a5fb699c9 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -56,19 +56,32 @@ operations one `usePendingAction` owner: ```ts const actions = usePendingAction(); -const remove = useConfirmController({ - runAction: actions.runExternal, +const remove = useConfirmController({ onConfirm: contact => removalFocus.remove(contact.id), }); + +return { + isDisabled: actions.isPending, + pendingPrimaryId: actions.pendingKey, + remove: { + ...remove, + confirm: async contact => (await actions.runExternal(() => remove.confirm(contact))) ?? false, + }, +}; ``` -Run direct actions with `actions.run(key, action)`. Pass `actions.isPending` to -the section controls' `disabled` props, including `Confirmation.disabled`. -`runExternal` returns the accepted promise, or `undefined` while another action -runs. It leaves row keys and inline errors unchanged and preserves rejections -for the confirmation dialog to handle. Both paths block overlapping calls before -React renders and stay pending until the accepted promise settles. Closing a -verification dialog does not release pending while cancellation is still settling. +Keep this coordination in the section controller. Run direct actions with +`actions.run(key, action)` and wrap the returned confirmation's `confirm` callback +with `actions.runExternal`. Pass `actions.isPending` to the section controls' +`disabled` props, including `Confirmation.disabled`. + +`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. Views receive the pending and disabled props without coordinating +the operations. `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 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 4559f4d436c..bb11b314719 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -80,12 +80,11 @@ 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 { - runAction?: (action: () => Promise) => Promise | undefined; /** The action the dialog confirms. Resolve to close; reject to keep the dialog open showing why. */ onConfirm: (payload: Payload) => Promise | void; /** The flow `onConfirm` runs under, when the action may need step-up verification. */ @@ -108,7 +107,6 @@ export function useConfirmController({ onConfirm, reverification, errorFallback, - runAction, }: ConfirmControllerOptions): ConfirmController { const handle = useMemo(() => Dialog.createHandle(), []); const [snapshot, send] = useMachine(confirmMachine); @@ -138,20 +136,21 @@ export function useConfirmController({ send({ type: 'CANCEL' }); }, confirm: payload => { - if (snapshot.value !== 'confirming') { - return; - } - const run = async () => { - await onConfirm(payload); - }; - if (runAction) { - const accepted = runAction(run); - if (accepted) { - send({ type: 'CONFIRM', run: () => accepted }); - } - return; - } - send({ type: 'CONFIRM', run }); + let completion: Promise | undefined; + send({ + type: 'CONFIRM', + 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/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 3bfcff4a8ec..d0f5a674ee6 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-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 c6ba700f473..c6d8cc77525 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 @@ -31,7 +31,6 @@ export function useUserProfileEmailSectionController({ fallback: () => rowRef.current?.querySelector('button:not([disabled])') ?? rowRef.current, }); const remove = useConfirmController({ - runAction: actions.runExternal, onConfirm: email => removalFocus.remove(email.id), reverification: removeReverification, }); @@ -45,7 +44,12 @@ export function useUserProfileEmailSectionController({ pendingPrimaryId: actions.pendingKey, rowRef, registerTrigger: removalFocus.registerTrigger, - remove: onRemoveEmail ? remove : undefined, + remove: onRemoveEmail + ? { + ...remove, + confirm: async email => (await actions.runExternal(() => remove.confirm(email))) ?? false, + } + : undefined, finalFocus: removalFocus.finalFocus, verificationFinalFocus: () => (verification.verifyingId ? removalFocus.trigger(verification.verifyingId) : null), setPrimaryReverification, 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 d2faa559b9c..b90085b29df 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 @@ -31,7 +31,6 @@ export function useUserProfilePhoneSectionController({ fallback: () => rowRef.current?.querySelector('button:not([disabled])') ?? rowRef.current, }); const remove = useConfirmController({ - runAction: actions.runExternal, onConfirm: phone => removalFocus.remove(phone.id), reverification: removeReverification, }); @@ -46,7 +45,12 @@ export function useUserProfilePhoneSectionController({ pendingPrimaryId: actions.pendingKey, rowRef, registerTrigger: removalFocus.registerTrigger, - remove: onRemovePhone ? remove : undefined, + remove: onRemovePhone + ? { + ...remove, + confirm: async phone => (await actions.runExternal(() => remove.confirm(phone))) ?? false, + } + : undefined, finalFocus: removalFocus.finalFocus, verificationFinalFocus: () => (verification.verifyingId ? removalFocus.trigger(verification.verifyingId) : null), setPrimaryReverification, 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 26aa625bf14..965bab7a43d 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 @@ -59,7 +59,6 @@ export function useUserProfileWeb3WalletsController({ panelTitle(), }); const removeWallet = useConfirmController({ - runAction: action.runExternal, onConfirm: wallet => removalFocus.remove(wallet.id), reverification: removeReverification, }); @@ -174,7 +173,13 @@ export function useUserProfileWeb3WalletsController({ isDisabled: action.isPending, sectionRef, registerTrigger: removalFocus.registerTrigger, - remove: remove ? removeWallet : undefined, + remove: remove + ? { + ...removeWallet, + confirm: async (wallet: UserProfileWeb3Wallet) => + (await action.runExternal(() => removeWallet.confirm(wallet))) ?? false, + } + : undefined, finalFocus: removalFocus.finalFocus, setPrimaryReverification, createReverification, diff --git a/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx b/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx index 53f5f1433cd..4d9fa334ebf 100644 --- a/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx +++ b/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx @@ -9,11 +9,16 @@ import { usePendingAction } from '../use-pending-action'; function useOwners(onConfirm: () => Promise) { const actions = usePendingAction(); const remove = useConfirmController({ - runAction: actions.runExternal, onConfirm, reverification: { status: 'loading', visible: true, onCancel: vi.fn() }, }); - return { actions, remove }; + return { + actions, + remove: { + ...remove, + confirm: async () => (await actions.runExternal(() => remove.confirm())) ?? false, + }, + }; } describe('shared action admission', () => { @@ -26,7 +31,7 @@ describe('shared action admission', () => { act(() => { void owners.actions.run('primary', () => pending.promise); - owners.remove.confirm(); + void owners.remove.confirm(); }); expect(onConfirm).not.toHaveBeenCalled(); @@ -48,7 +53,7 @@ describe('shared action admission', () => { const owners = result.current; await act(async () => { - owners.remove.confirm(); + void owners.remove.confirm(); expect(await owners.actions.run('primary', primary)).toBe(false); }); @@ -75,7 +80,7 @@ describe('shared action admission', () => { act(() => { void result.current.actions.run('primary', () => pending.promise); - result.current.remove.confirm(); + void result.current.remove.confirm(); }); expect(onConfirm).toHaveBeenCalledOnce(); @@ -94,7 +99,9 @@ describe('shared action admission', () => { expect(error).toBeDefined(); act(() => result.current.remove.onOpenChange(true)); - act(() => result.current.remove.confirm()); + act(() => { + void result.current.remove.confirm(); + }); expect(result.current.actions.isPending).toBe(true); expect(result.current.actions.pendingKey).toBeUndefined(); @@ -111,7 +118,9 @@ describe('shared action admission', () => { const primary = vi.fn(); const { result } = renderHook(() => useOwners(() => pending.promise)); act(() => result.current.remove.onOpenChange(true)); - act(() => result.current.remove.confirm()); + 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('primary', primary)).toBe(false)); From 26736b264788a68e2ef511904bc90f5296fd6152 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 13:22:39 -0600 Subject: [PATCH 08/12] fix(mosaic): list the primary identifier first and clear stale action errors --- .../user-profile-contact.utils.test.ts | 12 ++++-------- .../user-profile/user-profile-contact.utils.ts | 13 +++++++------ .../user-profile-email-section.feature.test.tsx | 10 +++++----- .../user-profile-phone-section.feature.test.tsx | 12 +++++------- ...rofile-web3-wallets-section.feature.test.tsx | 8 ++++---- ...r-profile-web3-wallets-section.model.test.ts | 17 +++-------------- .../user-profile-web3-wallets-section.model.ts | 10 ++++++---- .../use-pending-action-shared.test.tsx | 11 +++++------ packages/mosaic/src/hooks/use-pending-action.ts | 4 ++-- 9 files changed, 41 insertions(+), 56 deletions(-) diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-contact.utils.test.ts b/packages/mosaic/src/features/user-profile/__tests__/user-profile-contact.utils.test.ts index b70ad875de4..3c2604a96dc 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-contact.utils.test.ts +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-contact.utils.test.ts @@ -9,30 +9,26 @@ function contact(id: string, status: Contact['verification']['status'], expireAt } describe('toContacts', () => { - it('orders verified, then pending by expiry, then never started regardless of primary', () => { + it('orders primary first, then verified, then pending by expiry, then never started', () => { const items = [ contact('idn_unstarted', null), contact('idn_pending_late', 'unverified', new Date(2026, 0, 2)), contact('idn_verified_b', 'verified'), - contact('idn_verified_z', 'verified'), + contact('idn_primary', 'verified'), contact('idn_pending_soon', 'unverified', new Date(2026, 0, 1)), contact('idn_verified_a', 'verified'), ]; - const contacts = toContacts(items, 'idn_verified_z', item => item.id); - const changedPrimary = toContacts(items, 'idn_verified_b', item => item.id); + const contacts = toContacts(items, 'idn_primary', item => item.id); expect(contacts.map(item => item.id)).toEqual([ + 'idn_primary', 'idn_verified_a', 'idn_verified_b', - 'idn_verified_z', 'idn_pending_soon', 'idn_pending_late', 'idn_unstarted', ]); - expect(changedPrimary.map(item => item.id)).toEqual(contacts.map(item => item.id)); - expect(changedPrimary.find(item => item.id === 'idn_verified_b')?.isDefault).toBe(true); - expect(changedPrimary.find(item => item.id === 'idn_verified_z')?.isDefault).toBe(false); }); it('marks which contact is the primary one and which are verified', () => { diff --git a/packages/mosaic/src/features/user-profile/user-profile-contact.utils.ts b/packages/mosaic/src/features/user-profile/user-profile-contact.utils.ts index 39c6a9613c0..56b2f78d083 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-contact.utils.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-contact.utils.ts @@ -30,10 +30,11 @@ export function byId(items: T[], id: string, kind: str type Identification = { id: string; verification: Pick }; -export function sortByVerification(items: T[]): T[] { - const verified = items.filter(item => item.verification.status === 'verified'); - const pending = items.filter(item => item.verification.status && item.verification.status !== 'verified'); - const unstarted = items.filter(item => !item.verification.status); +export function sortByVerification(items: T[], primaryId: string | null): T[] { + const others = items.filter(item => item.id !== primaryId); + const verified = others.filter(item => item.verification.status === 'verified'); + const pending = others.filter(item => item.verification.status && item.verification.status !== 'verified'); + const unstarted = others.filter(item => !item.verification.status); verified.sort((a, b) => a.id.localeCompare(b.id)); pending.sort((a, b) => { @@ -42,7 +43,7 @@ export function sortByVerification(items: T[]): T[] { return aExpireAt && bExpireAt ? aExpireAt.getTime() - bExpireAt.getTime() : 0; }); - return [...verified, ...pending, ...unstarted]; + return [...items.filter(item => item.id === primaryId), ...verified, ...pending, ...unstarted]; } export function toContacts( @@ -50,7 +51,7 @@ export function toContacts( primaryId: string | null, value: (item: T) => string, ): UserProfileContact[] { - return sortByVerification(items).map(item => ({ + return sortByVerification(items, primaryId).map(item => ({ id: item.id, value: value(item), isDefault: item.id === primaryId, 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 0e41f723cf4..bb5ee417039 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 @@ -68,10 +68,9 @@ 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', async () => { + it('holds the menu while setting an address as primary, then moves the row first with focus', async () => { const other = fapiEmailAddress({ id: 'idn_other', email_address: 'bob@example.com', verification: VERIFIED }); const { actor } = await renderSection(signedInWithEmails([PRIMARY, other])); - const initialOrder = emailsListed(); const primary = holdRequests('post', '/v1/me'); try { await manageEmail(actor, 'bob@example.com', 'Set as primary'); @@ -85,7 +84,7 @@ describe('the user profile email section', () => { 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(initialOrder); + expect(emailsListed()).toEqual(['alice@example.com', 'bob@example.com']); } finally { primary.release(); await waitFor(() => @@ -94,7 +93,8 @@ describe('the user profile email section', () => { } expect(screen.getByRole('button', { name: 'Manage bob@example.com' })).not.toHaveAttribute('aria-disabled', 'true'); await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); - expect(emailsListed()).toEqual(initialOrder); + expect(emailsListed()).toEqual(['bob@example.com', 'alice@example.com']); + expect(screen.getByRole('button', { name: 'Manage bob@example.com' })).toHaveFocus(); expect(screen.queryByRole('progressbar', { name: 'Set as primary' })).toBeNull(); }); @@ -484,7 +484,7 @@ describe('email reverification', () => { await screen.findByRole('dialog', { name: 'Verify your email' }); await enterCode(actor, VERIFICATION_CODE); await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); - expect(emailsListed()).toEqual(['new@example.com', 'alice@example.com']); + expect(emailsListed()).toEqual(['alice@example.com', 'new@example.com']); }); it('closes quietly when confirmation is dismissed and starts clean on reopen', async () => { 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 a0e56983b35..500299d4804 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 @@ -92,9 +92,8 @@ 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 () => { + it('holds the menu while setting a number as primary, then moves the row first', async () => { const { actor } = await renderSection(signedIn([HOME, WORK])); - const initialOrder = listed(); const primary = holdRequests('post', '/v1/me'); try { await manage(actor, WORK_LABEL, 'Set as primary'); @@ -111,18 +110,18 @@ describe('the user profile phone section', () => { 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(initialOrder); + 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(initialOrder); + expect(listed()).toEqual([WORK_LABEL, HOME_LABEL]); expect(screen.queryByRole('progressbar', { name: 'Set as primary' })).toBeNull(); }); - it('lists verified numbers in order and marks the primary', async () => { + it('lists the primary number first and marks it', async () => { await renderSection(signedIn([HOME, WORK])); expect(listed()).toEqual([HOME_LABEL, WORK_LABEL]); @@ -211,8 +210,7 @@ describe('the user profile phone section', () => { await manage(actor, WORK_LABEL, 'Set as primary'); - await waitFor(() => expect(within(row()).getByText(WORK_LABEL).parentElement).toHaveTextContent('Primary')); - expect(listed()).toEqual([HOME_LABEL, WORK_LABEL]); + await waitFor(() => expect(listed()).toEqual([WORK_LABEL, HOME_LABEL])); expect(within(row()).getByText(WORK_LABEL).parentElement).toHaveTextContent('Primary'); }); 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 4f120f3ad34..45e4f009a6a 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('holds the wallet menu while setting primary and updates the badge in place', async () => { + it('holds the wallet menu while setting primary and moves the new primary first', async () => { const fapi = await renderWeb3({ web3_wallets: [ fapiWeb3Wallet({ id: 'wallet_1', web3_wallet: '0x1234567890abcdef' }), @@ -498,7 +498,7 @@ describe('Web3 wallets', () => { 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( + expect(screen.getAllByRole('button', { name: 'Manage MetaMask' })[0].closest('.cl-section-row')).toContainElement( screen.getByTitle('0xabcdef1234567890'), ); const selectedRow = screen.getByTitle('0xabcdef1234567890').closest('.cl-section-row'); @@ -755,14 +755,14 @@ describe('Web3 wallets', () => { primary_web3_wallet_id: 'wallet_2', }); const user = userEvent.setup(); - const first = screen.getAllByRole('button', { name: 'Manage MetaMask' })[0]; + const first = screen.getAllByRole('button', { name: 'Manage MetaMask' })[1]; first.focus(); await user.keyboard('{Enter}'); await user.click(await screen.findByRole('menuitem', { name: 'Remove wallet' })); await user.keyboard('{Escape}'); await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(2); - await openMenu(user, screen.getAllByRole('button', { name: 'Manage MetaMask' })[1]); + await openMenu(user, screen.getAllByRole('button', { name: 'Manage MetaMask' })[0]); await user.click(screen.getByRole('menuitem', { name: 'Remove wallet' })); expect(screen.getByRole('alertdialog')).toHaveTextContent('0xabcd...7890'); await user.click(screen.getByRole('button', { name: 'Remove' })); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.model.test.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.model.test.ts index 362bdc035d7..b35b021cd21 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.model.test.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.model.test.ts @@ -122,7 +122,7 @@ describe('Web3 wallet projection', () => { } }); - it('WEB3-04 orders verified, then unverified wallets without moving the primary or mutating the input', () => { + it('WEB3-04 orders primary, verified, then unverified wallets without mutating the input', () => { const wallets = Object.freeze([ { ...metamask, @@ -134,7 +134,7 @@ describe('Web3 wallet projection', () => { { ...metamask, id: 'primary', - verification: { ...metamask.verification, status: null }, + verification: { ...metamask.verification, status: 'unverified' as const }, }, { ...metamask, @@ -148,28 +148,17 @@ describe('Web3 wallet projection', () => { enabledStrategies: ['web3_metamask_signature'], allowCreation: false, }); - const changedPrimary = projectWeb3Wallets({ - wallets, - primaryId: 'unverified_later', - enabledStrategies: ['web3_metamask_signature'], - allowCreation: false, - }); expect(result.status).toBe('ready'); if (result.status === 'ready') { expect(result.wallets.map(wallet => wallet.id)).toEqual([ + 'primary', 'verified_a', 'verified_z', 'unverified_earlier', 'unverified_later', - 'primary', ]); expect(result.availableProviders).toEqual([]); } - if (result.status === 'ready' && changedPrimary.status === 'ready') { - expect(changedPrimary.wallets.map(wallet => wallet.id)).toEqual(result.wallets.map(wallet => wallet.id)); - expect(changedPrimary.wallets.find(wallet => wallet.id === 'unverified_later')?.isPrimary).toBe(true); - expect(changedPrimary.wallets.find(wallet => wallet.id === 'primary')?.isPrimary).toBe(false); - } expect(wallets.map(wallet => wallet.id)).toEqual([ 'unverified_later', 'verified_z', diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts index 965a41e7bfe..9da99dda1c5 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts @@ -45,11 +45,13 @@ export function projectWeb3Wallets({ wallet.verification.strategy === 'admin' || WEB3_PROVIDERS.some(provider => provider.strategy === wallet.verification.strategy), ); - const verified = knownWallets.filter(wallet => wallet.verification.status === 'verified'); - const unverified = knownWallets.filter( + const primary = knownWallets.filter(wallet => wallet.id === primaryId); + const remaining = knownWallets.filter(wallet => wallet.id !== primaryId); + const verified = remaining.filter(wallet => wallet.verification.status === 'verified'); + const unverified = remaining.filter( wallet => wallet.verification.status !== null && wallet.verification.status !== 'verified', ); - const withoutStatus = knownWallets.filter(wallet => wallet.verification.status === null); + const withoutStatus = remaining.filter(wallet => wallet.verification.status === null); verified.sort((first, second) => first.id.localeCompare(second.id)); unverified.sort((first, second) => { @@ -58,7 +60,7 @@ export function projectWeb3Wallets({ return firstExpiry && secondExpiry ? firstExpiry.getTime() - secondExpiry.getTime() : 0; }); - const sorted = [...verified, ...unverified, ...withoutStatus]; + const sorted = [...primary, ...verified, ...unverified, ...withoutStatus]; const connectedStrategies = new Set(wallets.map(wallet => wallet.verification.strategy)); diff --git a/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx b/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx index 4d9fa334ebf..84ac6693bfd 100644 --- a/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx +++ b/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx @@ -90,13 +90,12 @@ describe('shared action admission', () => { await act(() => pending.resolve()); }); - it('preserves the primary error and focus key while removal runs', async () => { + it('keeps the focus key and clears the primary error while removal runs', async () => { const pending = deferred(); const failure = new SaveError({ global: { code: 'action_blocked', message: 'Primary refused.' } }); const { result } = renderHook(() => useOwners(() => pending.promise)); await act(() => result.current.actions.run('email', () => Promise.reject(failure))); - const error = result.current.actions.error; - expect(error).toBeDefined(); + expect(result.current.actions.error).toBeDefined(); act(() => result.current.remove.onOpenChange(true)); act(() => { @@ -106,11 +105,11 @@ describe('shared action admission', () => { expect(result.current.actions.isPending).toBe(true); expect(result.current.actions.pendingKey).toBeUndefined(); expect(result.current.actions.lastKey).toBe('email'); - expect(result.current.actions.errorKey).toBe('email'); - expect(result.current.actions.error).toBe(error); + 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.error).toBe(error); + expect(result.current.actions.lastKey).toBe('email'); }); it.each(['resolve', 'reject'])('holds pending after cancellation until removal promises %s', async outcome => { diff --git a/packages/mosaic/src/hooks/use-pending-action.ts b/packages/mosaic/src/hooks/use-pending-action.ts index e659bbec8e8..f741ae4256a 100644 --- a/packages/mosaic/src/hooks/use-pending-action.ts +++ b/packages/mosaic/src/hooks/use-pending-action.ts @@ -31,7 +31,7 @@ type Pending = { kind: 'inline'; key: TKey } | { kind: 'external' }; /** * Runs inline and external actions 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 it starts. + * `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, @@ -48,6 +48,7 @@ export function usePendingAction({ } running.current = true; setPending(next); + setFailure(undefined); return (async () => { try { return await action(); @@ -62,7 +63,6 @@ export function usePendingAction({ return ( claim({ kind: 'inline', key }, async () => { setLastKey(key); - setFailure(undefined); try { await action(); return true; From afdd068d19eac626a0ca99e71c6a12eb26b44af2 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 21:15:03 -0600 Subject: [PATCH 09/12] refactor(mosaic): let controllers own identifier row pending state usePendingAction gains guard(), which runs a confirmation's confirm under the section lock, replacing runExternal and the wrapper each section copied. Section controllers put verify and primary pending flags on the rows they return, so views no longer compare pending keys. Confirmation drops the disabled prop, which no reachable state could set. --- packages/mosaic/AGENTS.md | 2 +- packages/mosaic/docs/controllers.md | 24 +++++------ .../__tests__/confirmation.feature.test.tsx | 20 +--------- .../src/blocks/confirmation/confirmation.tsx | 4 +- .../user-profile-contact-list-row.view.tsx | 12 ++---- .../user-profile-contact.types.ts | 5 +++ .../user-profile-email-section.controller.ts | 14 +++---- .../user-profile-email-section.types.ts | 5 +-- .../user-profile-email-section.view.tsx | 4 -- .../user-profile-phone-section.controller.ts | 15 ++++--- .../user-profile-phone-section.types.ts | 5 +-- .../user-profile-phone-section.view.tsx | 4 -- ...e-web3-wallets-section.controller.test.tsx | 16 +++++--- .../user-profile-web3-wallet-row.view.tsx | 7 +--- ...profile-web3-wallets-section.controller.ts | 36 ++++++++--------- .../user-profile-web3-wallets-section.tsx | 2 - ...user-profile-web3-wallets-section.types.ts | 5 ++- ...user-profile-web3-wallets-section.view.tsx | 11 +---- .../use-pending-action-shared.test.tsx | 40 +------------------ .../mosaic/src/hooks/use-pending-action.ts | 15 +++++-- 20 files changed, 87 insertions(+), 159 deletions(-) diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index 5b64bb5fa29..1cf9ce0a5a4 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -34,7 +34,7 @@ Most problems a feature hits already have a shared answer. Find the row for your | Problem | Use | | ------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------- | | An async action from a button or row: pending, an error, one lock | `usePendingAction` (`src/hooks/`) | -| Direct actions and confirmation dialogs that must exclude each other | Section controller wraps `confirm` with `usePendingAction.runExternal` (`src/hooks/`, `src/blocks/confirm/`) | +| Direct actions and confirmation dialogs that must exclude each other | `usePendingAction.guard` on the confirmation (`src/hooks/`, `src/blocks/confirm/`) | | A form: values, validation, submit, field errors | `useForm` (`src/components/form/`), with `validateAsync` for a server-side check | | A `` element, without the browser's validation popup | `Form` (`src/primitives/form/`) | | A model save whose Clerk errors should land on form fields | `save` (`src/utils/errors.ts`) in the model, or reject with `FormSubmitError` | diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index 32a5fb699c9..c281f9bd2c9 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -45,8 +45,11 @@ 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)), }; ``` @@ -61,27 +64,24 @@ const remove = useConfirmController({ }); return { + contacts: contacts.map(contact => ({ ...contact, isPrimaryPending: actions.pendingKey === contact.id })), isDisabled: actions.isPending, - pendingPrimaryId: actions.pendingKey, - remove: { - ...remove, - confirm: async contact => (await actions.runExternal(() => remove.confirm(contact))) ?? false, - }, + remove: actions.guard(remove), }; ``` Keep this coordination in the section controller. Run direct actions with -`actions.run(key, action)` and wrap the returned confirmation's `confirm` callback -with `actions.runExternal`. Pass `actions.isPending` to the section controls' -`disabled` props, including `Confirmation.disabled`. +`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. Views receive the pending and disabled props without coordinating -the operations. +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 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 b256db420ed..74d87837077 100644 --- a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx +++ b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx @@ -38,11 +38,7 @@ function Host({ async function renderAccount( options: ConfirmControllerOptions, - { - localization, - description, - disabled, - }: { localization?: MosaicLocalization; description?: ReactNode; disabled?: boolean } = {}, + { localization, description }: { localization?: MosaicLocalization; description?: ReactNode } = {}, ) { const ref: { current: ConfirmController | undefined } = { current: undefined }; serveFapi({ client: fapiClient([session]) }); @@ -57,7 +53,6 @@ async function renderAccount( title={TITLE} description={description ?? 'Google will be removed from this account.'} actionLabel='Remove' - disabled={disabled} /> ); }} @@ -75,19 +70,6 @@ async function renderAccount( const confirmButton = () => screen.getByRole('button', { name: 'Remove' }); describe('Confirmation', () => { - it('disables confirmation while leaving Cancel usable', async () => { - const onConfirm = vi.fn(() => Promise.resolve()); - const { user, open, closed } = await renderAccount({ onConfirm }, { disabled: true }); - await open(); - - expect(confirmButton()).toBeDisabled(); - await user.click(confirmButton()); - expect(onConfirm).not.toHaveBeenCalled(); - - await user.click(screen.getByRole('button', { name: 'Cancel' })); - await closed(); - }); - it('opens from its trigger, confirms, closes once the action lands, and hands focus back', async () => { const run = deferred(); const onConfirm = vi.fn(() => run.promise); diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.tsx b/packages/mosaic/src/blocks/confirmation/confirmation.tsx index 0387ddec10d..02dd5b79491 100644 --- a/packages/mosaic/src/blocks/confirmation/confirmation.tsx +++ b/packages/mosaic/src/blocks/confirmation/confirmation.tsx @@ -10,7 +10,6 @@ export type ConfirmationColor = 'negative' | 'primary'; export interface ConfirmationProps extends ConfirmBlockProps { /** The weight the confirming button carries. An action that can be undone takes `primary` (default: `negative`) */ color?: ConfirmationColor; - disabled?: boolean; } /** @@ -37,7 +36,7 @@ export interface ConfirmationProps extends ConfirmBlockProps */ -export function Confirmation({ color = 'negative', disabled, ...props }: ConfirmationProps) { +export function Confirmation({ color = 'negative', ...props }: ConfirmationProps) { return ( {({ title, description, actionLabel, cancelLabel, isPending, errorMessage, confirm }) => ( @@ -68,7 +67,6 @@ export function Confirmation({ color = 'negative', disabled, ... fullWidth color={color} isPending={isPending} - disabled={disabled && !isPending} onClick={confirm} > {actionLabel} 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 a55e508de6f..fd816f50fcf 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,11 +19,9 @@ export interface UserProfileContactListRowViewProps { addAction?: ReactNode; kind: 'email' | 'phone'; label: string; - items: UserProfileContact[]; + items: UserProfileContactRow[]; onAdd?: () => void; onVerify?: (id: string) => void; - pendingVerifyId?: string; - pendingPrimaryId?: string; isDisabled?: boolean; onSetPrimary?: (id: string) => void; onRemove?: (item: UserProfileContact) => void; @@ -36,8 +34,6 @@ export function UserProfileContactListRowView({ items, onAdd, onVerify, - pendingVerifyId, - pendingPrimaryId, isDisabled = false, onSetPrimary, onRemove, @@ -96,13 +92,13 @@ export function UserProfileContactListRowView({ actions.push({ label: item.isDefault ? m.completeVerification : m[kind].verify, onClick: () => 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), - status: pendingPrimaryId === item.id ? 'pending' : 'idle', + status: item.isPrimaryPending ? 'pending' : 'idle', }); } 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/user-profile-email-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts index c6d8cc77525..bcabf6d4247 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 @@ -36,20 +36,18 @@ export function useUserProfileEmailSectionController({ }); return { - emails, + emails: emails.map(email => ({ + ...email, + isVerifyPending: verification.pendingVerifyId === email.id, + isPrimaryPending: actions.pendingKey === email.id, + })), canAdd: Boolean(onCreateEmail), verification, error: actions.error, isDisabled: actions.isPending, - pendingPrimaryId: actions.pendingKey, rowRef, registerTrigger: removalFocus.registerTrigger, - remove: onRemoveEmail - ? { - ...remove, - confirm: async email => (await actions.runExternal(() => remove.confirm(email))) ?? false, - } - : undefined, + remove: onRemoveEmail ? actions.guard(remove) : undefined, finalFocus: removalFocus.finalFocus, verificationFinalFocus: () => (verification.verifyingId ? removalFocus.trigger(verification.verifyingId) : null), setPrimaryReverification, 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 88b7e698fe5..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,14 +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; - pendingPrimaryId: string | undefined; 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 6cee98a1799..2f1e1404947 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 @@ -14,7 +14,6 @@ export function UserProfileEmailSectionView({ emails, canAdd, isDisabled, - pendingPrimaryId, verification, error, rowRef, @@ -63,8 +62,6 @@ export function UserProfileEmailSectionView({ onSetPrimary={onSetPrimary} onVerify={onVerify} isDisabled={isDisabled} - pendingPrimaryId={pendingPrimaryId} - pendingVerifyId={verification.pendingVerifyId} > {error} @@ -79,7 +76,6 @@ export function UserProfileEmailSectionView({ {remove ? ( fill(email.isVerified ? m.email.removeDialog.verifiedDescription : m.email.removeDialog.description, { 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 b90085b29df..33e40dae3f9 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 @@ -36,21 +36,20 @@ 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, + })), defaultPhoneCountry, canAdd: Boolean(onCreatePhone), verification, error: actions.error, isDisabled: actions.isPending, - pendingPrimaryId: actions.pendingKey, rowRef, registerTrigger: removalFocus.registerTrigger, - remove: onRemovePhone - ? { - ...remove, - confirm: async phone => (await actions.runExternal(() => remove.confirm(phone))) ?? false, - } - : undefined, + remove: onRemovePhone ? actions.guard(remove) : undefined, finalFocus: removalFocus.finalFocus, verificationFinalFocus: () => (verification.verifyingId ? removalFocus.trigger(verification.verifyingId) : null), setPrimaryReverification, 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 d4abc3d5a6b..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,15 +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; - pendingPrimaryId: string | undefined; 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 3acdf8c8bc1..e3e8d6963b6 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 @@ -15,7 +15,6 @@ export function UserProfilePhoneSectionView({ defaultPhoneCountry, canAdd, isDisabled, - pendingPrimaryId, verification, error, rowRef, @@ -64,8 +63,6 @@ export function UserProfilePhoneSectionView({ onSetPrimary={onSetPrimary} onVerify={onVerify} isDisabled={isDisabled} - pendingPrimaryId={pendingPrimaryId} - pendingVerifyId={verification.pendingVerifyId} > {error} @@ -81,7 +78,6 @@ export function UserProfilePhoneSectionView({ {remove ? ( fill(phone.isVerified ? m.phone.removeDialog.verifiedDescription : m.phone.removeDialog.description, { 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..2cbfaa26cbe 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 @@ -14,6 +14,10 @@ const providers = [ { id: 'web3_solana_signature', provider: 'Solana', walletPicker: 'solana' }, ] satisfies UserProfileWeb3Provider[]; +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 +46,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); @@ -79,7 +83,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 +141,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 +176,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 +233,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 +241,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/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 553f747bf05..95c3380aca1 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,8 +17,6 @@ import type { UserProfileWeb3Provider, UserProfileWeb3Wallet } from './user-prof export function UserProfileWeb3WalletRowView({ wallet, triggerRef, - isPending = false, - isPrimaryPending = false, isDisabled = false, onConnect, onVerify, @@ -27,8 +25,6 @@ export function UserProfileWeb3WalletRowView({ }: { wallet: UserProfileWeb3Wallet | UserProfileWeb3Provider; triggerRef?: Ref; - isPending?: boolean; - isPrimaryPending?: boolean; isDisabled?: boolean; onConnect?: (id: string) => void; onVerify?: (id: string) => void; @@ -37,6 +33,7 @@ export function UserProfileWeb3WalletRowView({ }) { const m = useMessages('userProfileWeb3Wallets'); 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[] = []; @@ -48,7 +45,7 @@ export function UserProfileWeb3WalletRowView({ actions.push({ label: m.setPrimary, onClick: () => onSetPrimary(wallet.id), - status: isPrimaryPending ? 'pending' : 'idle', + status: linkedWallet.isPrimaryPending ? 'pending' : 'idle', }); } if (linkedWallet && onRemove && linkedWallet.canRemove !== false) { 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 965bab7a43d..474397e9b37 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 @@ -149,18 +149,22 @@ 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' }; + 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 && @@ -168,18 +172,10 @@ export function useUserProfileWeb3WalletsController({ !availableProviders.some(provider => provider.id === errorId) ? action.error : undefined, - pendingId, - pendingPrimaryId: owner?.kind === 'primary' ? pendingId : undefined, isDisabled: action.isPending, sectionRef, registerTrigger: removalFocus.registerTrigger, - remove: remove - ? { - ...removeWallet, - confirm: async (wallet: UserProfileWeb3Wallet) => - (await action.runExternal(() => removeWallet.confirm(wallet))) ?? false, - } - : 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 9a9eb2cab3d..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,8 +34,6 @@ function Web3Wallets({ model }: { model: ReadyWeb3WalletsModel }) { wallets={controller.wallets} sectionRef={controller.sectionRef} availableProviders={controller.availableProviders} - pendingId={controller.pendingId} - pendingPrimaryId={controller.pendingPrimaryId} isDisabled={controller.isDisabled} error={controller.error} 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 87c625e2391..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,14 +31,14 @@ export interface UserProfileWeb3Wallet { canRemove?: boolean; primaryError?: string; verifyError?: string; + isPending?: boolean; + isPrimaryPending?: boolean; } export interface UserProfileWeb3WalletsSectionViewProps { wallets: UserProfileWeb3Wallet[]; sectionRef?: Ref; availableProviders?: UserProfileWeb3Provider[]; - pendingId?: string; - pendingPrimaryId?: string; isDisabled?: boolean; error?: string; onConnect?: (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 fdd044c14d8..0b166eebab6 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,8 +10,6 @@ export function UserProfileWeb3WalletsSectionView({ wallets, sectionRef, availableProviders = [], - pendingId, - pendingPrimaryId, isDisabled = false, error, onConnect, @@ -27,7 +25,6 @@ export function UserProfileWeb3WalletsSectionView({ }: UserProfileWeb3WalletsSectionViewProps) { const m = useMessages('userProfileWeb3Wallets'); const hasRows = wallets.length > 0 || (availableProviders.length > 0 && Boolean(onConnect)) || Boolean(error); - const isBusy = isDisabled || pendingId !== undefined; return ( <> @@ -48,9 +45,7 @@ export function UserProfileWeb3WalletsSectionView({ key={wallet.id} wallet={wallet} triggerRef={registerTrigger(wallet.id)} - isPending={pendingId === wallet.id} - isPrimaryPending={pendingPrimaryId === wallet.id} - isDisabled={isBusy} + isDisabled={isDisabled} onVerify={onVerify} onSetPrimary={onSetPrimary} onRemove={remove ? wallet => remove.open(wallet) : undefined} @@ -62,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} /> )) @@ -76,7 +70,6 @@ export function UserProfileWeb3WalletsSectionView({ {remove ? ( fill(wallet.isVerified ? m.removeDialog.verifiedDescription : m.removeDialog.description, { diff --git a/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx b/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx index 84ac6693bfd..6c8c57e9289 100644 --- a/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx +++ b/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx @@ -12,13 +12,7 @@ function useOwners(onConfirm: () => Promise) { onConfirm, reverification: { status: 'loading', visible: true, onCancel: vi.fn() }, }); - return { - actions, - remove: { - ...remove, - confirm: async () => (await actions.runExternal(() => remove.confirm())) ?? false, - }, - }; + return { actions, remove: actions.guard(remove) }; } describe('shared action admission', () => { @@ -139,35 +133,3 @@ describe('shared action admission', () => { await act(async () => expect(await result.current.actions.run('primary', primary)).toBe(true)); }); }); - -describe('external pending actions', () => { - it('returns the accepted result and refuses reentrant work synchronously', async () => { - const { result } = renderHook(() => usePendingAction()); - const other = vi.fn(); - await act(async () => { - const accepted = result.current.runExternal(() => { - expect(result.current.runExternal(other)).toBeUndefined(); - return 42; - }); - expect(await accepted).toBe(42); - }); - expect(other).not.toHaveBeenCalled(); - expect(result.current.isPending).toBe(false); - }); - - it.each(['throw', 'reject'])('preserves the original %s and releases admission', async mode => { - const { result } = renderHook(() => usePendingAction()); - const failure = new Error('Original failure'); - await act(async () => { - const accepted = result.current.runExternal(() => { - if (mode === 'throw') { - throw failure; - } - return Promise.reject(failure); - }); - await expect(accepted).rejects.toBe(failure); - }); - expect(result.current.isPending).toBe(false); - await act(async () => expect(await result.current.runExternal(() => 'retry')).toBe('retry')); - }); -}); diff --git a/packages/mosaic/src/hooks/use-pending-action.ts b/packages/mosaic/src/hooks/use-pending-action.ts index f741ae4256a..e620d828510 100644 --- a/packages/mosaic/src/hooks/use-pending-action.ts +++ b/packages/mosaic/src/hooks/use-pending-action.ts @@ -10,9 +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; - runExternal: (action: () => T | Promise) => Promise | undefined; + guard: >( + confirm: TConfirm & PendingConfirm, + ) => TConfirm; pendingKey: TKey | undefined; lastKey: TKey | undefined; isPending: boolean; @@ -30,7 +36,7 @@ interface PendingActionFailure { type Pending = { kind: 'inline'; key: TKey } | { kind: 'external' }; /** - * Runs inline and external actions under one lock and tracks which inline key is pending, ran last, or last failed. + * 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({ @@ -78,7 +84,10 @@ export function usePendingAction({ return { run, - runExternal: action => claim({ kind: 'external' }, action), + guard: confirm => ({ + ...confirm, + confirm: async payload => (await claim({ kind: 'external' }, () => confirm.confirm(payload))) ?? false, + }), pendingKey: pending?.kind === 'inline' ? pending.key : undefined, lastKey, isPending: pending !== undefined, From 3c07da9647390ce20990375df628ce7ef7ad4a27 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 21:15:11 -0600 Subject: [PATCH 10/12] fix(mosaic): close the identifier menu once primary reverification opens Set as primary stayed pending while the session verification dialog was open, so its menu stayed open with a spinner behind the dialog. The pending status now ends when the dialog appears, as ActionMenu expects. --- .../__tests__/user-profile-email-section.feature.test.tsx | 1 + .../user-profile-email-section.controller.ts | 2 +- .../__tests__/user-profile-phone-section.feature.test.tsx | 1 + .../user-profile-phone-section.controller.ts | 2 +- .../user-profile-web3-wallets-section.controller.test.tsx | 4 ++++ .../user-profile-web3-wallets-section.feature.test.tsx | 1 + .../user-profile-web3-wallets-section.controller.ts | 6 +++++- 7 files changed, 14 insertions(+), 3 deletions(-) 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 bb5ee417039..72bd80229ad 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 @@ -344,6 +344,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}'); 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 bcabf6d4247..25e926af5ce 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 @@ -39,7 +39,7 @@ export function useUserProfileEmailSectionController({ emails: emails.map(email => ({ ...email, isVerifyPending: verification.pendingVerifyId === email.id, - isPrimaryPending: actions.pendingKey === email.id, + isPrimaryPending: actions.pendingKey === email.id && !setPrimaryReverification.visible, })), canAdd: Boolean(onCreateEmail), verification, 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 500299d4804..2e40516e6cc 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 @@ -485,6 +485,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}'); 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 33e40dae3f9..ef0203a5817 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 @@ -40,7 +40,7 @@ export function useUserProfilePhoneSectionController({ ...phone, value: stringToFormattedPhoneString(phone.value), isVerifyPending: verification.pendingVerifyId === phone.id, - isPrimaryPending: actions.pendingKey === phone.id, + isPrimaryPending: actions.pendingKey === phone.id && !setPrimaryReverification.visible, })), defaultPhoneCountry, canAdd: Boolean(onCreatePhone), 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 2cbfaa26cbe..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,8 @@ 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; } @@ -73,6 +76,7 @@ describe('Web3 connection controller', () => { verify: vi.fn(), setPrimary, remove: vi.fn(), + setPrimaryReverification: idleReverification, }), ); 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 45e4f009a6a..4592873d3fe 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 @@ -601,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}'); 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 474397e9b37..369cbad42be 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 @@ -151,7 +151,11 @@ export function useUserProfileWeb3WalletsController({ return { wallets: wallets.map(wallet => { const isPending = pendingId === wallet.id; - const row = { ...wallet, isPending, isPrimaryPending: isPending && owner?.kind === 'primary' }; + const row = { + ...wallet, + isPending, + isPrimaryPending: isPending && owner?.kind === 'primary' && !setPrimaryReverification.visible, + }; return errorId === wallet.id ? owner?.kind === 'primary' ? { ...row, primaryError: action.error } From f0bc48b25ca43692d2c6fd948eac6718c272bccb Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 21:37:41 -0600 Subject: [PATCH 11/12] fix(mosaic): hold identifier row order when the primary changes Email, phone and web3 wallet rows list the primary first when the section loads, then keep their positions when a new primary is set, so only the Primary badge moves. The new primary leads on the next load. --- .../__tests__/user-profile-email-section.feature.test.tsx | 4 ++-- .../user-profile-email-section.controller.ts | 7 +++++-- .../__tests__/user-profile-phone-section.feature.test.tsx | 8 ++++---- .../user-profile-phone-section.controller.ts | 7 +++++-- .../user-profile-web3-wallets-section.feature.test.tsx | 4 ++-- .../user-profile-web3-wallets-section.controller.ts | 5 ++++- 6 files changed, 22 insertions(+), 13 deletions(-) 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 72bd80229ad..c8efedae675 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 @@ -68,7 +68,7 @@ async function enterCode(actor: Actor, code: string) { } describe('the user profile email section', () => { - it('holds the menu while setting an address as primary, then moves the row first with focus', async () => { + 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'); @@ -93,7 +93,7 @@ describe('the user profile email section', () => { } expect(screen.getByRole('button', { name: 'Manage bob@example.com' })).not.toHaveAttribute('aria-disabled', 'true'); await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); - expect(emailsListed()).toEqual(['bob@example.com', 'alice@example.com']); + 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(); }); 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 25e926af5ce..af5666ef02e 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,6 +19,8 @@ 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, 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 2e40516e6cc..4b5052b7a63 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 @@ -92,7 +92,7 @@ async function enterCode(actor: Actor, code: string) { } describe('the user profile phone section', () => { - it('holds the menu while setting a number as primary, then moves the row first', async () => { + 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 { @@ -117,7 +117,7 @@ describe('the user profile phone section', () => { } expect(screen.getByRole('button', { name: `Manage ${WORK_LABEL}` })).not.toHaveAttribute('aria-disabled', 'true'); await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); - expect(listed()).toEqual([WORK_LABEL, HOME_LABEL]); + expect(listed()).toEqual([HOME_LABEL, WORK_LABEL]); expect(screen.queryByRole('progressbar', { name: 'Set as primary' })).toBeNull(); }); @@ -210,8 +210,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 () => { 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 ef0203a5817..3376aa8c0a5 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,6 +20,8 @@ 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, 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 4592873d3fe..ac57d6c1b52 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('holds the wallet menu while setting primary and moves the new primary first', 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' }), @@ -498,7 +498,7 @@ describe('Web3 wallets', () => { 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' })[0].closest('.cl-section-row')).toContainElement( + 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'); 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 369cbad42be..46793528920 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('userProfileWeb3Wallets'); + 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(); From ee99bc2ecca80cc61c854584e497d3ae3556de5c Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 21:48:13 -0600 Subject: [PATCH 12/12] test(mosaic): fold guard tests into the usePendingAction suite --- .../use-pending-action-shared.test.tsx | 135 ------------------ .../__tests__/use-pending-action.test.tsx | 133 +++++++++++++++++ 2 files changed, 133 insertions(+), 135 deletions(-) delete mode 100644 packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx diff --git a/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx b/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx deleted file mode 100644 index 6c8c57e9289..00000000000 --- a/packages/mosaic/src/hooks/__tests__/use-pending-action-shared.test.tsx +++ /dev/null @@ -1,135 +0,0 @@ -import { act, renderHook } from '@testing-library/react'; -import { describe, expect, it, vi } from 'vitest'; - -import { deferred } from '../../__tests__/async'; -import { useConfirmController } from '../../blocks/confirm'; -import { SaveError } from '../../utils/errors'; -import { usePendingAction } from '../use-pending-action'; - -function useOwners(onConfirm: () => Promise) { - const actions = usePendingAction(); - const remove = useConfirmController({ - onConfirm, - reverification: { status: 'loading', visible: true, onCancel: vi.fn() }, - }); - return { actions, remove: actions.guard(remove) }; -} - -describe('shared action admission', () => { - it('refuses removal in the same stack as primary without changing the dialog', async () => { - const pending = deferred(); - const onConfirm = vi.fn(() => Promise.resolve()); - const { result } = renderHook(() => useOwners(onConfirm)); - act(() => result.current.remove.onOpenChange(true)); - const owners = result.current; - - act(() => { - void owners.actions.run('primary', () => 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 primary in the same stack as removal without changing primary metadata', async () => { - const pending = deferred(); - const primary = vi.fn(); - const { result } = renderHook(() => useOwners(() => pending.promise)); - act(() => result.current.remove.onOpenChange(true)); - const owners = result.current; - - await act(async () => { - void owners.remove.confirm(); - expect(await owners.actions.run('primary', primary)).toBe(false); - }); - - expect(primary).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('primary', primary)).toBe(true)); - expect(primary).toHaveBeenCalledOnce(); - }); - - it('keeps a rejected removal error while primary makes confirmation busy', async () => { - const pending = deferred(); - const failure = new SaveError({ global: { code: 'action_blocked', message: 'Removal refused.' } }); - const onConfirm = vi.fn(() => Promise.reject(failure)); - const { result } = renderHook(() => useOwners(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('primary', () => 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 focus key and clears the primary error while removal runs', async () => { - const pending = deferred(); - const failure = new SaveError({ global: { code: 'action_blocked', message: 'Primary refused.' } }); - const { result } = renderHook(() => useOwners(() => pending.promise)); - await act(() => result.current.actions.run('email', () => 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('email'); - 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('email'); - }); - - it.each(['resolve', 'reject'])('holds pending after cancellation until removal promises %s', async outcome => { - const pending = deferred(); - const primary = vi.fn(); - const { result } = renderHook(() => useOwners(() => 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('primary', primary)).toBe(false)); - expect(primary).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 removal 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('primary', primary)).toBe(true)); - }); -}); 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)); + }, + ); +});