From 757dd04393205f38c6391f0b35440ad538b4cf61 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 15:54:56 -0600 Subject: [PATCH 01/15] 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 b9185a45480..6bd2c75cea9 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx @@ -72,6 +72,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 a50b9a12fbf..7ea876d38f9 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx @@ -95,6 +95,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 d7e9edea35d907f77aad99753a6b4b2c10a2aca6 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 16:14:05 -0600 Subject: [PATCH 02/15] fix(mosaic): coordinate identifier mutations across action owners --- .changeset/friendly-identifier-lock.md | 2 + .../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 ++++--- 23 files changed, 310 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/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 c0df19156fc..81d2adf4a92 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'; @@ -77,6 +78,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; @@ -89,6 +91,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. */ @@ -111,6 +114,7 @@ export function useConfirmController({ onConfirm, reverification, errorFallback, + actionLock, }: ConfirmControllerOptions): ConfirmController { const handle = useMemo(() => Dialog.createHandle(), []); const [snapshot, send] = useMachine(confirmMachine); @@ -124,6 +128,7 @@ export function useConfirmController({ isOpen: snapshot.value !== 'idle' && snapshot.context.open, isPending, canClose, + canConfirm: snapshot.value === 'confirming' && !actionLock?.isPending, errorMessage: error ? errorText(error, errorFallback) : undefined, step: isPending && reverification?.visible ? 'verify' : 'confirm', reverification, @@ -143,12 +148,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 c41fc8008bc..1ff3d42010b 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} @@ -66,6 +66,7 @@ export function Confirmation({ color = 'negative', ...props }: C type='button' 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 808965533ca..bc3a47c2897 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, @@ -97,7 +98,7 @@ function DestructiveStep({ form={formId} 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 84098de9650..e18c17462fb 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, @@ -45,6 +51,7 @@ export function UserProfileContactListRowView({ const messages = useMessages('userProfile'); const m = messages.contact; const nouns = messages[kind]; + const shownPrimaryId = useSpinDelay(pendingPrimaryId ?? null, { minDuration: 0 }); return ( {item.value} - {item.isDefault ? {m.primary} : null} + {item.isDefault ? ( + {m.primary} + ) : shownPrimaryId === item.id ? ( + + ) : null} {item.isVerified ? null : {m.unverified}} @@ -120,6 +135,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 6bd2c75cea9..584d0bd44fe 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 @@ -86,7 +86,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(() => @@ -94,7 +94,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 c110e120037..a821e2a5de9 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, @@ -36,6 +38,7 @@ export function UserProfileEmailSectionView({ color='neutral' size='sm' variant='outline' + disabled={isDisabled} /> } > @@ -60,6 +63,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 7ea876d38f9..9253c267460 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 @@ -108,13 +108,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 f5c0d37718b..e639ce9329b 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, @@ -37,6 +39,7 @@ export function UserProfilePhoneSectionView({ color='neutral' size='sm' variant='outline' + disabled={isDisabled} /> } > @@ -61,6 +64,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 d87adfbd593..2ed663678da 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'; @@ -19,6 +20,7 @@ export function UserProfileWeb3WalletRowView({ wallet, triggerRef, isPending = false, + isPrimaryPending = false, isDisabled = false, onConnect, onVerify, @@ -28,6 +30,7 @@ export function UserProfileWeb3WalletRowView({ wallet: UserProfileWeb3Wallet | UserProfileWeb3Provider; triggerRef?: Ref; isPending?: boolean; + isPrimaryPending?: boolean; isDisabled?: boolean; onConnect?: (id: string) => void; onVerify?: (id: string) => void; @@ -69,7 +72,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 9d242b0b1ae..6f599373ca6 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('userProfile').web3Wallets; - 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 aeab97de06c..5b30b2fdce7 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('userProfile').web3Wallets; 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 597f10e6d5aa8c610991006f35b9500dc561464d Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 20:03:39 -0600 Subject: [PATCH 03/15] 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 e18c17462fb..c7294a95e4b 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'; @@ -51,7 +49,6 @@ export function UserProfileContactListRowView({ const messages = useMessages('userProfile'); const m = messages.contact; const nouns = messages[kind]; - const shownPrimaryId = useSpinDelay(pendingPrimaryId ?? null, { minDuration: 0 }); return ( onSetPrimary(item.id) }); + actions.push({ + label: m.setPrimary, + onClick: () => onSetPrimary(item.id), + status: pendingPrimaryId === item.id ? 'pending' : 'idle', + }); } if (onRemove) { @@ -119,14 +120,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 584d0bd44fe..3392ea14aef 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx @@ -72,21 +72,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(() => @@ -94,6 +97,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(); }); @@ -411,6 +416,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')); }); @@ -524,7 +530,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 9253c267460..cf838ad6457 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx @@ -95,29 +95,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]); @@ -320,7 +328,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'); }); @@ -619,6 +628,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 7bf2d1ef0e7..8de1a4ca793 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 2ed663678da..7ff3e9e781e 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'; @@ -47,7 +46,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) }); @@ -72,14 +75,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 5b30b2fdce7..5ba1b25a49a 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('userProfile').web3Wallets; 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 92d3c02edc7fbba9b6fe30c772ebe558e82aeeaa Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 20:20:27 -0600 Subject: [PATCH 04/15] refactor(mosaic): pass confirmation disabled state directly --- .../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 ++-- 9 files changed, 29 insertions(+), 12 deletions(-) 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 81d2adf4a92..775a74b60ce 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -78,7 +78,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; @@ -128,7 +127,6 @@ export function useConfirmController({ isOpen: snapshot.value !== 'idle' && snapshot.context.open, 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 1ff3d42010b..ecb99650f6b 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} @@ -66,7 +67,7 @@ export function Confirmation({ color = 'negative', ...props }: C type='button' 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 bc3a47c2897..808965533ca 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, @@ -98,7 +97,7 @@ function DestructiveStep({ form={formId} 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 a821e2a5de9..396502abf5b 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 @@ -80,6 +80,7 @@ export function UserProfileEmailSectionView({ {remove ? ( fill(email.isVerified ? m.removeDialog.verifiedDescription : m.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 e639ce9329b..b3c8528a3b7 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 @@ -82,6 +82,7 @@ export function UserProfilePhoneSectionView({ {remove ? ( fill(phone.isVerified ? m.removeDialog.verifiedDescription : m.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 5ba1b25a49a..7d73527b7ae 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 ed3ce2dcb1780f3ae77bf3ca2d6b3cf12fd06ede Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 11:24:36 -0600 Subject: [PATCH 05/15] refactor(mosaic): share pending ownership for identifier actions --- .../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 ++++++----- 7 files changed, 105 insertions(+), 106 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/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts index 775a74b60ce..231ac60583c 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'; @@ -90,7 +89,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. */ @@ -113,7 +112,7 @@ export function useConfirmController({ onConfirm, reverification, errorFallback, - actionLock, + runAction, }: ConfirmControllerOptions): ConfirmController { const handle = useMemo(() => Dialog.createHandle(), []); const [snapshot, send] = useMachine(confirmMachine); @@ -149,8 +148,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 6f599373ca6..392400947cd 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('userProfile').web3Wallets; - 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 da55732a05af0a53cb408e439a6a41fb24be132d Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 11:54:20 -0600 Subject: [PATCH 06/15] refactor(mosaic): coordinate identifier actions in section controllers --- .../__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 +++++--- 8 files changed, 103 insertions(+), 37 deletions(-) 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 7fc0e8d8704..e82b0952eef 100644 --- a/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts +++ b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts @@ -114,6 +114,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; @@ -130,17 +169,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 231ac60583c..7fe2d539b5d 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -84,12 +84,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. */ @@ -112,7 +111,6 @@ export function useConfirmController({ onConfirm, reverification, errorFallback, - runAction, }: ConfirmControllerOptions): ConfirmController { const handle = useMemo(() => Dialog.createHandle(), []); const [snapshot, send] = useMachine(confirmMachine); @@ -142,20 +140,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 40b9b1f7df8..e0a051d3dcf 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts @@ -84,7 +84,7 @@ export function useUserProfileActiveDevicesController({ registerTrigger: removalFocus.registerTrigger, finalFocus: removalFocus.finalFocus, details, - signOutFromDetails: revoke ? device => details.confirm(device) : undefined, + signOutFromDetails: revoke ? device => void details.confirm(device) : undefined, signOut: revoke ? signOut : undefined, signOutAll: revokeAll ? signOutAll : undefined, // Confirming removes the other devices and the trigger with them — hence the dialog mounted diff --git a/packages/mosaic/src/features/user-profile/user-profile-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 392400947cd..a6877613f51 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 ed463d43ccb6c7f1688cc0370ebca1d7685812cd Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 13:22:39 -0600 Subject: [PATCH 07/15] 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 3392ea14aef..d6259b650d4 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx @@ -72,10 +72,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'); @@ -89,7 +88,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(() => @@ -98,7 +97,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(); }); @@ -530,7 +530,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 cf838ad6457..b91e353d687 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx @@ -95,9 +95,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'); @@ -114,18 +113,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]); @@ -328,8 +327,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 8de1a4ca793..3e9baf4931c 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 b14dca160e06f8611268bc036d7d41a1158a7004 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 21:15:03 -0600 Subject: [PATCH 08/15] 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. --- .../__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 +++++-- 18 files changed, 74 insertions(+), 146 deletions(-) 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 ecb99650f6b..c41fc8008bc 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 }) => ( @@ -67,7 +66,6 @@ export function Confirmation({ color = 'negative', disabled, ... type='button' 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 c7294a95e4b..f45b3c99a6e 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-contact-list-row.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-contact-list-row.view.tsx @@ -10,7 +10,7 @@ import { Section } from '../../components/section'; import { Plus } from '../../icons/glyphs/plus'; import { fill, useMessages } from '../../localization'; import { truncationStyles } from '../../styles/typography.styles'; -import type { UserProfileContact } from './user-profile-contact.types'; +import type { UserProfileContact, UserProfileContactRow } from './user-profile-contact.types'; import { styles } from './user-profile-profile-panel.styles'; export interface UserProfileContactListRowViewProps { @@ -19,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, @@ -97,13 +93,13 @@ export function UserProfileContactListRowView({ actions.push({ label: item.isDefault ? m.completeVerification : nouns.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 396502abf5b..6941aa480b1 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx @@ -14,7 +14,6 @@ export function UserProfileEmailSectionView({ emails, canAdd, isDisabled, - pendingPrimaryId, verification, error, rowRef, @@ -64,8 +63,6 @@ export function UserProfileEmailSectionView({ onSetPrimary={onSetPrimary} onVerify={onVerify} isDisabled={isDisabled} - pendingPrimaryId={pendingPrimaryId} - pendingVerifyId={verification.pendingVerifyId} > {error} @@ -80,7 +77,6 @@ export function UserProfileEmailSectionView({ {remove ? ( fill(email.isVerified ? m.removeDialog.verifiedDescription : m.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 b3c8528a3b7..f40fdcf72b6 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx @@ -15,7 +15,6 @@ export function UserProfilePhoneSectionView({ defaultPhoneCountry, canAdd, isDisabled, - pendingPrimaryId, verification, error, rowRef, @@ -65,8 +64,6 @@ export function UserProfilePhoneSectionView({ onSetPrimary={onSetPrimary} onVerify={onVerify} isDisabled={isDisabled} - pendingPrimaryId={pendingPrimaryId} - pendingVerifyId={verification.pendingVerifyId} > {error} @@ -82,7 +79,6 @@ export function UserProfilePhoneSectionView({ {remove ? ( fill(phone.isVerified ? m.removeDialog.verifiedDescription : m.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 7ff3e9e781e..961183a3479 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 @@ -18,8 +18,6 @@ import type { UserProfileWeb3Provider, UserProfileWeb3Wallet } from './user-prof export function UserProfileWeb3WalletRowView({ wallet, triggerRef, - isPending = false, - isPrimaryPending = false, isDisabled = false, onConnect, onVerify, @@ -28,8 +26,6 @@ export function UserProfileWeb3WalletRowView({ }: { wallet: UserProfileWeb3Wallet | UserProfileWeb3Provider; triggerRef?: Ref; - isPending?: boolean; - isPrimaryPending?: boolean; isDisabled?: boolean; onConnect?: (id: string) => void; onVerify?: (id: string) => void; @@ -38,6 +34,7 @@ export function UserProfileWeb3WalletRowView({ }) { const m = useMessages('userProfile').web3Wallets; const linkedWallet = 'address' in wallet ? wallet : undefined; + const isPending = wallet.isPending ?? false; const address = linkedWallet?.address; const shortAddress = address && (address.length <= 10 ? address : `${address.slice(0, 6)}...${address.slice(-4)}`); const actions: ActionMenuAction[] = []; @@ -49,7 +46,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 a6877613f51..f5667bc7eea 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 7d73527b7ae..a400c7b0329 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx @@ -10,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('userProfile').web3Wallets; 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 03f7136008c53e4045b1f79b3262186f0b1a627d Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 21:15:11 -0600 Subject: [PATCH 09/15] 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 d6259b650d4..1d463315b4e 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 @@ -390,6 +390,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 b91e353d687..08551bfcf4f 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 @@ -602,6 +602,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 3e9baf4931c..50eb3b87dfe 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 f5667bc7eea..c371c3ced34 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 d12ed7fb9817af631f1c8b2edd1cf66c9048d196 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 21:37:41 -0600 Subject: [PATCH 10/15] 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 1d463315b4e..9f8954eb6ed 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx @@ -72,7 +72,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'); @@ -97,7 +97,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 08551bfcf4f..80952c1fe7b 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx @@ -95,7 +95,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 { @@ -120,7 +120,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(); }); @@ -327,8 +327,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 50eb3b87dfe..b711cc2ec6a 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx @@ -464,7 +464,7 @@ describe('Web3 wallets', () => { await waitFor(() => expect(screen.getByText('Primary')).toBeInTheDocument()); }); - it('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 c371c3ced34..e5d7fff0b05 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts @@ -5,6 +5,7 @@ import { usePanelTitle } from '../../../components/panel'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; import { useMessages } from '../../../localization'; +import { applyOrder } from '../../../utils/apply-order'; import type { ReadyWeb3WalletsModel, UserProfileWeb3Provider, @@ -23,7 +24,7 @@ type WalletAction = | { kind: 'verify' | 'primary'; walletId: string }; export function useUserProfileWeb3WalletsController({ - wallets, + wallets: modelWallets, availableProviders, connect, verify, @@ -45,6 +46,8 @@ export function useUserProfileWeb3WalletsController({ | 'createReverification' >) { const messages = useMessages('userProfile').web3Wallets; + const [order] = useState(() => modelWallets.map(wallet => wallet.id)); + const wallets = applyOrder(order, modelWallets, wallet => wallet.id); const action = usePendingAction({ errorFallback: messages.errors.generic }); const [picker, setPicker] = useState({ open: false }); const [owner, setOwner] = useState(); From d5055e322255d8a83e37a060c5dd5a886c2aa841 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 21:48:13 -0600 Subject: [PATCH 11/15] 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)); + }, + ); +}); From cbafd6da7288fbd9edc9433d54827b43b4097124 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 3 Nov 2026 16:09:33 -0700 Subject: [PATCH 12/15] refactor(mosaic): drop usePendingAction.guard and combine pending state in sections Identifier sections disable their controls from both the pending action and the removal confirmation instead of decorating the confirm controller, and confirm returns void again. --- .../__tests__/confirm.controller.test.ts | 53 +------ .../src/blocks/confirm/confirm-shell.tsx | 2 +- .../src/blocks/confirm/confirm.controller.ts | 20 +-- ...ofile-active-devices-section.controller.ts | 2 +- .../user-profile-email-section.controller.ts | 4 +- .../user-profile-phone-section.controller.ts | 4 +- ...profile-web3-wallets-section.controller.ts | 4 +- .../__tests__/use-pending-action.test.tsx | 133 ------------------ .../mosaic/src/hooks/use-pending-action.ts | 67 +++------ 9 files changed, 40 insertions(+), 249 deletions(-) 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 e82b0952eef..7fc0e8d8704 100644 --- a/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts +++ b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts @@ -114,45 +114,6 @@ 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; @@ -169,23 +130,17 @@ describe('useConfirmController', () => { const onConfirm = vi.fn(() => pending.promise); const { result } = renderHook(() => useConfirmController({ onConfirm })); - await act(async () => expect(await result.current.confirm()).toBe(false)); + act(() => result.current.confirm()); expect(onConfirm).not.toHaveBeenCalled(); act(() => result.current.onOpenChange(true)); - const confirm = result.current.confirm; - let completion: ReturnType | undefined; - let duplicate: ReturnType | undefined; - act(() => { - completion = confirm(); - duplicate = confirm(); - }); + act(() => result.current.confirm()); + act(() => result.current.confirm()); expect(onConfirm).toHaveBeenCalledOnce(); - expect(await duplicate).toBe(false); await act(async () => { pending.resolve(); - expect(await completion).toBe(true); + await pending.promise; }); await waitFor(() => expect(result.current.isOpen).toBe(false)); }); diff --git a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx index 40205499858..9c8b094194e 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: () => void controller.confirm(payload), + confirm: () => 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 7fe2d539b5d..c0df19156fc 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -84,8 +84,8 @@ export interface ConfirmController { /** Opens the dialog about `payload` */ open: (payload: Payload) => void; onOpenChange: (open: boolean) => void; - /** Runs the action for `payload`. The dialog closes when it resolves and shows why when it rejects. Resolves whether the action succeeded. */ - confirm: (payload: Payload) => Promise; + /** Runs the action for `payload`. The dialog closes when it resolves and shows why when it rejects. */ + confirm: (payload: Payload) => void; } export interface ConfirmControllerOptions { @@ -140,21 +140,15 @@ export function useConfirmController({ send({ type: 'CANCEL' }); }, confirm: payload => { - let completion: Promise | undefined; + if (snapshot.value !== 'confirming') { + return; + } send({ type: 'CONFIRM', - run: () => { - const operation = (async () => { - await onConfirm(payload); - })(); - completion = operation.then( - () => true, - () => false, - ); - return operation; + run: async () => { + await onConfirm(payload); }, }); - 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 e0a051d3dcf..40b9b1f7df8 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 => void details.confirm(device) : undefined, + signOutFromDetails: revoke ? device => 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 af5666ef02e..f9c73803cb6 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 @@ -47,10 +47,10 @@ export function useUserProfileEmailSectionController({ canAdd: Boolean(onCreateEmail), verification, error: actions.error, - isDisabled: actions.isPending, + isDisabled: actions.isPending || remove.isPending, rowRef, registerTrigger: removalFocus.registerTrigger, - remove: onRemoveEmail ? actions.guard(remove) : undefined, + remove: onRemoveEmail ? 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.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.controller.ts index 3376aa8c0a5..c5a587ad606 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 @@ -49,10 +49,10 @@ export function useUserProfilePhoneSectionController({ canAdd: Boolean(onCreatePhone), verification, error: actions.error, - isDisabled: actions.isPending, + isDisabled: actions.isPending || remove.isPending, rowRef, registerTrigger: removalFocus.registerTrigger, - remove: onRemovePhone ? actions.guard(remove) : undefined, + remove: onRemovePhone ? 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-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 e5d7fff0b05..40ecbad645b 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 @@ -179,10 +179,10 @@ export function useUserProfileWeb3WalletsController({ !availableProviders.some(provider => provider.id === errorId) ? action.error : undefined, - isDisabled: action.isPending, + isDisabled: action.isPending || removeWallet.isPending, sectionRef, registerTrigger: removalFocus.registerTrigger, - remove: remove ? action.guard(removeWallet) : undefined, + remove: remove ? removeWallet : undefined, finalFocus: removalFocus.finalFocus, setPrimaryReverification, createReverification, 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 9179197a568..b2e39244861 100644 --- a/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx +++ b/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx @@ -5,24 +5,13 @@ 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()); @@ -275,125 +264,3 @@ describe('usePendingAction', () => { expect(result.current.errorKey).toBeUndefined(); }); }); - -describe('usePendingAction guard', () => { - it('refuses a guarded confirm while a run holds the lock, leaving the dialog unchanged', async () => { - const pending = deferred(); - const onConfirm = vi.fn(() => Promise.resolve()); - const { result } = renderHook(() => useGuardedConfirm(onConfirm)); - act(() => result.current.remove.onOpenChange(true)); - const owners = result.current; - - act(() => { - void owners.actions.run('row', () => pending.promise); - void owners.remove.confirm(); - }); - - expect(onConfirm).not.toHaveBeenCalled(); - expect(result.current.remove.isOpen).toBe(true); - expect(result.current.remove.isPending).toBe(false); - expect(result.current.actions.isPending).toBe(true); - await act(() => pending.resolve()); - expect(result.current.actions.isPending).toBe(false); - await act(() => result.current.remove.confirm()); - expect(onConfirm).toHaveBeenCalledOnce(); - expect(result.current.remove.isOpen).toBe(false); - }); - - it('refuses a run while a guarded confirm holds the lock, leaving its key and error unset', async () => { - const pending = deferred(); - const direct = vi.fn(); - const { result } = renderHook(() => useGuardedConfirm(() => pending.promise)); - act(() => result.current.remove.onOpenChange(true)); - const owners = result.current; - - await act(async () => { - void owners.remove.confirm(); - expect(await owners.actions.run('row', direct)).toBe(false); - }); - - expect(direct).not.toHaveBeenCalled(); - expect(result.current.actions.lastKey).toBeUndefined(); - expect(result.current.actions.error).toBeUndefined(); - expect(result.current.actions.isPending).toBe(true); - await act(() => pending.resolve()); - await act(async () => expect(await owners.actions.run('row', direct)).toBe(true)); - expect(direct).toHaveBeenCalledOnce(); - }); - - it('keeps the dialog error when a run refuses its retry', async () => { - const pending = deferred(); - const failure = new SaveError({ global: { code: 'action_blocked', message: 'Confirm refused.' } }); - const onConfirm = vi.fn(() => Promise.reject(failure)); - const { result } = renderHook(() => useGuardedConfirm(onConfirm)); - act(() => result.current.remove.onOpenChange(true)); - await act(() => result.current.remove.confirm()); - const error = result.current.remove.errorMessage; - expect(error).toBeDefined(); - expect(result.current.remove.isOpen).toBe(true); - expect(result.current.actions.error).toBeUndefined(); - - act(() => { - void result.current.actions.run('row', () => pending.promise); - void result.current.remove.confirm(); - }); - - expect(onConfirm).toHaveBeenCalledOnce(); - expect(result.current.remove.errorMessage).toBe(error); - expect(result.current.remove.isOpen).toBe(true); - expect(result.current.remove.isPending).toBe(false); - await act(() => pending.resolve()); - }); - - it('keeps the last key and clears the run error while a guarded confirm runs', async () => { - const pending = deferred(); - const failure = new SaveError({ global: { code: 'action_blocked', message: 'Run refused.' } }); - const { result } = renderHook(() => useGuardedConfirm(() => pending.promise)); - await act(() => result.current.actions.run('row', () => Promise.reject(failure))); - expect(result.current.actions.error).toBeDefined(); - - act(() => result.current.remove.onOpenChange(true)); - act(() => { - void result.current.remove.confirm(); - }); - - expect(result.current.actions.isPending).toBe(true); - expect(result.current.actions.pendingKey).toBeUndefined(); - expect(result.current.actions.lastKey).toBe('row'); - expect(result.current.actions.errorKey).toBeUndefined(); - expect(result.current.actions.error).toBeUndefined(); - await act(() => pending.resolve()); - expect(result.current.actions.isPending).toBe(false); - expect(result.current.actions.lastKey).toBe('row'); - }); - - it.each(['resolve', 'reject'])( - 'holds the lock after the dialog is cancelled until its action promises %s', - async outcome => { - const pending = deferred(); - const direct = vi.fn(); - const { result } = renderHook(() => useGuardedConfirm(() => pending.promise)); - act(() => result.current.remove.onOpenChange(true)); - act(() => { - void result.current.remove.confirm(); - }); - act(() => result.current.remove.onOpenChange(false)); - expect(result.current.remove.isOpen).toBe(false); - await act(async () => expect(await result.current.actions.run('row', direct)).toBe(false)); - expect(direct).not.toHaveBeenCalled(); - expect(result.current.actions.isPending).toBe(true); - expect(result.current.actions.pendingKey).toBeUndefined(); - - await act(() => { - if (outcome === 'reject') { - pending.reject(new Error('Late confirm failure')); - } else { - pending.resolve(); - } - }); - expect(result.current.actions.isPending).toBe(false); - expect(result.current.remove.errorMessage).toBeUndefined(); - await act(async () => expect(await result.current.actions.run('row', direct)).toBe(true)); - }, - ); -}); diff --git a/packages/mosaic/src/hooks/use-pending-action.ts b/packages/mosaic/src/hooks/use-pending-action.ts index e620d828510..bfeb88f3274 100644 --- a/packages/mosaic/src/hooks/use-pending-action.ts +++ b/packages/mosaic/src/hooks/use-pending-action.ts @@ -10,15 +10,8 @@ export interface PendingActionOptions { errorFallback?: string; } -export interface PendingConfirm { - confirm: (payload: TPayload) => Promise; -} - export interface PendingAction { run: (key: TKey, action: () => Promise | void, options?: PendingActionOptions) => Promise; - guard: >( - confirm: TConfirm & PendingConfirm, - ) => TConfirm; pendingKey: TKey | undefined; lastKey: TKey | undefined; isPending: boolean; @@ -33,64 +26,46 @@ interface PendingActionFailure { errorFallback: string | undefined; } -type Pending = { kind: 'inline'; key: TKey } | { kind: 'external' }; - /** - * 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. + * 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. */ export function usePendingAction({ errorFallback, }: PendingActionOptions = {}): PendingAction { const errorText = useErrorText(); - const running = useRef(false); - const [pending, setPending] = useState>(); + const [pendingKey, setPendingKey] = useState(); const [lastKey, setLastKey] = useState(); const [failure, setFailure] = useState>(); + const running = useRef(false); - const claim = (next: Pending, action: () => T | Promise): Promise | undefined => { + const run = async (key: TKey, action: () => Promise | void, options: PendingActionOptions = {}) => { if (running.current) { - return undefined; + return false; } running.current = true; - setPending(next); + setPendingKey(key); + setLastKey(key); setFailure(undefined); - return (async () => { - try { - return await action(); - } finally { - running.current = false; - setPending(undefined); + try { + await action(); + return true; + } catch (cause) { + if (!isReverificationCancelledError(cause)) { + setFailure({ key, error: toLocalizableError(cause), errorFallback: options.errorFallback }); } - })(); - }; - - const run = async (key: TKey, action: () => Promise | void, options: PendingActionOptions = {}) => { - return ( - claim({ kind: 'inline', key }, async () => { - setLastKey(key); - try { - await action(); - return true; - } catch (cause) { - if (!isReverificationCancelledError(cause)) { - setFailure({ key, error: toLocalizableError(cause), errorFallback: options.errorFallback }); - } - return false; - } - }) ?? false - ); + return false; + } finally { + running.current = false; + setPendingKey(undefined); + } }; return { run, - guard: confirm => ({ - ...confirm, - confirm: async payload => (await claim({ kind: 'external' }, () => confirm.confirm(payload))) ?? false, - }), - pendingKey: pending?.kind === 'inline' ? pending.key : undefined, + pendingKey, lastKey, - isPending: pending !== undefined, + isPending: pendingKey !== undefined, error: failure ? errorText(failure.error, failure.errorFallback ?? errorFallback) : undefined, errorKey: failure?.key, reset: () => setFailure(undefined), From 9c95423b5a49a3b726a8034665d493bf1977c0f0 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 3 Nov 2026 16:09:54 -0700 Subject: [PATCH 13/15] test(mosaic): cover a confirm run that outlives its cancelled dialog --- .../__tests__/confirm.controller.test.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) 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 7fc0e8d8704..0dea6f3b024 100644 --- a/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts +++ b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts @@ -170,4 +170,43 @@ describe('useConfirmController', () => { await waitFor(() => expect(result.current.isPending).toBe(false)); expect(result.current.isOpen).toBe(false); }); + it.each(['resolve', 'reject'] as const)( + 'stays pending after the dialog is cancelled until the run can %s, without a second run', + async outcome => { + const run = deferred(); + const onConfirm = vi.fn(() => run.promise); + const reverification = { status: 'loading' as const, onCancel: vi.fn(), visible: false }; + const { result } = renderHook(() => useConfirmController({ onConfirm, reverification })); + + act(() => result.current.onOpenChange(true)); + act(() => result.current.confirm()); + act(() => result.current.onOpenChange(false)); + + expect(reverification.onCancel).toHaveBeenCalledOnce(); + expect(result.current.isOpen).toBe(false); + expect(result.current.isPending).toBe(true); + + act(() => result.current.onOpenChange(true)); + act(() => result.current.confirm()); + + expect(onConfirm).toHaveBeenCalledOnce(); + expect(result.current.isOpen).toBe(false); + + await act(async () => { + if (outcome === 'resolve') { + run.resolve(); + } else { + run.reject(blocked); + } + await run.promise.catch(() => undefined); + }); + + expect(result.current.isPending).toBe(false); + expect(result.current.isOpen).toBe(false); + expect(result.current.errorMessage).toBeUndefined(); + + act(() => result.current.onOpenChange(true)); + expect(result.current.isOpen).toBe(true); + }, + ); }); From 6240b4d38a89c75a4ba4d310a19b971033aca3d9 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 3 Nov 2026 16:10:10 -0700 Subject: [PATCH 14/15] test(mosaic): cover identifier section locks around removal and set primary --- packages/mosaic/docs/controllers.md | 25 +++++++++++- ...ser-profile-email-section.feature.test.tsx | 39 +++++++++++++++++++ ...file-web3-wallets-section.feature.test.tsx | 3 ++ 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index d8ccf117e9d..20178a70609 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -37,12 +37,33 @@ The choice is invisible from outside: the controller returns plain props either const actions = usePendingAction({ errorFallback: messages.errors.generic }); return { - rows: rows.map(row => (actions.errorKey === row.id ? { ...row, error: actions.error } : row)), - pendingId: actions.pendingKey, + rows: rows.map(row => ({ + ...row, + isPending: actions.pendingKey === row.id, + error: actions.errorKey === row.id ? actions.error : undefined, + })), onRemove: id => void actions.run(id, () => onRemove(id)), }; ``` +When direct actions and a confirmation must not run together, keep each owner and combine their pending state in the section controller: + +```ts +const actions = usePendingAction(); +const remove = useConfirmController({ + onConfirm: contact => removalFocus.remove(contact.id), + reverification, +}); + +return { + contacts: contacts.map(contact => ({ ...contact, isPrimaryPending: actions.pendingKey === contact.id })), + isDisabled: actions.isPending || remove.isPending, + remove, +}; +``` + +Pass `isDisabled` to every control that starts one of the actions, including the item that opens the confirmation. `remove.isPending` stays true after the dialog closes during a reverification until the confirmed action settles, and the controller ignores `confirm` until then, so closing the dialog never lets a second action start. 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. A cancelled reverification resolves `false`, shows no error and releases the lock. - To hold pending through an OAuth redirect, await a delay inside the action (`user-profile-connected-accounts-section.controller.ts`). - It is not for a form (`useForm`), a confirm step (`useConfirmController`), an action whose result drives other state (a machine), or a check that reruns as the user types (`useDebouncedAsync`, or a field's `validateAsync`). 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 9f8954eb6ed..f9789877eae 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 @@ -102,6 +102,20 @@ describe('the user profile email section', () => { expect(screen.queryByRole('progressbar', { name: 'Set as primary' })).toBeNull(); }); + it('sets an address as primary once when Set as primary is double-clicked', async () => { + const other = fapiEmailAddress({ id: 'idn_other', email_address: 'bob@example.com', verification: VERIFIED }); + const { actor, fapi } = await renderSection(signedInWithEmails([PRIMARY, other])); + const primary = holdRequests('post', '/v1/me'); + + await openMenu(actor, within(emailRow()).getByRole('button', { name: 'Manage bob@example.com' })); + await actor.dblClick(screen.getByRole('menuitem', { name: 'Set as primary' })); + await waitFor(() => expect(primary.requests).toHaveLength(1)); + + primary.release(); + await waitFor(() => expect(fapi.client.sessions[0]?.user?.primary_email_address_id).toBe('idn_other')); + expect(primary.requests).toHaveLength(1); + }); + it('adds an address and verifies the code it was sent', async () => { const { actor } = await renderSection(signedInWithEmails([])); expect(emailRow()).toHaveTextContent('No email addresses added'); @@ -695,4 +709,29 @@ describe('Removing an address', () => { await waitFor(() => expect(emailsListed()).toEqual(['alice@example.com'])); expect(screen.queryByRole('alertdialog')).toBeNull(); }); + + it('locks the section while a removal runs, and unlocks it when the removal fails with the error in the dialog', async () => { + const second = fapiEmailAddress({ id: 'idn_second', email_address: 'bob@example.com', verification: VERIFIED }); + const { actor } = await renderSection(signedInWithEmails([PRIMARY, second])); + const removal = holdRequests('post', '/v1/me/email_addresses/idn_second', { _method: 'DELETE' }); + const manageAlice = () => screen.getByRole('button', { name: 'Manage alice@example.com', hidden: true }); + const addEmail = () => screen.getByRole('button', { name: 'Add email', hidden: true }); + + await manageEmail(actor, 'bob@example.com', 'Remove email'); + const dialog = await screen.findByRole('alertdialog', { name: 'Remove email address?' }); + await actor.click(within(dialog).getByRole('button', { name: 'Remove' })); + await waitFor(() => expect(removal.requests).toHaveLength(1)); + + expect(manageAlice()).toHaveAttribute('aria-disabled', 'true'); + expect(addEmail()).toBeDisabled(); + + removal.fail('last_identification', 'You cannot remove your last sign-in method.'); + await waitFor(() => + expect(within(dialog).getByRole('alert')).toHaveTextContent('You cannot remove your last sign-in method.'), + ); + expect(dialog).toBeVisible(); + expect(manageAlice()).not.toHaveAttribute('aria-disabled', 'true'); + expect(addEmail()).toBeEnabled(); + expect(emailsListed()).toEqual(['alice@example.com', 'bob@example.com']); + }); }); 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 b711cc2ec6a..36e043fd0dd 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 @@ -558,12 +558,15 @@ describe('Web3 wallets', () => { await user.click(confirm); await waitFor(() => expect(hold.requests).toHaveLength(1)); expect(confirm).toHaveAttribute('aria-busy', 'true'); + const manage = screen.getByRole('button', { name: 'Manage MetaMask', hidden: true }); + expect(manage).toHaveAttribute('aria-disabled', 'true'); hold.fail('wallet_removal_failed'); await waitFor(() => expect(within(screen.getByRole('alertdialog')).getByRole('alert')).toHaveTextContent('wallet_removal_failed'), ); await waitFor(() => expect(confirm).not.toHaveAttribute('aria-busy', 'true')); + expect(manage).not.toHaveAttribute('aria-disabled', 'true'); const retryFapi = serveFapi(fapi); await user.click(confirm); await waitFor(() => expect(retryFapi.client.sessions[0]?.user.web3_wallets).toHaveLength(0)); From 4ee222d42db47b619beb7c416bf538db8a04d0f3 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Fri, 6 Nov 2026 13:38:14 -0700 Subject: [PATCH 15/15] revert(mosaic): drop unrelated useListRemovalFocus change --- .../src/hooks/use-list-removal-focus.ts | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/packages/mosaic/src/hooks/use-list-removal-focus.ts b/packages/mosaic/src/hooks/use-list-removal-focus.ts index 38a4604ede1..73621b38222 100644 --- a/packages/mosaic/src/hooks/use-list-removal-focus.ts +++ b/packages/mosaic/src/hooks/use-list-removal-focus.ts @@ -1,4 +1,3 @@ -import type { RefCallback } from 'react'; import { useRef } from 'react'; export function useListRemovalFocus({ @@ -11,23 +10,14 @@ 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) => { - const existing = triggerRefs.current.get(id); - if (existing) { - return existing; + const registerTrigger = (id: string) => (element: HTMLButtonElement | null) => { + if (element) { + triggers.current.set(id, element); + } else { + triggers.current.delete(id); } - 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) => {