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 742f7698028..1cf9ce0a5a4 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -34,6 +34,7 @@ Most problems a feature hits already have a shared answer. Find the row for your | Problem | Use | | ------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------- | | An async action from a button or row: pending, an error, one lock | `usePendingAction` (`src/hooks/`) | +| Direct actions and confirmation dialogs that must exclude each other | `usePendingAction.guard` on the confirmation (`src/hooks/`, `src/blocks/confirm/`) | | A form: values, validation, submit, field errors | `useForm` (`src/components/form/`), with `validateAsync` for a server-side check | | A `
` element, without the browser's validation popup | `Form` (`src/primitives/form/`) | | A model save whose Clerk errors should land on form fields | `save` (`src/utils/errors.ts`) in the model, or reject with `FormSubmitError` | diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index d2aca72f306..c281f9bd2c9 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -45,12 +45,44 @@ It owns the error (see "Errors"): const actions = usePendingAction({ errorFallback: messages.errors.generic }); return { - rows: rows.map(row => (actions.errorKey === row.id ? { ...row, error: actions.error } : row)), - pendingId: actions.pendingKey, + rows: rows.map(row => ({ + ...row, + isPending: actions.pendingKey === row.id, + error: actions.errorKey === row.id ? actions.error : undefined, + })), onRemove: id => void actions.run(id, () => onRemove(id)), }; ``` +When a direct action and a confirmation must not run together, give both +operations one `usePendingAction` owner: + +```ts +const actions = usePendingAction(); +const remove = useConfirmController({ + onConfirm: contact => removalFocus.remove(contact.id), +}); + +return { + contacts: contacts.map(contact => ({ ...contact, isPrimaryPending: actions.pendingKey === contact.id })), + isDisabled: actions.isPending, + remove: actions.guard(remove), +}; +``` + +Keep this coordination in the section controller. Run direct actions with +`actions.run(key, action)` and pass the confirmation through `actions.guard`, which +runs its `confirm` under the same lock. Pass `actions.isPending` to the section +controls' `disabled` props. + +`confirm` resolves `true` when the operation succeeds, or `false` when it fails or +the dialog ignores the call. Its promise follows the operation even if the +dialog closes during cancellation. The shared pending owner waits for that +promise, so closing the dialog does not enable another action before cancellation +settles. Removal keeps its dialog errors, and inline row errors and focus keys +stay unchanged. Put each row's pending state on the row the controller returns, +so views render it without comparing pending keys. + `run` resolves `true` on success, so the caller can close a dialog or move focus. To hold pending through an OAuth redirect, await a delay inside the action (`user-profile-connected-accounts-section.controller.ts`). When the user diff --git a/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts index e6ec9447bfa..0d9d554bcb4 100644 --- a/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts +++ b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts @@ -91,6 +91,45 @@ describe('confirmMachine', () => { }); describe('useConfirmController', () => { + it.each(['resolve', 'reject'])( + 'keeps its completion pending after cancellation until the action can %s', + async outcome => { + const pending = deferred(); + const { result } = renderHook(() => + useConfirmController({ + onConfirm: () => pending.promise, + reverification: { status: 'loading', visible: true, onCancel: vi.fn() }, + }), + ); + const completed = vi.fn(); + act(() => result.current.onOpenChange(true)); + let completion: ReturnType | undefined; + act(() => { + completion = result.current.confirm(); + }); + expect(completion).toBeInstanceOf(Promise); + void completion?.then(completed); + act(() => result.current.onOpenChange(false)); + await act(async () => { + await Promise.resolve(); + }); + expect(result.current.isOpen).toBe(false); + expect(completed).not.toHaveBeenCalled(); + + await act(async () => { + if (outcome === 'reject') { + pending.reject(cancelled); + } else { + pending.resolve(); + } + await completion; + }); + + expect(completed).toHaveBeenCalledWith(outcome === 'resolve'); + expect(result.current.errorMessage).toBeUndefined(); + }, + ); + it('ignores open while no block is mounted on the controller, and keeps one handle across renders', () => { const { result, rerender } = renderHook(() => useConfirmController({ onConfirm: () => Promise.resolve() })); const { handle } = result.current; @@ -107,17 +146,23 @@ describe('useConfirmController', () => { const onConfirm = vi.fn(() => pending.promise); const { result } = renderHook(() => useConfirmController({ onConfirm })); - act(() => result.current.confirm()); + await act(async () => expect(await result.current.confirm()).toBe(false)); expect(onConfirm).not.toHaveBeenCalled(); act(() => result.current.onOpenChange(true)); - act(() => result.current.confirm()); - act(() => result.current.confirm()); + const confirm = result.current.confirm; + let completion: ReturnType | undefined; + let duplicate: ReturnType | undefined; + act(() => { + completion = confirm(); + duplicate = confirm(); + }); expect(onConfirm).toHaveBeenCalledOnce(); + expect(await duplicate).toBe(false); await act(async () => { pending.resolve(); - await pending.promise; + expect(await completion).toBe(true); }); await waitFor(() => expect(result.current.isOpen).toBe(false)); }); diff --git a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx index 9c8b094194e..40205499858 100644 --- a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx +++ b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx @@ -69,7 +69,7 @@ function ConfirmCard({ cancelLabel, isPending: controller.isPending, errorMessage: controller.errorMessage, - confirm: () => controller.confirm(payload), + confirm: () => void controller.confirm(payload), }); return ( diff --git a/packages/mosaic/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts index 6924f103b5b..bb11b314719 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -80,8 +80,8 @@ export interface ConfirmController { /** Opens the dialog about `payload` */ open: (payload: Payload) => void; onOpenChange: (open: boolean) => void; - /** Runs the action for `payload`. The dialog closes when it resolves and shows why when it rejects. */ - confirm: (payload: Payload) => void; + /** Runs the action for `payload`. The dialog closes when it resolves and shows why when it rejects. Resolves whether the action succeeded. */ + confirm: (payload: Payload) => Promise; } export interface ConfirmControllerOptions { @@ -136,15 +136,21 @@ export function useConfirmController({ send({ type: 'CANCEL' }); }, confirm: payload => { - if (snapshot.value !== 'confirming') { - return; - } + let completion: Promise | undefined; send({ type: 'CONFIRM', - run: async () => { - await onConfirm(payload); + run: () => { + const operation = (async () => { + await onConfirm(payload); + })(); + completion = operation.then( + () => true, + () => false, + ); + return operation; }, }); + return completion ?? Promise.resolve(false); }, }; } diff --git a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx index 2de465ee6ff..74d87837077 100644 --- a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx +++ b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx @@ -82,6 +82,7 @@ describe('Confirmation', () => { await user.click(confirmButton()); expect(onConfirm).toHaveBeenCalledOnce(); expect(confirmButton()).toHaveAttribute('aria-busy', 'true'); + expect(confirmButton()).toHaveFocus(); run.resolve(); await closed(); diff --git a/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts index 3bfcff4a8ec..d0f5a674ee6 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts @@ -84,7 +84,7 @@ export function useUserProfileActiveDevicesController({ registerTrigger: removalFocus.registerTrigger, finalFocus: removalFocus.finalFocus, details, - signOutFromDetails: revoke ? device => details.confirm(device) : undefined, + signOutFromDetails: revoke ? device => void details.confirm(device) : undefined, signOut: revoke ? signOut : undefined, signOutAll: revokeAll ? signOutAll : undefined, // Confirming removes the other devices and the trigger with them — hence the dialog mounted diff --git a/packages/mosaic/src/features/user-profile/user-profile-contact-list-row.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-contact-list-row.view.tsx index 46e05dfc421..fd816f50fcf 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-contact-list-row.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-contact-list-row.view.tsx @@ -10,7 +10,7 @@ import { Section } from '../../components/section'; import { Plus } from '../../icons/glyphs/plus'; import { fill, useMessages } from '../../localization'; import { truncationStyles } from '../../styles/typography.styles'; -import type { UserProfileContact } from './user-profile-contact.types'; +import type { UserProfileContact, UserProfileContactRow } from './user-profile-contact.types'; import { styles } from './user-profile-profile-panel.styles'; export interface UserProfileContactListRowViewProps { @@ -19,10 +19,10 @@ export interface UserProfileContactListRowViewProps { addAction?: ReactNode; kind: 'email' | 'phone'; label: string; - items: UserProfileContact[]; + items: UserProfileContactRow[]; onAdd?: () => void; onVerify?: (id: string) => void; - pendingVerifyId?: string; + isDisabled?: boolean; onSetPrimary?: (id: string) => void; onRemove?: (item: UserProfileContact) => void; children?: ReactNode; @@ -34,7 +34,7 @@ export function UserProfileContactListRowView({ items, onAdd, onVerify, - pendingVerifyId, + isDisabled = false, onSetPrimary, onRemove, addAction, @@ -64,6 +64,7 @@ export function UserProfileContactListRowView({ size='sm' variant='outline' onClick={onAdd} + disabled={isDisabled} > onVerify(item.id), - status: pendingVerifyId === item.id ? 'pending' : 'idle', + status: item.isVerifyPending ? 'pending' : 'idle', }); } else if (item.isVerified && !item.isDefault && onSetPrimary) { - actions.push({ label: m.setPrimary, onClick: () => onSetPrimary(item.id) }); + actions.push({ + label: m.setPrimary, + onClick: () => onSetPrimary(item.id), + status: item.isPrimaryPending ? 'pending' : 'idle', + }); } if (onRemove) { @@ -119,6 +124,8 @@ export function UserProfileContactListRowView({ diff --git a/packages/mosaic/src/features/user-profile/user-profile-contact.types.ts b/packages/mosaic/src/features/user-profile/user-profile-contact.types.ts index 1f178b0f450..80126c87925 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-contact.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-contact.types.ts @@ -5,6 +5,11 @@ export interface UserProfileContact { isVerified: boolean; } +export interface UserProfileContactRow extends UserProfileContact { + isVerifyPending: boolean; + isPrimaryPending: boolean; +} + export type UserProfileEmail = UserProfileContact; export type UserProfilePhone = UserProfileContact; diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx index d28b064404a..c8efedae675 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx @@ -68,6 +68,36 @@ async function enterCode(actor: Actor, code: string) { } describe('the user profile email section', () => { + it('holds the menu and row order while setting an address as primary, then returns focus', async () => { + const other = fapiEmailAddress({ id: 'idn_other', email_address: 'bob@example.com', verification: VERIFIED }); + const { actor } = await renderSection(signedInWithEmails([PRIMARY, other])); + const primary = holdRequests('post', '/v1/me'); + try { + await manageEmail(actor, 'bob@example.com', 'Set as primary'); + await waitFor(() => expect(primary.requests).toHaveLength(1)); + const menu = screen.getByRole('menu'); + const item = within(menu).getByRole('menuitem', { name: 'Set as primary' }); + expect(item).toHaveAttribute('aria-busy', 'true'); + const manage = screen.getByRole('button', { name: 'Manage bob@example.com' }); + expect(manage).toHaveAttribute('aria-disabled', 'true'); + expect(within(menu).getByRole('menuitem', { name: 'Remove email' })).toHaveAttribute('aria-disabled', 'true'); + expect(screen.queryByRole('alertdialog')).toBeNull(); + expect(screen.getByRole('button', { name: 'Add email' })).toBeDisabled(); + expect(await within(item).findByRole('progressbar', { name: 'Set as primary' })).toBeVisible(); + expect(emailsListed()).toEqual(['alice@example.com', 'bob@example.com']); + } finally { + primary.release(); + await waitFor(() => + expect(within(emailRow()).getByText('bob@example.com').parentElement).toHaveTextContent('Primary'), + ); + } + expect(screen.getByRole('button', { name: 'Manage bob@example.com' })).not.toHaveAttribute('aria-disabled', 'true'); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + expect(emailsListed()).toEqual(['alice@example.com', 'bob@example.com']); + expect(screen.getByRole('button', { name: 'Manage bob@example.com' })).toHaveFocus(); + expect(screen.queryByRole('progressbar', { name: 'Set as primary' })).toBeNull(); + }); + it('adds an address and verifies the code it was sent', async () => { const { actor } = await renderSection(signedInWithEmails([])); expect(emailRow()).toHaveTextContent('No email addresses added'); @@ -314,6 +344,7 @@ describe('email reverification', () => { await manageEmail(actor, 'bob@example.com', 'Set as primary'); expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); + await waitFor(() => expect(screen.queryByRole('menu', { hidden: true })).toBeNull()); await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus()); await actor.keyboard('hunter2{Enter}'); @@ -340,6 +371,7 @@ describe('email reverification', () => { expect(within(emailRow()).queryByRole('alert')).toBeNull(); expect(within(emailRow()).getByRole('button', { name: 'Manage bob@example.com' })).toHaveFocus(); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); await manageEmail(actor, 'bob@example.com', 'Set as primary'); await waitFor(() => expect(fapi.client.sessions[0]?.user?.primary_email_address_id).toBe('idn_second')); }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts index 822bdeca9b9..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,12 +19,14 @@ export function useUserProfileEmailSectionController({ setPrimaryReverification, createReverification, }: Omit): UserProfileEmailSectionViewProps { + const [order] = useState(() => modelEmails.map(email => email.id)); + const emails = applyOrder(order, modelEmails, email => email.id); const verification = useUserProfileAddEmailController({ username, onCreate: onCreateEmail, reverification: createReverification, }); - const setPrimary = usePendingAction(); + const actions = usePendingAction(); const rowRef = useRef(null); const removalFocus = useListRemovalFocus({ ids: emails.map(email => email.id), @@ -36,23 +39,28 @@ export function useUserProfileEmailSectionController({ }); return { - emails, + emails: emails.map(email => ({ + ...email, + isVerifyPending: verification.pendingVerifyId === email.id, + isPrimaryPending: actions.pendingKey === email.id && !setPrimaryReverification.visible, + })), canAdd: Boolean(onCreateEmail), verification, - error: setPrimary.error, + error: actions.error, + isDisabled: actions.isPending, rowRef, registerTrigger: removalFocus.registerTrigger, - remove: onRemoveEmail ? remove : undefined, + remove: onRemoveEmail ? actions.guard(remove) : undefined, finalFocus: removalFocus.finalFocus, verificationFinalFocus: () => (verification.verifyingId ? removalFocus.trigger(verification.verifyingId) : null), setPrimaryReverification, - setPrimaryFinalFocus: () => (setPrimary.lastKey ? removalFocus.trigger(setPrimary.lastKey) : null), + setPrimaryFinalFocus: () => (actions.lastKey ? removalFocus.trigger(actions.lastKey) : null), onVerify: id => { const email = emails.find(email => email.id === id); if (email) { verification.onVerifyEmail(email, getEmailVerifier(id)); } }, - onSetPrimary: id => void setPrimary.run(id, () => onSetPrimaryEmail(id)), + onSetPrimary: id => void actions.run(id, () => onSetPrimaryEmail(id)), }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.types.ts index 1b4985b0644..222554f844b 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.types.ts @@ -3,12 +3,13 @@ import type { Ref } from 'react'; import type { ConfirmController } from '../../../blocks/confirm'; import type { DialogFocusTarget } from '../../../components/dialog'; import type { ReverificationController } from '../../reverification'; -import type { UserProfileEmail, UserProfileEmailVerifier } from '../user-profile-contact.types'; +import type { UserProfileContactRow, UserProfileEmail, UserProfileEmailVerifier } from '../user-profile-contact.types'; import type { UserProfileAddEmailController } from './user-profile-add-email.controller'; export interface UserProfileEmailSectionViewProps { - emails: UserProfileEmail[]; + emails: UserProfileContactRow[]; canAdd: boolean; + isDisabled: boolean; verification: UserProfileAddEmailController; error: string | undefined; rowRef: Ref; diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx index 2e3de1fd1ce..2f1e1404947 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx @@ -13,6 +13,7 @@ import type { UserProfileEmailSectionViewProps } from './user-profile-email-sect export function UserProfileEmailSectionView({ emails, canAdd, + isDisabled, verification, error, rowRef, @@ -35,6 +36,7 @@ export function UserProfileEmailSectionView({ color='neutral' size='sm' variant='outline' + disabled={isDisabled} /> } > @@ -59,7 +61,7 @@ export function UserProfileEmailSectionView({ onRemove={remove ? email => remove.open(email) : undefined} onSetPrimary={onSetPrimary} onVerify={onVerify} - pendingVerifyId={verification.pendingVerifyId} + isDisabled={isDisabled} > {error} diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx index b796c5bf17d..4b5052b7a63 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx @@ -92,6 +92,35 @@ async function enterCode(actor: Actor, code: string) { } describe('the user profile phone section', () => { + it('holds the menu and row order while setting a number as primary', async () => { + const { actor } = await renderSection(signedIn([HOME, WORK])); + const primary = holdRequests('post', '/v1/me'); + try { + await manage(actor, WORK_LABEL, 'Set as primary'); + await waitFor(() => expect(primary.requests).toHaveLength(1)); + const menu = screen.getByRole('menu'); + const item = within(menu).getByRole('menuitem', { name: 'Set as primary' }); + expect(item).toHaveAttribute('aria-busy', 'true'); + const manageWork = screen.getByRole('button', { name: `Manage ${WORK_LABEL}` }); + expect(manageWork).toHaveAttribute('aria-disabled', 'true'); + expect(within(menu).getByRole('menuitem', { name: 'Remove phone number' })).toHaveAttribute( + 'aria-disabled', + 'true', + ); + expect(screen.queryByRole('alertdialog')).toBeNull(); + expect(screen.getByRole('button', { name: 'Add phone number' })).toBeDisabled(); + expect(await within(item).findByRole('progressbar', { name: 'Set as primary' })).toBeVisible(); + expect(listed()).toEqual([HOME_LABEL, WORK_LABEL]); + } finally { + primary.release(); + await waitFor(() => expect(within(row()).getByText(WORK_LABEL).parentElement).toHaveTextContent('Primary')); + } + expect(screen.getByRole('button', { name: `Manage ${WORK_LABEL}` })).not.toHaveAttribute('aria-disabled', 'true'); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + expect(listed()).toEqual([HOME_LABEL, WORK_LABEL]); + expect(screen.queryByRole('progressbar', { name: 'Set as primary' })).toBeNull(); + }); + it('lists the primary number first and marks it', async () => { await renderSection(signedIn([HOME, WORK])); @@ -181,8 +210,8 @@ describe('the user profile phone section', () => { await manage(actor, WORK_LABEL, 'Set as primary'); - await waitFor(() => expect(listed()).toEqual([WORK_LABEL, HOME_LABEL])); - expect(within(row()).getByText(WORK_LABEL).parentElement).toHaveTextContent('Primary'); + await waitFor(() => expect(within(row()).getByText(WORK_LABEL).parentElement).toHaveTextContent('Primary')); + expect(listed()).toEqual([HOME_LABEL, WORK_LABEL]); }); it('removes a number once the removal is confirmed', async () => { @@ -456,6 +485,7 @@ describe('phone reverification', () => { await manage(actor, WORK_LABEL, 'Set as primary'); expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); + await waitFor(() => expect(screen.queryByRole('menu', { hidden: true })).toBeNull()); await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus()); await actor.keyboard('hunter2{Enter}'); @@ -480,6 +510,7 @@ describe('phone reverification', () => { expect(within(row()).queryByRole('alert')).toBeNull(); expect(within(row()).getByRole('button', { name: `Manage ${WORK_LABEL}` })).toHaveFocus(); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); await manage(actor, WORK_LABEL, 'Set as primary'); await waitFor(() => expect(fapi.client.sessions[0]?.user?.primary_phone_number_id).toBe('idn_work')); }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.controller.ts index 9419a425010..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,11 +20,13 @@ export function useUserProfilePhoneSectionController({ setPrimaryReverification, createReverification, }: Omit): UserProfilePhoneSectionViewProps { + const [order] = useState(() => modelPhones.map(phone => phone.id)); + const phones = applyOrder(order, modelPhones, phone => phone.id); const verification = useUserProfileAddPhoneController({ onCreate: onCreatePhone, reverification: createReverification, }); - const setPrimary = usePendingAction(); + const actions = usePendingAction(); const rowRef = useRef(null); const removalFocus = useListRemovalFocus({ ids: phones.map(phone => phone.id), @@ -36,24 +39,30 @@ export function useUserProfilePhoneSectionController({ }); return { - phones: phones.map(phone => ({ ...phone, value: stringToFormattedPhoneString(phone.value) })), + phones: phones.map(phone => ({ + ...phone, + value: stringToFormattedPhoneString(phone.value), + isVerifyPending: verification.pendingVerifyId === phone.id, + isPrimaryPending: actions.pendingKey === phone.id && !setPrimaryReverification.visible, + })), defaultPhoneCountry, canAdd: Boolean(onCreatePhone), verification, - error: setPrimary.error, + error: actions.error, + isDisabled: actions.isPending, rowRef, registerTrigger: removalFocus.registerTrigger, - remove: onRemovePhone ? remove : undefined, + remove: onRemovePhone ? actions.guard(remove) : undefined, finalFocus: removalFocus.finalFocus, verificationFinalFocus: () => (verification.verifyingId ? removalFocus.trigger(verification.verifyingId) : null), setPrimaryReverification, - setPrimaryFinalFocus: () => (setPrimary.lastKey ? removalFocus.trigger(setPrimary.lastKey) : null), + setPrimaryFinalFocus: () => (actions.lastKey ? removalFocus.trigger(actions.lastKey) : null), onVerify: id => { const phone = phones.find(phone => phone.id === id); if (phone) { verification.onVerifyPhone(phone, getPhoneVerifier(id)); } }, - onSetPrimary: id => void setPrimary.run(id, () => onSetPrimaryPhone(id)), + onSetPrimary: id => void actions.run(id, () => onSetPrimaryPhone(id)), }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.types.ts index b9a659e3f4c..a15b5130d1e 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.types.ts @@ -4,13 +4,14 @@ import type { ConfirmController } from '../../../blocks/confirm'; import type { DialogFocusTarget } from '../../../components/dialog'; import type { CountryIso } from '../../../components/phone-input'; import type { ReverificationController } from '../../reverification'; -import type { UserProfilePhone, UserProfilePhoneVerifier } from '../user-profile-contact.types'; +import type { UserProfileContactRow, UserProfilePhone, UserProfilePhoneVerifier } from '../user-profile-contact.types'; import type { UserProfileAddPhoneController } from './user-profile-add-phone.controller'; export interface UserProfilePhoneSectionViewProps { - phones: UserProfilePhone[]; + phones: UserProfileContactRow[]; defaultPhoneCountry?: CountryIso; canAdd: boolean; + isDisabled: boolean; verification: UserProfileAddPhoneController; error: string | undefined; rowRef: Ref; diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx index ffcfe7bf5dc..e3e8d6963b6 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx @@ -14,6 +14,7 @@ export function UserProfilePhoneSectionView({ phones, defaultPhoneCountry, canAdd, + isDisabled, verification, error, rowRef, @@ -36,6 +37,7 @@ export function UserProfilePhoneSectionView({ color='neutral' size='sm' variant='outline' + disabled={isDisabled} /> } > @@ -60,7 +62,7 @@ export function UserProfilePhoneSectionView({ onRemove={remove ? phone => remove.open(phone) : undefined} onSetPrimary={onSetPrimary} onVerify={onVerify} - pendingVerifyId={verification.pendingVerifyId} + isDisabled={isDisabled} > {error} diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.controller.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.controller.test.tsx index 70fd2af2da2..01c5924a71d 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.controller.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.controller.test.tsx @@ -2,6 +2,7 @@ import { act, renderHook } from '@testing-library/react'; import { describe, expect, it, vi } from 'vitest'; import { deferred } from '../../../../__tests__/async'; +import type { ReverificationController } from '../../../reverification'; import { useUserProfileWeb3WalletsController } from '../user-profile-web3-wallets-section.controller'; import type { ReadyWeb3WalletsModel, @@ -14,6 +15,12 @@ const providers = [ { id: 'web3_solana_signature', provider: 'Solana', walletPicker: 'solana' }, ] satisfies UserProfileWeb3Provider[]; +const idleReverification = { status: 'idle', visible: false } satisfies ReverificationController; + +function pendingRowId(controller: ReturnType) { + return [...controller.wallets, ...controller.availableProviders].find(row => row.isPending)?.id; +} + describe('Web3 connection controller', () => { it('does not move an address-phase connection to a different provider with the same address', async () => { const pending = deferred(); @@ -42,7 +49,7 @@ describe('Web3 connection controller', () => { act(() => { void result.current.onConnect('web3_metamask_signature'); }); - expect(result.current.pendingId).toBe('web3_metamask_signature'); + expect(pendingRowId(result.current)).toBe('web3_metamask_signature'); await act(async () => { pending.reject(new Error('Rejected')); await pending.promise.catch(() => undefined); @@ -69,6 +76,7 @@ describe('Web3 connection controller', () => { verify: vi.fn(), setPrimary, remove: vi.fn(), + setPrimaryReverification: idleReverification, }), ); @@ -79,7 +87,7 @@ describe('Web3 connection controller', () => { act(() => { void result.current.onSetPrimary('other'); }); - expect(result.current.pendingId).toBe('other'); + expect(pendingRowId(result.current)).toBe('other'); await act(async () => { primary.reject(new Error('Primary rejected')); await primary.promise.catch(() => undefined); @@ -137,7 +145,7 @@ describe('Web3 connection controller', () => { act(() => { void result.current.onConnect('web3_metamask_signature'); }); - expect(result.current.pendingId).toBe('web3_metamask_signature'); + expect(pendingRowId(result.current)).toBe('web3_metamask_signature'); await act(async () => { pending.resolve(); await pending.promise; @@ -172,7 +180,7 @@ describe('Web3 connection controller', () => { void result.current.onSetPrimary('other'); void result.current.onConnect('web3_metamask_signature'); }); - expect(result.current.pendingId).toBe('target'); + expect(pendingRowId(result.current)).toBe('target'); expect(setPrimary).not.toHaveBeenCalled(); await act(async () => { pending.reject(new Error('Retry rejected')); @@ -229,7 +237,7 @@ describe('Web3 connection controller', () => { address: provider.walletPicker === 'solana' ? address : address.toLowerCase(), }); }); - expect(result.current.pendingId).toBe('target'); + expect(pendingRowId(result.current)).toBe('target'); act(() => reportTarget?.({ kind: 'wallet', id: 'target' })); @@ -237,7 +245,7 @@ describe('Web3 connection controller', () => { signature.reject(new Error('Signature rejected')); await signature.promise.catch(() => undefined); }); - expect(result.current.pendingId).toBeUndefined(); + expect(pendingRowId(result.current)).toBeUndefined(); expect(result.current.wallets[1]?.verifyError).toBe('Something went wrong. Please try again.'); expect(result.current.wallets[0]?.verifyError).toBeUndefined(); }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx index cb9fc8857d2..ac57d6c1b52 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx @@ -464,7 +464,7 @@ describe('Web3 wallets', () => { await waitFor(() => expect(screen.getByText('Primary')).toBeInTheDocument()); }); - it('sets a verified wallet as primary through Clerk', async () => { + it('holds the wallet menu while setting primary and updates the badge in place', async () => { const fapi = await renderWeb3({ web3_wallets: [ fapiWeb3Wallet({ id: 'wallet_1', web3_wallet: '0x1234567890abcdef' }), @@ -475,13 +475,32 @@ describe('Web3 wallets', () => { const user = userEvent.setup(); await waitFor(() => expect(screen.getAllByRole('button', { name: 'Manage MetaMask' })).toHaveLength(2)); + expect(screen.getAllByRole('button', { name: 'Manage MetaMask' })[1].closest('.cl-section-row')).toContainElement( + screen.getByTitle('0xabcdef1234567890'), + ); await openMenu(user, screen.getAllByRole('button', { name: 'Manage MetaMask' })[0]); expect(screen.queryByRole('menuitem', { name: 'Set as primary' })).toBeNull(); await user.keyboard('{Escape}'); await openMenu(user, screen.getAllByRole('button', { name: 'Manage MetaMask' })[1]); - await user.click(screen.getByRole('menuitem', { name: 'Set as primary' })); + const primary = holdRequests('post', '/v1/me'); + try { + await user.click(screen.getByRole('menuitem', { name: 'Set as primary' })); + await waitFor(() => expect(primary.requests).toHaveLength(1)); + const menu = screen.getByRole('menu'); + const item = within(menu).getByRole('menuitem', { name: 'Set as primary' }); + expect(item).toHaveAttribute('aria-busy', 'true'); + expect(within(menu).getByRole('menuitem', { name: 'Remove wallet' })).toHaveAttribute('aria-disabled', 'true'); + expect(await within(item).findByRole('progressbar', { name: 'Set as primary' })).toBeVisible(); + expect(screen.getByRole('button', { name: 'Connect Solana' })).toBeDisabled(); + } finally { + primary.release(); + } await waitFor(() => expect(fapi.client.sessions[0]?.user.primary_web3_wallet_id).toBe('wallet_2')); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + expect(screen.getAllByRole('button', { name: 'Manage MetaMask' })[1].closest('.cl-section-row')).toContainElement( + screen.getByTitle('0xabcdef1234567890'), + ); const selectedRow = screen.getByTitle('0xabcdef1234567890').closest('.cl-section-row'); expect(selectedRow).not.toBeNull(); if (selectedRow) { @@ -582,6 +601,7 @@ describe('Web3 wallets', () => { await openMenu(user, secondManage()); await user.click(screen.getByRole('menuitem', { name: 'Set as primary' })); expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); + await waitFor(() => expect(screen.queryByRole('menu', { hidden: true })).toBeNull()); await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus()); await user.keyboard('hunter2{Enter}'); @@ -604,6 +624,7 @@ describe('Web3 wallets', () => { expect(screen.queryByRole('alert')).toBeNull(); await waitFor(() => expect(secondManage()).toHaveFocus()); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); await openMenu(user, secondManage()); await user.click(screen.getByRole('menuitem', { name: 'Set as primary' })); await waitFor(() => expect(fapi.client.sessions[0]?.user.primary_web3_wallet_id).toBe('wallet_2')); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx index e788761c532..95c3380aca1 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallet-row.view.tsx @@ -17,7 +17,6 @@ import type { UserProfileWeb3Provider, UserProfileWeb3Wallet } from './user-prof export function UserProfileWeb3WalletRowView({ wallet, triggerRef, - isPending = false, isDisabled = false, onConnect, onVerify, @@ -26,7 +25,6 @@ export function UserProfileWeb3WalletRowView({ }: { wallet: UserProfileWeb3Wallet | UserProfileWeb3Provider; triggerRef?: Ref; - isPending?: boolean; isDisabled?: boolean; onConnect?: (id: string) => void; onVerify?: (id: string) => void; @@ -35,6 +33,7 @@ export function UserProfileWeb3WalletRowView({ }) { const m = useMessages('userProfileWeb3Wallets'); const linkedWallet = 'address' in wallet ? wallet : undefined; + const isPending = wallet.isPending ?? false; const address = linkedWallet?.address; const shortAddress = address && (address.length <= 10 ? address : `${address.slice(0, 6)}...${address.slice(-4)}`); const actions: ActionMenuAction[] = []; @@ -43,7 +42,11 @@ export function UserProfileWeb3WalletRowView({ actions.push({ label: m.verify, onClick: () => onVerify(wallet.id) }); } if (linkedWallet && !linkedWallet.isPrimary && linkedWallet.isVerified && onSetPrimary) { - actions.push({ label: m.setPrimary, onClick: () => onSetPrimary(wallet.id) }); + actions.push({ + label: m.setPrimary, + onClick: () => onSetPrimary(wallet.id), + status: linkedWallet.isPrimaryPending ? 'pending' : 'idle', + }); } if (linkedWallet && onRemove && linkedWallet.canRemove !== false) { actions.push({ label: m.remove, color: 'negative', onClick: () => onRemove(linkedWallet) }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts index 4fba28cb105..46793528920 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts @@ -5,6 +5,7 @@ import { usePanelTitle } from '../../../components/panel'; import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus'; import { usePendingAction } from '../../../hooks/use-pending-action'; import { useMessages } from '../../../localization'; +import { applyOrder } from '../../../utils/apply-order'; import type { ReadyWeb3WalletsModel, UserProfileWeb3Provider, @@ -23,7 +24,7 @@ type WalletAction = | { kind: 'verify' | 'primary'; walletId: string }; export function useUserProfileWeb3WalletsController({ - wallets, + wallets: modelWallets, availableProviders, connect, verify, @@ -45,6 +46,8 @@ export function useUserProfileWeb3WalletsController({ | 'createReverification' >) { const messages = useMessages('userProfileWeb3Wallets'); + const [order] = useState(() => modelWallets.map(wallet => wallet.id)); + const wallets = applyOrder(order, modelWallets, wallet => wallet.id); const action = usePendingAction({ errorFallback: messages.errors.generic }); const [picker, setPicker] = useState({ open: false }); const [owner, setOwner] = useState(); @@ -58,7 +61,6 @@ export function useUserProfileWeb3WalletsController({ sectionRef.current ?? panelTitle(), }); - // TODO: Share the action lock during removal while preserving errors for the confirmation dialog. const removeWallet = useConfirmController({ onConfirm: wallet => removalFocus.remove(wallet.id), reverification: removeReverification, @@ -150,18 +152,26 @@ export function useUserProfileWeb3WalletsController({ const actionFinalFocus = () => (ownerId ? removalFocus.trigger(ownerId) : null); return { - wallets: wallets.map(wallet => - errorId === wallet.id + wallets: wallets.map(wallet => { + const isPending = pendingId === wallet.id; + const row = { + ...wallet, + isPending, + isPrimaryPending: isPending && owner?.kind === 'primary' && !setPrimaryReverification.visible, + }; + return errorId === wallet.id ? owner?.kind === 'primary' - ? { ...wallet, primaryError: action.error } - : { ...wallet, verifyError: action.error } - : wallet, - ), - availableProviders: availableProviders.map(provider => - errorId === provider.id && (!picker.open || picker.kind !== 'connect' || picker.provider.id !== provider.id) - ? { ...provider, connectError: action.error } - : provider, - ), + ? { ...row, primaryError: action.error } + : { ...row, verifyError: action.error } + : row; + }), + availableProviders: availableProviders.map(provider => { + const row = { ...provider, isPending: pendingId === provider.id }; + return errorId === provider.id && + (!picker.open || picker.kind !== 'connect' || picker.provider.id !== provider.id) + ? { ...row, connectError: action.error } + : row; + }), error: !picker.open && errorId && @@ -169,10 +179,10 @@ export function useUserProfileWeb3WalletsController({ !availableProviders.some(provider => provider.id === errorId) ? action.error : undefined, - pendingId, + isDisabled: action.isPending, sectionRef, registerTrigger: removalFocus.registerTrigger, - remove: remove ? removeWallet : undefined, + remove: remove ? action.guard(removeWallet) : undefined, finalFocus: removalFocus.finalFocus, setPrimaryReverification, createReverification, diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.tsx index 38f77348d3a..99a1e7d83b2 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.tsx @@ -34,7 +34,7 @@ function Web3Wallets({ model }: { model: ReadyWeb3WalletsModel }) { wallets={controller.wallets} sectionRef={controller.sectionRef} availableProviders={controller.availableProviders} - pendingId={controller.pendingId} + isDisabled={controller.isDisabled} error={controller.error} onConnect={id => { void controller.onConnect(id); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts index a0ce8fec689..5cfc8fab839 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts @@ -13,6 +13,7 @@ export interface UserProfileWeb3Provider { iconUrl?: string; monochromeIcon?: boolean; connectError?: string; + isPending?: boolean; } export interface UserProfileWeb3Wallet { @@ -30,13 +31,15 @@ export interface UserProfileWeb3Wallet { canRemove?: boolean; primaryError?: string; verifyError?: string; + isPending?: boolean; + isPrimaryPending?: boolean; } export interface UserProfileWeb3WalletsSectionViewProps { wallets: UserProfileWeb3Wallet[]; sectionRef?: Ref; availableProviders?: UserProfileWeb3Provider[]; - pendingId?: string; + isDisabled?: boolean; error?: string; onConnect?: (id: string) => void; onVerify?: (id: string) => void; diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx index bd259076163..0b166eebab6 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.view.tsx @@ -10,7 +10,7 @@ export function UserProfileWeb3WalletsSectionView({ wallets, sectionRef, availableProviders = [], - pendingId, + isDisabled = false, error, onConnect, onVerify, @@ -25,7 +25,6 @@ export function UserProfileWeb3WalletsSectionView({ }: UserProfileWeb3WalletsSectionViewProps) { const m = useMessages('userProfileWeb3Wallets'); const hasRows = wallets.length > 0 || (availableProviders.length > 0 && Boolean(onConnect)) || Boolean(error); - const isBusy = pendingId !== undefined; return ( <> @@ -46,8 +45,7 @@ export function UserProfileWeb3WalletsSectionView({ key={wallet.id} wallet={wallet} triggerRef={registerTrigger(wallet.id)} - isPending={pendingId === wallet.id} - isDisabled={isBusy} + isDisabled={isDisabled} onVerify={onVerify} onSetPrimary={onSetPrimary} onRemove={remove ? wallet => remove.open(wallet) : undefined} @@ -59,8 +57,7 @@ export function UserProfileWeb3WalletsSectionView({ key={provider.id} wallet={provider} triggerRef={registerTrigger(provider.id)} - isPending={pendingId === provider.id} - isDisabled={isBusy} + isDisabled={isDisabled} onConnect={onConnect} /> )) diff --git a/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx b/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx index b2e39244861..9179197a568 100644 --- a/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx +++ b/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx @@ -5,13 +5,24 @@ import { describe, expect, it, vi } from 'vitest'; import { deferred } from '../../__tests__/async'; import { clerkApiError } from '../../__tests__/clerk-errors'; +import { useConfirmController } from '../../blocks/confirm'; import { MosaicProvider } from '../../mosaic-provider'; +import { SaveError } from '../../utils/errors'; import { usePendingAction } from '../use-pending-action'; function blocked() { return clerkApiError('action_blocked', 'Raw server sentence.'); } +function useGuardedConfirm(onConfirm: () => Promise) { + const actions = usePendingAction(); + const remove = useConfirmController({ + onConfirm, + reverification: { status: 'loading', visible: true, onCancel: vi.fn() }, + }); + return { actions, remove: actions.guard(remove) }; +} + describe('usePendingAction', () => { it('reports success and leaves no error', async () => { const { result } = renderHook(() => usePendingAction()); @@ -264,3 +275,125 @@ describe('usePendingAction', () => { expect(result.current.errorKey).toBeUndefined(); }); }); + +describe('usePendingAction guard', () => { + it('refuses a guarded confirm while a run holds the lock, leaving the dialog unchanged', async () => { + const pending = deferred(); + const onConfirm = vi.fn(() => Promise.resolve()); + const { result } = renderHook(() => useGuardedConfirm(onConfirm)); + act(() => result.current.remove.onOpenChange(true)); + const owners = result.current; + + act(() => { + void owners.actions.run('row', () => pending.promise); + void owners.remove.confirm(); + }); + + expect(onConfirm).not.toHaveBeenCalled(); + expect(result.current.remove.isOpen).toBe(true); + expect(result.current.remove.isPending).toBe(false); + expect(result.current.actions.isPending).toBe(true); + await act(() => pending.resolve()); + expect(result.current.actions.isPending).toBe(false); + await act(() => result.current.remove.confirm()); + expect(onConfirm).toHaveBeenCalledOnce(); + expect(result.current.remove.isOpen).toBe(false); + }); + + it('refuses a run while a guarded confirm holds the lock, leaving its key and error unset', async () => { + const pending = deferred(); + const direct = vi.fn(); + const { result } = renderHook(() => useGuardedConfirm(() => pending.promise)); + act(() => result.current.remove.onOpenChange(true)); + const owners = result.current; + + await act(async () => { + void owners.remove.confirm(); + expect(await owners.actions.run('row', direct)).toBe(false); + }); + + expect(direct).not.toHaveBeenCalled(); + expect(result.current.actions.lastKey).toBeUndefined(); + expect(result.current.actions.error).toBeUndefined(); + expect(result.current.actions.isPending).toBe(true); + await act(() => pending.resolve()); + await act(async () => expect(await owners.actions.run('row', direct)).toBe(true)); + expect(direct).toHaveBeenCalledOnce(); + }); + + it('keeps the dialog error when a run refuses its retry', async () => { + const pending = deferred(); + const failure = new SaveError({ global: { code: 'action_blocked', message: 'Confirm refused.' } }); + const onConfirm = vi.fn(() => Promise.reject(failure)); + const { result } = renderHook(() => useGuardedConfirm(onConfirm)); + act(() => result.current.remove.onOpenChange(true)); + await act(() => result.current.remove.confirm()); + const error = result.current.remove.errorMessage; + expect(error).toBeDefined(); + expect(result.current.remove.isOpen).toBe(true); + expect(result.current.actions.error).toBeUndefined(); + + act(() => { + void result.current.actions.run('row', () => pending.promise); + void result.current.remove.confirm(); + }); + + expect(onConfirm).toHaveBeenCalledOnce(); + expect(result.current.remove.errorMessage).toBe(error); + expect(result.current.remove.isOpen).toBe(true); + expect(result.current.remove.isPending).toBe(false); + await act(() => pending.resolve()); + }); + + it('keeps the last key and clears the run error while a guarded confirm runs', async () => { + const pending = deferred(); + const failure = new SaveError({ global: { code: 'action_blocked', message: 'Run refused.' } }); + const { result } = renderHook(() => useGuardedConfirm(() => pending.promise)); + await act(() => result.current.actions.run('row', () => Promise.reject(failure))); + expect(result.current.actions.error).toBeDefined(); + + act(() => result.current.remove.onOpenChange(true)); + act(() => { + void result.current.remove.confirm(); + }); + + expect(result.current.actions.isPending).toBe(true); + expect(result.current.actions.pendingKey).toBeUndefined(); + expect(result.current.actions.lastKey).toBe('row'); + expect(result.current.actions.errorKey).toBeUndefined(); + expect(result.current.actions.error).toBeUndefined(); + await act(() => pending.resolve()); + expect(result.current.actions.isPending).toBe(false); + expect(result.current.actions.lastKey).toBe('row'); + }); + + it.each(['resolve', 'reject'])( + 'holds the lock after the dialog is cancelled until its action promises %s', + async outcome => { + const pending = deferred(); + const direct = vi.fn(); + const { result } = renderHook(() => useGuardedConfirm(() => pending.promise)); + act(() => result.current.remove.onOpenChange(true)); + act(() => { + void result.current.remove.confirm(); + }); + act(() => result.current.remove.onOpenChange(false)); + expect(result.current.remove.isOpen).toBe(false); + await act(async () => expect(await result.current.actions.run('row', direct)).toBe(false)); + expect(direct).not.toHaveBeenCalled(); + expect(result.current.actions.isPending).toBe(true); + expect(result.current.actions.pendingKey).toBeUndefined(); + + await act(() => { + if (outcome === 'reject') { + pending.reject(new Error('Late confirm failure')); + } else { + pending.resolve(); + } + }); + expect(result.current.actions.isPending).toBe(false); + expect(result.current.remove.errorMessage).toBeUndefined(); + await act(async () => expect(await result.current.actions.run('row', direct)).toBe(true)); + }, + ); +}); diff --git a/packages/mosaic/src/hooks/use-list-removal-focus.ts b/packages/mosaic/src/hooks/use-list-removal-focus.ts index 73621b38222..38a4604ede1 100644 --- a/packages/mosaic/src/hooks/use-list-removal-focus.ts +++ b/packages/mosaic/src/hooks/use-list-removal-focus.ts @@ -1,3 +1,4 @@ +import type { RefCallback } from 'react'; import { useRef } from 'react'; export function useListRemovalFocus({ @@ -10,14 +11,23 @@ export function useListRemovalFocus({ fallback: () => HTMLElement | null; }) { const triggers = useRef(new Map()); + const triggerRefs = useRef(new Map>()); const removed = useRef<{ id: string; index: number } | undefined>(undefined); - const registerTrigger = (id: string) => (element: HTMLButtonElement | null) => { - if (element) { - triggers.current.set(id, element); - } else { - triggers.current.delete(id); + const registerTrigger = (id: string) => { + const existing = triggerRefs.current.get(id); + if (existing) { + return existing; } + const ref = (element: HTMLButtonElement | null) => { + if (element) { + triggers.current.set(id, element); + } else { + triggers.current.delete(id); + } + }; + triggerRefs.current.set(id, ref); + return ref; }; const remove = async (id: string) => { diff --git a/packages/mosaic/src/hooks/use-pending-action.ts b/packages/mosaic/src/hooks/use-pending-action.ts index bfeb88f3274..e620d828510 100644 --- a/packages/mosaic/src/hooks/use-pending-action.ts +++ b/packages/mosaic/src/hooks/use-pending-action.ts @@ -10,8 +10,15 @@ export interface PendingActionOptions { errorFallback?: string; } +export interface PendingConfirm { + confirm: (payload: TPayload) => Promise; +} + export interface PendingAction { run: (key: TKey, action: () => Promise | void, options?: PendingActionOptions) => Promise; + guard: >( + confirm: TConfirm & PendingConfirm, + ) => TConfirm; pendingKey: TKey | undefined; lastKey: TKey | undefined; isPending: boolean; @@ -26,46 +33,64 @@ interface PendingActionFailure { errorFallback: string | undefined; } +type Pending = { kind: 'inline'; key: TKey } | { kind: 'external' }; + /** - * Runs inline actions, such as row buttons, under one lock and tracks which key is pending, which key ran last, and which key last failed. - * `run` resolves `true` on success and `false` on failure, ignores calls while any run is in flight, and clears the last error when it starts. + * Runs inline actions and guarded confirmations under one lock and tracks which inline key is pending, ran last, or last failed. + * `run` resolves `true` on success and `false` on failure, ignores calls while any action is in flight, and clears the last error when any action starts. */ export function usePendingAction({ errorFallback, }: PendingActionOptions = {}): PendingAction { const errorText = useErrorText(); - const [pendingKey, setPendingKey] = useState(); + const running = useRef(false); + const [pending, setPending] = useState>(); const [lastKey, setLastKey] = useState(); const [failure, setFailure] = useState>(); - const running = useRef(false); - const run = async (key: TKey, action: () => Promise | void, options: PendingActionOptions = {}) => { + const claim = (next: Pending, action: () => T | Promise): Promise | undefined => { if (running.current) { - return false; + return undefined; } running.current = true; - setPendingKey(key); - setLastKey(key); + setPending(next); setFailure(undefined); - try { - await action(); - return true; - } catch (cause) { - if (!isReverificationCancelledError(cause)) { - setFailure({ key, error: toLocalizableError(cause), errorFallback: options.errorFallback }); + return (async () => { + try { + return await action(); + } finally { + running.current = false; + setPending(undefined); } - return false; - } finally { - running.current = false; - setPendingKey(undefined); - } + })(); + }; + + const run = async (key: TKey, action: () => Promise | void, options: PendingActionOptions = {}) => { + return ( + claim({ kind: 'inline', key }, async () => { + setLastKey(key); + try { + await action(); + return true; + } catch (cause) { + if (!isReverificationCancelledError(cause)) { + setFailure({ key, error: toLocalizableError(cause), errorFallback: options.errorFallback }); + } + return false; + } + }) ?? false + ); }; return { run, - pendingKey, + guard: confirm => ({ + ...confirm, + confirm: async payload => (await claim({ kind: 'external' }, () => confirm.confirm(payload))) ?? false, + }), + pendingKey: pending?.kind === 'inline' ? pending.key : undefined, lastKey, - isPending: pendingKey !== undefined, + isPending: pending !== undefined, error: failure ? errorText(failure.error, failure.errorFallback ?? errorFallback) : undefined, errorKey: failure?.key, reset: () => setFailure(undefined),