From b6d4280387d4c7cb6f8a0e6327c6fa84b784ce38 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 15:54:56 -0600 Subject: [PATCH 01/13] 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 4abe0f4d86b..9b8cdc70c20 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 a7486b6ab514fa7e381d433c194b29b716de6a18 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 16:14:05 -0600 Subject: [PATCH 02/13] fix(mosaic): coordinate identifier mutations across action owners --- .changeset/friendly-identifier-lock.md | 2 + packages/mosaic/AGENTS.md | 1 + packages/mosaic/docs/controllers.md | 31 ++++ .../src/blocks/confirm/confirm-shell.tsx | 2 + .../src/blocks/confirm/confirm.controller.ts | 22 ++- .../__tests__/confirmation.feature.test.tsx | 1 + .../src/blocks/confirmation/confirmation.tsx | 3 +- .../src/blocks/destructive/destructive.tsx | 3 +- .../user-profile-contact-list-row.view.tsx | 19 ++- ...ser-profile-email-section.feature.test.tsx | 4 +- .../user-profile-email-section.controller.ts | 7 +- .../user-profile-email-section.types.ts | 2 + .../user-profile-email-section.view.tsx | 5 + ...ser-profile-phone-section.feature.test.tsx | 4 +- .../user-profile-phone-section.controller.ts | 7 +- .../user-profile-phone-section.types.ts | 2 + .../user-profile-phone-section.view.tsx | 5 + .../user-profile-web3-wallet-row.view.tsx | 12 +- ...profile-web3-wallets-section.controller.ts | 8 +- ...user-profile-web3-wallets-section.types.ts | 2 + ...user-profile-web3-wallets-section.view.tsx | 7 +- .../hooks/__tests__/use-action-lock.test.tsx | 139 ++++++++++++++++++ packages/mosaic/src/hooks/use-action-lock.ts | 29 ++++ .../src/hooks/use-list-removal-focus.ts | 20 ++- .../mosaic/src/hooks/use-pending-action.ts | 51 ++++--- 25 files changed, 342 insertions(+), 46 deletions(-) create mode 100644 .changeset/friendly-identifier-lock.md create mode 100644 packages/mosaic/src/hooks/__tests__/use-action-lock.test.tsx create mode 100644 packages/mosaic/src/hooks/use-action-lock.ts diff --git a/.changeset/friendly-identifier-lock.md b/.changeset/friendly-identifier-lock.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/friendly-identifier-lock.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index a8689147e98..b1168785073 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -66,6 +66,7 @@ Most problems a feature hits already have a shared answer. Find the row for your | Deciding which part renders `Card.Root` | Only the surface, never a screen or `*Steps`, see [Views](docs/views.md) | | An interaction with an async lifecycle or two values that move together | A machine in the controller file, via `setup()` (`src/machine/`) | | Styles several components share | `src/styles/*.styles.ts` | +| Direct actions and confirmation dialogs that must exclude each other | `useActionLock` injected into `usePendingAction` and `useConfirmController` (`src/hooks/`, `src/blocks/confirm/`) | ## Signs you are about to invent a pattern diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index 94f05e37503..d7e403da9a4 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -51,6 +51,37 @@ return { }; ``` +When a direct action and a confirmation must share admission, create one +`useActionLock` (`src/hooks/use-action-lock.ts`) and inject it into both owners: + +```ts +const actionLock = useActionLock(); +const primary = usePendingAction({ actionLock }); +const remove = useConfirmController({ + actionLock, + onConfirm: contact => removalFocus.remove(contact.id), +}); +``` + +The lock admits synchronously, so two calls in one event cannot both start. +Its `tryRun` returns `undefined` without invoking busy work, or a promise that +preserves the accepted action's result or rejection. It stays held until the +actual action settles, even if a dialog closes and stops observing the action. +Opening a dialog does not acquire the lock. The lock owns no errors or row IDs. +`usePendingAction` keeps its inline errors and boolean result; refused runs +return `false` without changing metadata. `useConfirmController` keeps removal +errors in its dialog; refusal leaves the dialog and its error unchanged. +Its `canConfirm` projection disables only the confirmation submit control. +Admission still runs through the lock, not the rendered projection. Without an +injected lock, each owner retains its existing independent behavior. + +`actionLock` is a hook option, not a per-run option. Project its `isPending` to +the controls in the shared section. For contact primary changes, close the menu +on selection by leaving `ActionMenu.status` undefined, disable Add and keep +Manage disabled but focusable, and show a delayed row spinner beside the value. +Use one `useSpinDelay` outside the row map with `minDuration: 0`, so feedback ends +with the request. The Primary badge replaces the spinner after success. + `run` resolves `true` on success, so the caller can close a dialog or move focus. To hold pending through an OAuth redirect, await a delay inside the action (`user-profile-connected-accounts-section.controller.ts`). When the user diff --git a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx index 9c8b094194e..6c2aff082e6 100644 --- a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx +++ b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx @@ -34,6 +34,7 @@ export interface ConfirmStepProps { actionLabel: string; cancelLabel: string; isPending: boolean; + canConfirm: boolean; errorMessage: string | undefined; confirm: () => void; } @@ -68,6 +69,7 @@ function ConfirmCard({ actionLabel: resolve(actionLabel, payload), cancelLabel, isPending: controller.isPending, + canConfirm: controller.canConfirm, errorMessage: controller.errorMessage, confirm: () => controller.confirm(payload), }); diff --git a/packages/mosaic/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts index 6924f103b5b..18c70af44b1 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -4,6 +4,7 @@ import { useMemo } from 'react'; import type { DialogHandle } from '../../components/dialog'; import { Dialog } from '../../components/dialog'; import type { ReverificationController } from '../../features/reverification'; +import type { ActionLock } from '../../hooks/use-action-lock'; import type { ErrorDescription } from '../../localization'; import { useErrorText } from '../../localization'; import { setup } from '../../machine/setup'; @@ -73,6 +74,7 @@ export interface ConfirmController { isPending: boolean; /** Whether the dialog can close right now. False while the action is past the point of cancelling */ canClose: boolean; + canConfirm: boolean; errorMessage: string | undefined; /** Which card the dialog shows: the confirm step, or the reverification the action was interrupted by. */ step: ConfirmStep; @@ -85,6 +87,7 @@ export interface ConfirmController { } export interface ConfirmControllerOptions { + actionLock?: ActionLock; /** The action the dialog confirms. Resolve to close; reject to keep the dialog open showing why. */ onConfirm: (payload: Payload) => Promise | void; /** The flow `onConfirm` runs under, when the action may need step-up verification. */ @@ -107,6 +110,7 @@ export function useConfirmController({ onConfirm, reverification, errorFallback, + actionLock, }: ConfirmControllerOptions): ConfirmController { const handle = useMemo(() => Dialog.createHandle(), []); const [snapshot, send] = useMachine(confirmMachine); @@ -120,6 +124,7 @@ export function useConfirmController({ isOpen: snapshot.value !== 'idle', isPending, canClose, + canConfirm: snapshot.value === 'confirming' && !actionLock?.isPending, errorMessage: error ? errorText(error, errorFallback) : undefined, step: isPending && reverification?.visible ? 'verify' : 'confirm', reverification, @@ -139,12 +144,17 @@ export function useConfirmController({ if (snapshot.value !== 'confirming') { return; } - send({ - type: 'CONFIRM', - run: async () => { - await onConfirm(payload); - }, - }); + const run = async () => { + await onConfirm(payload); + }; + if (actionLock) { + const accepted = actionLock.tryRun(run); + if (accepted) { + send({ type: 'CONFIRM', run: () => accepted }); + } + return; + } + send({ type: 'CONFIRM', run }); }, }; } diff --git a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx index 2de465ee6ff..74d87837077 100644 --- a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx +++ b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx @@ -82,6 +82,7 @@ describe('Confirmation', () => { await user.click(confirmButton()); expect(onConfirm).toHaveBeenCalledOnce(); expect(confirmButton()).toHaveAttribute('aria-busy', 'true'); + expect(confirmButton()).toHaveFocus(); run.resolve(); await closed(); diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.tsx b/packages/mosaic/src/blocks/confirmation/confirmation.tsx index 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 31527a6de6a..0c9eb403593 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 9b8cdc70c20..30a40cb1151 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 9b0618176aa..6f4d9922ece 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx @@ -7,6 +7,7 @@ import { Badge } from '../../../components/badge'; import { SubmitButton } from '../../../components/button'; import { Icon } from '../../../components/icon'; import { Section } from '../../../components/section'; +import { Spinner } from '../../../components/spinner'; import { ArrowUpRight } from '../../../icons/glyphs/arrow-up-right'; import { fill, useMessages } from '../../../localization'; import { truncationStyles } from '../../../styles/typography.styles'; @@ -18,6 +19,7 @@ export function UserProfileWeb3WalletRowView({ wallet, triggerRef, isPending = false, + isPrimaryPending = false, isDisabled = false, onConnect, onVerify, @@ -27,6 +29,7 @@ export function UserProfileWeb3WalletRowView({ wallet: UserProfileWeb3Wallet | UserProfileWeb3Provider; triggerRef?: Ref; isPending?: boolean; + isPrimaryPending?: boolean; isDisabled?: boolean; onConnect?: (id: string) => void; onVerify?: (id: string) => void; @@ -68,7 +71,14 @@ export function UserProfileWeb3WalletRowView({ > {wallet.provider || shortAddress} - {linkedWallet?.isPrimary ? {m.primary} : null} + {linkedWallet?.isPrimary ? ( + {m.primary} + ) : isPrimaryPending ? ( + + ) : null} {linkedWallet && !linkedWallet.isVerified ? ( {isPending ? m.verifying : m.unverified} ) : null} diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts index 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 028f04fac54ba5889eaf6287c55502d630917be8 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 20:03:39 -0600 Subject: [PATCH 03/13] 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 30a40cb1151..cc25bd48df7 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]); @@ -221,7 +229,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'); }); @@ -520,6 +529,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 6f4d9922ece..2ef32381a34 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx @@ -7,7 +7,6 @@ import { Badge } from '../../../components/badge'; import { SubmitButton } from '../../../components/button'; import { Icon } from '../../../components/icon'; import { Section } from '../../../components/section'; -import { Spinner } from '../../../components/spinner'; import { ArrowUpRight } from '../../../icons/glyphs/arrow-up-right'; import { fill, useMessages } from '../../../localization'; import { truncationStyles } from '../../../styles/typography.styles'; @@ -46,7 +45,11 @@ export function UserProfileWeb3WalletRowView({ actions.push({ label: m.verify, onClick: () => onVerify(wallet.id) }); } if (linkedWallet && !linkedWallet.isPrimary && linkedWallet.isVerified && onSetPrimary) { - actions.push({ label: m.setPrimary, onClick: () => onSetPrimary(wallet.id) }); + actions.push({ + label: m.setPrimary, + onClick: () => onSetPrimary(wallet.id), + status: isPrimaryPending ? 'pending' : 'idle', + }); } if (linkedWallet && onRemove && linkedWallet.canRemove !== false) { actions.push({ label: m.remove, color: 'negative', onClick: () => onRemove(linkedWallet) }); @@ -71,14 +74,7 @@ export function UserProfileWeb3WalletRowView({ > {wallet.provider || shortAddress} - {linkedWallet?.isPrimary ? ( - {m.primary} - ) : isPrimaryPending ? ( - - ) : null} + {linkedWallet?.isPrimary ? {m.primary} : null} {linkedWallet && !linkedWallet.isVerified ? ( {isPending ? m.verifying : m.unverified} ) : null} diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts index 9da99dda1c5..965a41e7bfe 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts @@ -45,13 +45,11 @@ export function projectWeb3Wallets({ wallet.verification.strategy === 'admin' || WEB3_PROVIDERS.some(provider => provider.strategy === wallet.verification.strategy), ); - const primary = knownWallets.filter(wallet => wallet.id === primaryId); - const remaining = knownWallets.filter(wallet => wallet.id !== primaryId); - const verified = remaining.filter(wallet => wallet.verification.status === 'verified'); - const unverified = remaining.filter( + const verified = knownWallets.filter(wallet => wallet.verification.status === 'verified'); + const unverified = knownWallets.filter( wallet => wallet.verification.status !== null && wallet.verification.status !== 'verified', ); - const withoutStatus = remaining.filter(wallet => wallet.verification.status === null); + const withoutStatus = knownWallets.filter(wallet => wallet.verification.status === null); verified.sort((first, second) => first.id.localeCompare(second.id)); unverified.sort((first, second) => { @@ -60,7 +58,7 @@ export function projectWeb3Wallets({ return firstExpiry && secondExpiry ? firstExpiry.getTime() - secondExpiry.getTime() : 0; }); - const sorted = [...primary, ...verified, ...unverified, ...withoutStatus]; + const sorted = [...verified, ...unverified, ...withoutStatus]; const connectedStrategies = new Set(wallets.map(wallet => wallet.verification.strategy)); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.tsx index 38f77348d3a..9a9eb2cab3d 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.tsx @@ -35,6 +35,8 @@ function Web3Wallets({ model }: { model: ReadyWeb3WalletsModel }) { sectionRef={controller.sectionRef} availableProviders={controller.availableProviders} pendingId={controller.pendingId} + pendingPrimaryId={controller.pendingPrimaryId} + isDisabled={controller.isDisabled} error={controller.error} onConnect={id => { void controller.onConnect(id); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx index 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 612632ae8c4b4f20a060a6f76a2d024e59e9fcee Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 20:20:27 -0600 Subject: [PATCH 04/13] refactor(mosaic): pass confirmation disabled state directly --- packages/mosaic/docs/controllers.md | 4 ++-- .../src/blocks/confirm/confirm-shell.tsx | 2 -- .../src/blocks/confirm/confirm.controller.ts | 2 -- .../__tests__/confirmation.feature.test.tsx | 20 ++++++++++++++++++- .../src/blocks/confirmation/confirmation.tsx | 7 ++++--- .../src/blocks/destructive/destructive.tsx | 3 +-- .../user-profile-email-section.view.tsx | 1 + .../user-profile-phone-section.view.tsx | 1 + ...user-profile-web3-wallets-section.view.tsx | 1 + .../hooks/__tests__/use-action-lock.test.tsx | 4 ++-- 10 files changed, 31 insertions(+), 14 deletions(-) diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index d7e403da9a4..e9a7bbc61b7 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -71,8 +71,8 @@ Opening a dialog does not acquire the lock. The lock owns no errors or row IDs. `usePendingAction` keeps its inline errors and boolean result; refused runs return `false` without changing metadata. `useConfirmController` keeps removal errors in its dialog; refusal leaves the dialog and its error unchanged. -Its `canConfirm` projection disables only the confirmation submit control. -Admission still runs through the lock, not the rendered projection. Without an +Pass the section's pending state to `Confirmation.disabled` to disable its submit +control while another action runs. Admission still runs through the lock. Without an injected lock, each owner retains its existing independent behavior. `actionLock` is a hook option, not a per-run option. Project its `isPending` to diff --git a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx index 6c2aff082e6..9c8b094194e 100644 --- a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx +++ b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx @@ -34,7 +34,6 @@ export interface ConfirmStepProps { actionLabel: string; cancelLabel: string; isPending: boolean; - canConfirm: boolean; errorMessage: string | undefined; confirm: () => void; } @@ -69,7 +68,6 @@ function ConfirmCard({ actionLabel: resolve(actionLabel, payload), cancelLabel, isPending: controller.isPending, - canConfirm: controller.canConfirm, errorMessage: controller.errorMessage, confirm: () => controller.confirm(payload), }); diff --git a/packages/mosaic/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts index 18c70af44b1..faf04107a81 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -74,7 +74,6 @@ export interface ConfirmController { isPending: boolean; /** Whether the dialog can close right now. False while the action is past the point of cancelling */ canClose: boolean; - canConfirm: boolean; errorMessage: string | undefined; /** Which card the dialog shows: the confirm step, or the reverification the action was interrupted by. */ step: ConfirmStep; @@ -124,7 +123,6 @@ export function useConfirmController({ isOpen: snapshot.value !== 'idle', isPending, canClose, - canConfirm: snapshot.value === 'confirming' && !actionLock?.isPending, errorMessage: error ? errorText(error, errorFallback) : undefined, step: isPending && reverification?.visible ? 'verify' : 'confirm', reverification, diff --git a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx index 74d87837077..b256db420ed 100644 --- a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx +++ b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx @@ -38,7 +38,11 @@ function Host({ async function renderAccount( options: ConfirmControllerOptions, - { localization, description }: { localization?: MosaicLocalization; description?: ReactNode } = {}, + { + localization, + description, + disabled, + }: { localization?: MosaicLocalization; description?: ReactNode; disabled?: boolean } = {}, ) { const ref: { current: ConfirmController | undefined } = { current: undefined }; serveFapi({ client: fapiClient([session]) }); @@ -53,6 +57,7 @@ async function renderAccount( title={TITLE} description={description ?? 'Google will be removed from this account.'} actionLabel='Remove' + disabled={disabled} /> ); }} @@ -70,6 +75,19 @@ async function renderAccount( const confirmButton = () => screen.getByRole('button', { name: 'Remove' }); describe('Confirmation', () => { + it('disables confirmation while leaving Cancel usable', async () => { + const onConfirm = vi.fn(() => Promise.resolve()); + const { user, open, closed } = await renderAccount({ onConfirm }, { disabled: true }); + await open(); + + expect(confirmButton()).toBeDisabled(); + await user.click(confirmButton()); + expect(onConfirm).not.toHaveBeenCalled(); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + await closed(); + }); + it('opens from its trigger, confirms, closes once the action lands, and hands focus back', async () => { const run = deferred(); const onConfirm = vi.fn(() => run.promise); diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.tsx b/packages/mosaic/src/blocks/confirmation/confirmation.tsx index 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 0c9eb403593..31527a6de6a 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 325ce976361326fe76d8599f37bda41c66fc4805 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 27 Oct 2026 20:23:02 -0600 Subject: [PATCH 05/13] docs(mosaic): trim shared action lock guidance --- packages/mosaic/docs/controllers.md | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index e9a7bbc61b7..e14fbb322c6 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -51,7 +51,7 @@ return { }; ``` -When a direct action and a confirmation must share admission, create one +When a direct action and a confirmation must not run together, create one `useActionLock` (`src/hooks/use-action-lock.ts`) and inject it into both owners: ```ts @@ -63,24 +63,10 @@ const remove = useConfirmController({ }); ``` -The lock admits synchronously, so two calls in one event cannot both start. -Its `tryRun` returns `undefined` without invoking busy work, or a promise that -preserves the accepted action's result or rejection. It stays held until the -actual action settles, even if a dialog closes and stops observing the action. -Opening a dialog does not acquire the lock. The lock owns no errors or row IDs. -`usePendingAction` keeps its inline errors and boolean result; refused runs -return `false` without changing metadata. `useConfirmController` keeps removal -errors in its dialog; refusal leaves the dialog and its error unchanged. -Pass the section's pending state to `Confirmation.disabled` to disable its submit -control while another action runs. Admission still runs through the lock. Without an -injected lock, each owner retains its existing independent behavior. - -`actionLock` is a hook option, not a per-run option. Project its `isPending` to -the controls in the shared section. For contact primary changes, close the menu -on selection by leaving `ActionMenu.status` undefined, disable Add and keep -Manage disabled but focusable, and show a delayed row spinner beside the value. -Use one `useSpinDelay` outside the row map with `minDuration: 0`, so feedback ends -with the request. The Primary badge replaces the spinner after success. +Pass `actionLock.isPending` to the section controls' `disabled` props, including +`Confirmation.disabled`. The lock prevents overlapping requests even before React +renders the disabled controls, and stays held until the action settles. Each owner +keeps its existing error handling. `actionLock` is a hook option, not a per-run option. `run` resolves `true` on success, so the caller can close a dialog or move focus. To hold pending through an OAuth redirect, await a delay inside the From 4251133d0c006c62dd6b6090ecf37322e1da1707 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 11:24:36 -0600 Subject: [PATCH 06/13] refactor(mosaic): share pending ownership for identifier actions --- packages/mosaic/AGENTS.md | 2 +- packages/mosaic/docs/controllers.md | 20 +++-- .../src/blocks/confirm/confirm.controller.ts | 9 +- .../user-profile-email-section.controller.ts | 16 ++-- .../user-profile-phone-section.controller.ts | 16 ++-- ...profile-web3-wallets-section.controller.ts | 8 +- ...tsx => use-pending-action-shared.test.tsx} | 84 ++++++++++++------- packages/mosaic/src/hooks/use-action-lock.ts | 29 ------- .../mosaic/src/hooks/use-pending-action.ts | 49 ++++++----- 9 files changed, 117 insertions(+), 116 deletions(-) rename packages/mosaic/src/hooks/__tests__/{use-action-lock.test.tsx => use-pending-action-shared.test.tsx} (58%) delete mode 100644 packages/mosaic/src/hooks/use-action-lock.ts diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index b1168785073..7402da2dd64 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -66,7 +66,7 @@ Most problems a feature hits already have a shared answer. Find the row for your | Deciding which part renders `Card.Root` | Only the surface, never a screen or `*Steps`, see [Views](docs/views.md) | | An interaction with an async lifecycle or two values that move together | A machine in the controller file, via `setup()` (`src/machine/`) | | Styles several components share | `src/styles/*.styles.ts` | -| Direct actions and confirmation dialogs that must exclude each other | `useActionLock` injected into `usePendingAction` and `useConfirmController` (`src/hooks/`, `src/blocks/confirm/`) | +| Direct actions and confirmation dialogs that must exclude each other | One `usePendingAction` owner, with `runExternal` passed as `useConfirmController`'s `runAction` (`src/hooks/`, `src/blocks/confirm/`) | ## Signs you are about to invent a pattern diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index e14fbb322c6..f751424d660 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -51,22 +51,24 @@ return { }; ``` -When a direct action and a confirmation must not run together, create one -`useActionLock` (`src/hooks/use-action-lock.ts`) and inject it into both owners: +When a direct action and a confirmation must not run together, give both +operations one `usePendingAction` owner: ```ts -const actionLock = useActionLock(); -const primary = usePendingAction({ actionLock }); +const actions = usePendingAction(); const remove = useConfirmController({ - actionLock, + runAction: actions.runExternal, onConfirm: contact => removalFocus.remove(contact.id), }); ``` -Pass `actionLock.isPending` to the section controls' `disabled` props, including -`Confirmation.disabled`. The lock prevents overlapping requests even before React -renders the disabled controls, and stays held until the action settles. Each owner -keeps its existing error handling. `actionLock` is a hook option, not a per-run option. +Run direct actions with `actions.run(key, action)`. Pass `actions.isPending` to +the section controls' `disabled` props, including `Confirmation.disabled`. +`runExternal` returns the accepted promise, or `undefined` while another action +runs. It leaves row keys and inline errors unchanged and preserves rejections +for the confirmation dialog to handle. Both paths block overlapping calls before +React renders and stay pending until the accepted promise settles. Closing a +verification dialog does not release pending while cancellation is still settling. `run` resolves `true` on success, so the caller can close a dialog or move focus. To hold pending through an OAuth redirect, await a delay inside the diff --git a/packages/mosaic/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts index faf04107a81..4559f4d436c 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -4,7 +4,6 @@ import { useMemo } from 'react'; import type { DialogHandle } from '../../components/dialog'; import { Dialog } from '../../components/dialog'; import type { ReverificationController } from '../../features/reverification'; -import type { ActionLock } from '../../hooks/use-action-lock'; import type { ErrorDescription } from '../../localization'; import { useErrorText } from '../../localization'; import { setup } from '../../machine/setup'; @@ -86,7 +85,7 @@ export interface ConfirmController { } export interface ConfirmControllerOptions { - actionLock?: ActionLock; + runAction?: (action: () => Promise) => Promise | undefined; /** The action the dialog confirms. Resolve to close; reject to keep the dialog open showing why. */ onConfirm: (payload: Payload) => Promise | void; /** The flow `onConfirm` runs under, when the action may need step-up verification. */ @@ -109,7 +108,7 @@ export function useConfirmController({ onConfirm, reverification, errorFallback, - actionLock, + runAction, }: ConfirmControllerOptions): ConfirmController { const handle = useMemo(() => Dialog.createHandle(), []); const [snapshot, send] = useMachine(confirmMachine); @@ -145,8 +144,8 @@ export function useConfirmController({ const run = async () => { await onConfirm(payload); }; - if (actionLock) { - const accepted = actionLock.tryRun(run); + if (runAction) { + const accepted = runAction(run); if (accepted) { send({ type: 'CONFIRM', run: () => accepted }); } diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts index 6f0bd477d76..c6ba700f473 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts @@ -1,7 +1,6 @@ import { useRef } from 'react'; import { useConfirmController } from '../../../blocks/confirm'; -import { useActionLock } from '../../../hooks/use-action-lock'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; import type { UserProfileEmail } from '../user-profile-contact.types'; @@ -24,8 +23,7 @@ export function useUserProfileEmailSectionController({ onCreate: onCreateEmail, reverification: createReverification, }); - const actionLock = useActionLock(); - const setPrimary = usePendingAction({ actionLock }); + const actions = usePendingAction(); const rowRef = useRef(null); const removalFocus = useListRemovalFocus({ ids: emails.map(email => email.id), @@ -33,7 +31,7 @@ export function useUserProfileEmailSectionController({ fallback: () => rowRef.current?.querySelector('button:not([disabled])') ?? rowRef.current, }); const remove = useConfirmController({ - actionLock, + runAction: actions.runExternal, onConfirm: email => removalFocus.remove(email.id), reverification: removeReverification, }); @@ -42,22 +40,22 @@ export function useUserProfileEmailSectionController({ emails, canAdd: Boolean(onCreateEmail), verification, - error: setPrimary.error, - isDisabled: actionLock.isPending, - pendingPrimaryId: setPrimary.pendingKey, + error: actions.error, + isDisabled: actions.isPending, + pendingPrimaryId: actions.pendingKey, rowRef, registerTrigger: removalFocus.registerTrigger, remove: onRemoveEmail ? remove : undefined, finalFocus: removalFocus.finalFocus, verificationFinalFocus: () => (verification.verifyingId ? removalFocus.trigger(verification.verifyingId) : null), setPrimaryReverification, - setPrimaryFinalFocus: () => (setPrimary.lastKey ? removalFocus.trigger(setPrimary.lastKey) : null), + setPrimaryFinalFocus: () => (actions.lastKey ? removalFocus.trigger(actions.lastKey) : null), onVerify: id => { const email = emails.find(email => email.id === id); if (email) { verification.onVerifyEmail(email, getEmailVerifier(id)); } }, - onSetPrimary: id => void setPrimary.run(id, () => onSetPrimaryEmail(id)), + onSetPrimary: id => void actions.run(id, () => onSetPrimaryEmail(id)), }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.controller.ts index 08cf24cbc8b..d2faa559b9c 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.controller.ts @@ -2,7 +2,6 @@ import { stringToFormattedPhoneString } from '@clerk/shared/phone'; import { useRef } from 'react'; import { useConfirmController } from '../../../blocks/confirm'; -import { useActionLock } from '../../../hooks/use-action-lock'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; import type { UserProfilePhone } from '../user-profile-contact.types'; @@ -24,8 +23,7 @@ export function useUserProfilePhoneSectionController({ onCreate: onCreatePhone, reverification: createReverification, }); - const actionLock = useActionLock(); - const setPrimary = usePendingAction({ actionLock }); + const actions = usePendingAction(); const rowRef = useRef(null); const removalFocus = useListRemovalFocus({ ids: phones.map(phone => phone.id), @@ -33,7 +31,7 @@ export function useUserProfilePhoneSectionController({ fallback: () => rowRef.current?.querySelector('button:not([disabled])') ?? rowRef.current, }); const remove = useConfirmController({ - actionLock, + runAction: actions.runExternal, onConfirm: phone => removalFocus.remove(phone.id), reverification: removeReverification, }); @@ -43,22 +41,22 @@ export function useUserProfilePhoneSectionController({ defaultPhoneCountry, canAdd: Boolean(onCreatePhone), verification, - error: setPrimary.error, - isDisabled: actionLock.isPending, - pendingPrimaryId: setPrimary.pendingKey, + error: actions.error, + isDisabled: actions.isPending, + pendingPrimaryId: actions.pendingKey, rowRef, registerTrigger: removalFocus.registerTrigger, remove: onRemovePhone ? remove : undefined, finalFocus: removalFocus.finalFocus, verificationFinalFocus: () => (verification.verifyingId ? removalFocus.trigger(verification.verifyingId) : null), setPrimaryReverification, - setPrimaryFinalFocus: () => (setPrimary.lastKey ? removalFocus.trigger(setPrimary.lastKey) : null), + setPrimaryFinalFocus: () => (actions.lastKey ? removalFocus.trigger(actions.lastKey) : null), onVerify: id => { const phone = phones.find(phone => phone.id === id); if (phone) { verification.onVerifyPhone(phone, getPhoneVerifier(id)); } }, - onSetPrimary: id => void setPrimary.run(id, () => onSetPrimaryPhone(id)), + onSetPrimary: id => void actions.run(id, () => onSetPrimaryPhone(id)), }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts index 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 9a21c72e44422cb5e4a6e36548d639b4fe0d1fa2 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 11:54:20 -0600 Subject: [PATCH 07/13] refactor(mosaic): coordinate identifier actions in section controllers --- packages/mosaic/AGENTS.md | 2 +- packages/mosaic/docs/controllers.md | 31 +++++++---- .../__tests__/confirm.controller.test.ts | 53 +++++++++++++++++-- .../src/blocks/confirm/confirm-shell.tsx | 2 +- .../src/blocks/confirm/confirm.controller.ts | 35 ++++++------ ...ofile-active-devices-section.controller.ts | 2 +- .../user-profile-email-section.controller.ts | 8 ++- .../user-profile-phone-section.controller.ts | 8 ++- ...profile-web3-wallets-section.controller.ts | 9 +++- .../use-pending-action-shared.test.tsx | 23 +++++--- 10 files changed, 126 insertions(+), 47 deletions(-) diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index 7402da2dd64..463b9c9c748 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -66,7 +66,7 @@ Most problems a feature hits already have a shared answer. Find the row for your | Deciding which part renders `Card.Root` | Only the surface, never a screen or `*Steps`, see [Views](docs/views.md) | | An interaction with an async lifecycle or two values that move together | A machine in the controller file, via `setup()` (`src/machine/`) | | Styles several components share | `src/styles/*.styles.ts` | -| Direct actions and confirmation dialogs that must exclude each other | One `usePendingAction` owner, with `runExternal` passed as `useConfirmController`'s `runAction` (`src/hooks/`, `src/blocks/confirm/`) | +| Direct actions and confirmation dialogs that must exclude each other | Section controller wraps `confirm` with `usePendingAction.runExternal` (`src/hooks/`, `src/blocks/confirm/`) | ## Signs you are about to invent a pattern diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index f751424d660..3b3b2dfaf31 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -56,19 +56,32 @@ operations one `usePendingAction` owner: ```ts const actions = usePendingAction(); -const remove = useConfirmController({ - runAction: actions.runExternal, +const remove = useConfirmController({ onConfirm: contact => removalFocus.remove(contact.id), }); + +return { + isDisabled: actions.isPending, + pendingPrimaryId: actions.pendingKey, + remove: { + ...remove, + confirm: async contact => (await actions.runExternal(() => remove.confirm(contact))) ?? false, + }, +}; ``` -Run direct actions with `actions.run(key, action)`. Pass `actions.isPending` to -the section controls' `disabled` props, including `Confirmation.disabled`. -`runExternal` returns the accepted promise, or `undefined` while another action -runs. It leaves row keys and inline errors unchanged and preserves rejections -for the confirmation dialog to handle. Both paths block overlapping calls before -React renders and stay pending until the accepted promise settles. Closing a -verification dialog does not release pending while cancellation is still settling. +Keep this coordination in the section controller. Run direct actions with +`actions.run(key, action)` and wrap the returned confirmation's `confirm` callback +with `actions.runExternal`. Pass `actions.isPending` to the section controls' +`disabled` props, including `Confirmation.disabled`. + +`confirm` resolves `true` when the operation succeeds, or `false` when it fails or +the dialog ignores the call. Its promise follows the operation even if the +dialog closes during cancellation. The shared pending owner waits for that +promise, so closing the dialog does not enable another action before cancellation +settles. Removal keeps its dialog errors, and inline row errors and focus keys +stay unchanged. Views receive the pending and disabled props without coordinating +the operations. `run` resolves `true` on success, so the caller can close a dialog or move focus. To hold pending through an OAuth redirect, await a delay inside the diff --git a/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts index e6ec9447bfa..0d9d554bcb4 100644 --- a/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts +++ b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts @@ -91,6 +91,45 @@ describe('confirmMachine', () => { }); describe('useConfirmController', () => { + it.each(['resolve', 'reject'])( + 'keeps its completion pending after cancellation until the action can %s', + async outcome => { + const pending = deferred(); + const { result } = renderHook(() => + useConfirmController({ + onConfirm: () => pending.promise, + reverification: { status: 'loading', visible: true, onCancel: vi.fn() }, + }), + ); + const completed = vi.fn(); + act(() => result.current.onOpenChange(true)); + let completion: ReturnType | undefined; + act(() => { + completion = result.current.confirm(); + }); + expect(completion).toBeInstanceOf(Promise); + void completion?.then(completed); + act(() => result.current.onOpenChange(false)); + await act(async () => { + await Promise.resolve(); + }); + expect(result.current.isOpen).toBe(false); + expect(completed).not.toHaveBeenCalled(); + + await act(async () => { + if (outcome === 'reject') { + pending.reject(cancelled); + } else { + pending.resolve(); + } + await completion; + }); + + expect(completed).toHaveBeenCalledWith(outcome === 'resolve'); + expect(result.current.errorMessage).toBeUndefined(); + }, + ); + it('ignores open while no block is mounted on the controller, and keeps one handle across renders', () => { const { result, rerender } = renderHook(() => useConfirmController({ onConfirm: () => Promise.resolve() })); const { handle } = result.current; @@ -107,17 +146,23 @@ describe('useConfirmController', () => { const onConfirm = vi.fn(() => pending.promise); const { result } = renderHook(() => useConfirmController({ onConfirm })); - act(() => result.current.confirm()); + await act(async () => expect(await result.current.confirm()).toBe(false)); expect(onConfirm).not.toHaveBeenCalled(); act(() => result.current.onOpenChange(true)); - act(() => result.current.confirm()); - act(() => result.current.confirm()); + const confirm = result.current.confirm; + let completion: ReturnType | undefined; + let duplicate: ReturnType | undefined; + act(() => { + completion = confirm(); + duplicate = confirm(); + }); expect(onConfirm).toHaveBeenCalledOnce(); + expect(await duplicate).toBe(false); await act(async () => { pending.resolve(); - await pending.promise; + expect(await completion).toBe(true); }); await waitFor(() => expect(result.current.isOpen).toBe(false)); }); diff --git a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx index 9c8b094194e..40205499858 100644 --- a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx +++ b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx @@ -69,7 +69,7 @@ function ConfirmCard({ cancelLabel, isPending: controller.isPending, errorMessage: controller.errorMessage, - confirm: () => controller.confirm(payload), + confirm: () => void controller.confirm(payload), }); return ( diff --git a/packages/mosaic/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts index 4559f4d436c..bb11b314719 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -80,12 +80,11 @@ export interface ConfirmController { /** Opens the dialog about `payload` */ open: (payload: Payload) => void; onOpenChange: (open: boolean) => void; - /** Runs the action for `payload`. The dialog closes when it resolves and shows why when it rejects. */ - confirm: (payload: Payload) => void; + /** Runs the action for `payload`. The dialog closes when it resolves and shows why when it rejects. Resolves whether the action succeeded. */ + confirm: (payload: Payload) => Promise; } export interface ConfirmControllerOptions { - runAction?: (action: () => Promise) => Promise | undefined; /** The action the dialog confirms. Resolve to close; reject to keep the dialog open showing why. */ onConfirm: (payload: Payload) => Promise | void; /** The flow `onConfirm` runs under, when the action may need step-up verification. */ @@ -108,7 +107,6 @@ export function useConfirmController({ onConfirm, reverification, errorFallback, - runAction, }: ConfirmControllerOptions): ConfirmController { const handle = useMemo(() => Dialog.createHandle(), []); const [snapshot, send] = useMachine(confirmMachine); @@ -138,20 +136,21 @@ export function useConfirmController({ send({ type: 'CANCEL' }); }, confirm: payload => { - if (snapshot.value !== 'confirming') { - return; - } - const run = async () => { - await onConfirm(payload); - }; - if (runAction) { - const accepted = runAction(run); - if (accepted) { - send({ type: 'CONFIRM', run: () => accepted }); - } - return; - } - send({ type: 'CONFIRM', run }); + let completion: Promise | undefined; + send({ + type: 'CONFIRM', + run: () => { + const operation = (async () => { + await onConfirm(payload); + })(); + completion = operation.then( + () => true, + () => false, + ); + return operation; + }, + }); + return completion ?? Promise.resolve(false); }, }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts index 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 1b760aa583a48e2da3ee26fe5a1ef099e906c106 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 13:22:39 -0600 Subject: [PATCH 08/13] 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 cc25bd48df7..11c80d1c6dd 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]); @@ -229,8 +228,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 39651c032ab96491c0f2e0fa07561f499f52dd95 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 21:15:03 -0600 Subject: [PATCH 09/13] refactor(mosaic): let controllers own identifier row pending state usePendingAction gains guard(), which runs a confirmation's confirm under the section lock, replacing runExternal and the wrapper each section copied. Section controllers put verify and primary pending flags on the rows they return, so views no longer compare pending keys. Confirmation drops the disabled prop, which no reachable state could set. --- packages/mosaic/AGENTS.md | 2 +- packages/mosaic/docs/controllers.md | 24 +++++------ .../__tests__/confirmation.feature.test.tsx | 20 +--------- .../src/blocks/confirmation/confirmation.tsx | 4 +- .../user-profile-contact-list-row.view.tsx | 12 ++---- .../user-profile-contact.types.ts | 5 +++ .../user-profile-email-section.controller.ts | 14 +++---- .../user-profile-email-section.types.ts | 5 +-- .../user-profile-email-section.view.tsx | 4 -- .../user-profile-phone-section.controller.ts | 15 ++++--- .../user-profile-phone-section.types.ts | 5 +-- .../user-profile-phone-section.view.tsx | 4 -- ...e-web3-wallets-section.controller.test.tsx | 16 +++++--- .../user-profile-web3-wallet-row.view.tsx | 7 +--- ...profile-web3-wallets-section.controller.ts | 36 ++++++++--------- .../user-profile-web3-wallets-section.tsx | 2 - ...user-profile-web3-wallets-section.types.ts | 5 ++- ...user-profile-web3-wallets-section.view.tsx | 11 +---- .../use-pending-action-shared.test.tsx | 40 +------------------ .../mosaic/src/hooks/use-pending-action.ts | 15 +++++-- 20 files changed, 87 insertions(+), 159 deletions(-) diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index 463b9c9c748..0999d823e89 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -66,7 +66,7 @@ Most problems a feature hits already have a shared answer. Find the row for your | Deciding which part renders `Card.Root` | Only the surface, never a screen or `*Steps`, see [Views](docs/views.md) | | An interaction with an async lifecycle or two values that move together | A machine in the controller file, via `setup()` (`src/machine/`) | | Styles several components share | `src/styles/*.styles.ts` | -| Direct actions and confirmation dialogs that must exclude each other | Section controller wraps `confirm` with `usePendingAction.runExternal` (`src/hooks/`, `src/blocks/confirm/`) | +| Direct actions and confirmation dialogs that must exclude each other | `usePendingAction.guard` on the confirmation (`src/hooks/`, `src/blocks/confirm/`) | ## Signs you are about to invent a pattern diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index 3b3b2dfaf31..67686da6a11 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -45,8 +45,11 @@ It owns the error (see "Errors"): const actions = usePendingAction({ errorFallback: messages.errors.generic }); return { - rows: rows.map(row => (actions.errorKey === row.id ? { ...row, error: actions.error } : row)), - pendingId: actions.pendingKey, + rows: rows.map(row => ({ + ...row, + isPending: actions.pendingKey === row.id, + error: actions.errorKey === row.id ? actions.error : undefined, + })), onRemove: id => void actions.run(id, () => onRemove(id)), }; ``` @@ -61,27 +64,24 @@ const remove = useConfirmController({ }); return { + contacts: contacts.map(contact => ({ ...contact, isPrimaryPending: actions.pendingKey === contact.id })), isDisabled: actions.isPending, - pendingPrimaryId: actions.pendingKey, - remove: { - ...remove, - confirm: async contact => (await actions.runExternal(() => remove.confirm(contact))) ?? false, - }, + remove: actions.guard(remove), }; ``` Keep this coordination in the section controller. Run direct actions with -`actions.run(key, action)` and wrap the returned confirmation's `confirm` callback -with `actions.runExternal`. Pass `actions.isPending` to the section controls' -`disabled` props, including `Confirmation.disabled`. +`actions.run(key, action)` and pass the confirmation through `actions.guard`, which +runs its `confirm` under the same lock. Pass `actions.isPending` to the section +controls' `disabled` props. `confirm` resolves `true` when the operation succeeds, or `false` when it fails or the dialog ignores the call. Its promise follows the operation even if the dialog closes during cancellation. The shared pending owner waits for that promise, so closing the dialog does not enable another action before cancellation settles. Removal keeps its dialog errors, and inline row errors and focus keys -stay unchanged. Views receive the pending and disabled props without coordinating -the operations. +stay unchanged. Put each row's pending state on the row the controller returns, +so views render it without comparing pending keys. `run` resolves `true` on success, so the caller can close a dialog or move focus. To hold pending through an OAuth redirect, await a delay inside the diff --git a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx index b256db420ed..74d87837077 100644 --- a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx +++ b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx @@ -38,11 +38,7 @@ function Host({ async function renderAccount( options: ConfirmControllerOptions, - { - localization, - description, - disabled, - }: { localization?: MosaicLocalization; description?: ReactNode; disabled?: boolean } = {}, + { localization, description }: { localization?: MosaicLocalization; description?: ReactNode } = {}, ) { const ref: { current: ConfirmController | undefined } = { current: undefined }; serveFapi({ client: fapiClient([session]) }); @@ -57,7 +53,6 @@ async function renderAccount( title={TITLE} description={description ?? 'Google will be removed from this account.'} actionLabel='Remove' - disabled={disabled} /> ); }} @@ -75,19 +70,6 @@ async function renderAccount( const confirmButton = () => screen.getByRole('button', { name: 'Remove' }); describe('Confirmation', () => { - it('disables confirmation while leaving Cancel usable', async () => { - const onConfirm = vi.fn(() => Promise.resolve()); - const { user, open, closed } = await renderAccount({ onConfirm }, { disabled: true }); - await open(); - - expect(confirmButton()).toBeDisabled(); - await user.click(confirmButton()); - expect(onConfirm).not.toHaveBeenCalled(); - - await user.click(screen.getByRole('button', { name: 'Cancel' })); - await closed(); - }); - it('opens from its trigger, confirms, closes once the action lands, and hands focus back', async () => { const run = deferred(); const onConfirm = vi.fn(() => run.promise); diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.tsx b/packages/mosaic/src/blocks/confirmation/confirmation.tsx index 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 2ef32381a34..aa4ba4bc86b 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx @@ -17,8 +17,6 @@ import type { UserProfileWeb3Provider, UserProfileWeb3Wallet } from './user-prof export function UserProfileWeb3WalletRowView({ wallet, triggerRef, - isPending = false, - isPrimaryPending = false, isDisabled = false, onConnect, onVerify, @@ -27,8 +25,6 @@ export function UserProfileWeb3WalletRowView({ }: { wallet: UserProfileWeb3Wallet | UserProfileWeb3Provider; triggerRef?: Ref; - isPending?: boolean; - isPrimaryPending?: boolean; isDisabled?: boolean; onConnect?: (id: string) => void; onVerify?: (id: string) => void; @@ -37,6 +33,7 @@ export function UserProfileWeb3WalletRowView({ }) { const m = useMessages('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[] = []; @@ -48,7 +45,7 @@ export function UserProfileWeb3WalletRowView({ actions.push({ label: m.setPrimary, onClick: () => onSetPrimary(wallet.id), - status: isPrimaryPending ? 'pending' : 'idle', + status: linkedWallet.isPrimaryPending ? 'pending' : 'idle', }); } if (linkedWallet && onRemove && linkedWallet.canRemove !== false) { diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts index 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 bacf95e8a2815b9c0cf81d566197f5fc2ee93b7b Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 21:15:11 -0600 Subject: [PATCH 10/13] 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 11c80d1c6dd..a15f6e0b1b5 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 @@ -503,6 +503,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 12ed0d1ec43ac7d491e4838d9d4db7692d4f7d41 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 21:37:41 -0600 Subject: [PATCH 11/13] 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 a15f6e0b1b5..7e9f0c8fd0a 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(); }); @@ -228,8 +228,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 c724da6ef6feaf9aa04eceeee4e9c1e68e96182d Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 28 Oct 2026 21:48:13 -0600 Subject: [PATCH 12/13] 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 74bd0892f854dc12121b0ab8e6bee7d518807467 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Tue, 3 Nov 2026 14:37:26 -0700 Subject: [PATCH 13/13] style(mosaic): format contributor guidance --- packages/mosaic/AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index 0999d823e89..15d989539dd 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -66,7 +66,7 @@ Most problems a feature hits already have a shared answer. Find the row for your | Deciding which part renders `Card.Root` | Only the surface, never a screen or `*Steps`, see [Views](docs/views.md) | | An interaction with an async lifecycle or two values that move together | A machine in the controller file, via `setup()` (`src/machine/`) | | Styles several components share | `src/styles/*.styles.ts` | -| Direct actions and confirmation dialogs that must exclude each other | `usePendingAction.guard` on the confirmation (`src/hooks/`, `src/blocks/confirm/`) | +| Direct actions and confirmation dialogs that must exclude each other | `usePendingAction.guard` on the confirmation (`src/hooks/`, `src/blocks/confirm/`) | ## Signs you are about to invent a pattern