fix(mosaic): coordinate pending identifier actions

.changeset/friendly-identifier-lock.md

@@ -0,0 +1,2 @@
+---
+---

packages/mosaic/docs/controllers.md

@@ -37,12 +37,33 @@ The choice is invisible from outside: the controller returns plain props either
 const actions = usePendingAction({ errorFallback: messages.errors.generic });
 
 return {
-  rows: rows.map(row => (actions.errorKey === row.id ? { ...row, error: actions.error } : row)),
-  pendingId: actions.pendingKey,
+  rows: rows.map(row => ({
+    ...row,
+    isPending: actions.pendingKey === row.id,
+    error: actions.errorKey === row.id ? actions.error : undefined,
+  })),
   onRemove: id => void actions.run(id, () => onRemove(id)),
 };
 ```
 
+When direct actions and a confirmation must not run together, keep each owner and combine their pending state in the section controller:
+
+```ts
+const actions = usePendingAction();
+const remove = useConfirmController<UserProfileEmail>({
+  onConfirm: contact => removalFocus.remove(contact.id),
+  reverification,
+});
+
+return {
+  contacts: contacts.map(contact => ({ ...contact, isPrimaryPending: actions.pendingKey === contact.id })),
+  isDisabled: actions.isPending || remove.isPending,
+  remove,
+};
+```
+
+Pass `isDisabled` to every control that starts one of the actions, including the item that opens the confirmation. `remove.isPending` stays true after the dialog closes during a reverification until the confirmed action settles, and the controller ignores `confirm` until then, so closing the dialog never lets a second action start. Put each row's pending state on the row the controller returns, so views render it without comparing pending keys.
+
 - `run` resolves `true` on success, so the caller can close a dialog or move focus. A cancelled reverification resolves `false`, shows no error and releases the lock.
 - To hold pending through an OAuth redirect, await a delay inside the action (`user-profile-connected-accounts-section.controller.ts`).
 - It is not for a form (`useForm`), a confirm step (`useConfirmController`), an action whose result drives other state (a machine), or a check that reruns as the user types (`useDebouncedAsync`, or a field's `validateAsync`).

packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts

@@ -170,4 +170,43 @@ describe('useConfirmController', () => {
     await waitFor(() => expect(result.current.isPending).toBe(false));
     expect(result.current.isOpen).toBe(false);
   });
+  it.each(['resolve', 'reject'] as const)(
+    'stays pending after the dialog is cancelled until the run can %s, without a second run',
+    async outcome => {
+      const run = deferred<void>();
+      const onConfirm = vi.fn(() => run.promise);
+      const reverification = { status: 'loading' as const, onCancel: vi.fn(), visible: false };
+      const { result } = renderHook(() => useConfirmController({ onConfirm, reverification }));
+
+      act(() => result.current.onOpenChange(true));
+      act(() => result.current.confirm());
+      act(() => result.current.onOpenChange(false));
+
+      expect(reverification.onCancel).toHaveBeenCalledOnce();
+      expect(result.current.isOpen).toBe(false);
+      expect(result.current.isPending).toBe(true);
+
+      act(() => result.current.onOpenChange(true));
+      act(() => result.current.confirm());
+
+      expect(onConfirm).toHaveBeenCalledOnce();
+      expect(result.current.isOpen).toBe(false);
+
+      await act(async () => {
+        if (outcome === 'resolve') {
+          run.resolve();
+        } else {
+          run.reject(blocked);
+        }
+        await run.promise.catch(() => undefined);
+      });
+
+      expect(result.current.isPending).toBe(false);
+      expect(result.current.isOpen).toBe(false);
+      expect(result.current.errorMessage).toBeUndefined();
+
+      act(() => result.current.onOpenChange(true));
+      expect(result.current.isOpen).toBe(true);
+    },
+  );
 });

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();

packages/mosaic/src/features/user-profile/user-profile-contact-list-row.view.tsx

@@ -10,7 +10,7 @@ import { Section } from '../../components/section';
 import { Plus } from '../../icons/glyphs/plus';
 import { fill, useMessages } from '../../localization';
 import { truncationStyles } from '../../styles/typography.styles';
-import type { UserProfileContact } from './user-profile-contact.types';
+import type { UserProfileContact, UserProfileContactRow } from './user-profile-contact.types';
 import { styles } from './user-profile-profile-panel.styles';
 
 export interface UserProfileContactListRowViewProps {
@@ -19,10 +19,10 @@ export interface UserProfileContactListRowViewProps {
   addAction?: ReactNode;
   kind: 'email' | 'phone';
   label: string;
-  items: UserProfileContact[];
+  items: UserProfileContactRow[];
   onAdd?: () => void;
   onVerify?: (id: string) => void;
-  pendingVerifyId?: string;
+  isDisabled?: boolean;
   onSetPrimary?: (id: string) => void;
   onRemove?: (item: UserProfileContact) => void;
   children?: ReactNode;
@@ -34,7 +34,7 @@ export function UserProfileContactListRowView({
   items,
   onAdd,
   onVerify,
-  pendingVerifyId,
+  isDisabled = false,
   onSetPrimary,
   onRemove,
   addAction,
@@ -65,6 +65,7 @@ export function UserProfileContactListRowView({
               size='sm'
               variant='outline'
               onClick={onAdd}
+              disabled={isDisabled}
             >
               <Icon
                 glyph={Plus}
@@ -92,10 +93,14 @@ 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) });
+                actions.push({
+                  label: m.setPrimary,
+                  onClick: () => onSetPrimary(item.id),
+                  status: item.isPrimaryPending ? 'pending' : 'idle',
+                });
               }
 
               if (onRemove) {
@@ -120,6 +125,8 @@ export function UserProfileContactListRowView({
                       <ActionMenu
                         triggerRef={triggerRef?.(item.id)}
                         actions={actions}
+                        disabled={isDisabled}
+                        focusableWhenDisabled
                         label={fill(m.manageValue, { value: item.value })}
                       />
                     </Section.Actions>

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;

packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx

@@ -72,6 +72,50 @@ 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('sets an address as primary once when Set as primary is double-clicked', async () => {
+    const other = fapiEmailAddress({ id: 'idn_other', email_address: 'bob@example.com', verification: VERIFIED });
+    const { actor, fapi } = await renderSection(signedInWithEmails([PRIMARY, other]));
+    const primary = holdRequests('post', '/v1/me');
+
+    await openMenu(actor, within(emailRow()).getByRole('button', { name: 'Manage bob@example.com' }));
+    await actor.dblClick(screen.getByRole('menuitem', { name: 'Set as primary' }));
+    await waitFor(() => expect(primary.requests).toHaveLength(1));
+
+    primary.release();
+    await waitFor(() => expect(fapi.client.sessions[0]?.user?.primary_email_address_id).toBe('idn_other'));
+    expect(primary.requests).toHaveLength(1);
+  });
+
   it('adds an address and verifies the code it was sent', async () => {
     const { actor } = await renderSection(signedInWithEmails([]));
     expect(emailRow()).toHaveTextContent('No email addresses added');
@@ -360,6 +404,7 @@ describe('email reverification', () => {
 
     await manageEmail(actor, 'bob@example.com', 'Set as primary');
     expect(await screen.findByText('Enter your current password to continue')).toBeVisible();
+    await waitFor(() => expect(screen.queryByRole('menu', { hidden: true })).toBeNull());
     await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus());
     await actor.keyboard('hunter2{Enter}');
 
@@ -386,6 +431,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'));
   });
@@ -663,4 +709,29 @@ describe('Removing an address', () => {
     await waitFor(() => expect(emailsListed()).toEqual(['alice@example.com']));
     expect(screen.queryByRole('alertdialog')).toBeNull();
   });
+
+  it('locks the section while a removal runs, and unlocks it when the removal fails with the error in the dialog', async () => {
+    const second = fapiEmailAddress({ id: 'idn_second', email_address: 'bob@example.com', verification: VERIFIED });
+    const { actor } = await renderSection(signedInWithEmails([PRIMARY, second]));
+    const removal = holdRequests('post', '/v1/me/email_addresses/idn_second', { _method: 'DELETE' });
+    const manageAlice = () => screen.getByRole('button', { name: 'Manage alice@example.com', hidden: true });
+    const addEmail = () => screen.getByRole('button', { name: 'Add email', hidden: true });
+
+    await manageEmail(actor, 'bob@example.com', 'Remove email');
+    const dialog = await screen.findByRole('alertdialog', { name: 'Remove email address?' });
+    await actor.click(within(dialog).getByRole('button', { name: 'Remove' }));
+    await waitFor(() => expect(removal.requests).toHaveLength(1));
+
+    expect(manageAlice()).toHaveAttribute('aria-disabled', 'true');
+    expect(addEmail()).toBeDisabled();
+
+    removal.fail('last_identification', 'You cannot remove your last sign-in method.');
+    await waitFor(() =>
+      expect(within(dialog).getByRole('alert')).toHaveTextContent('You cannot remove your last sign-in method.'),
+    );
+    expect(dialog).toBeVisible();
+    expect(manageAlice()).not.toHaveAttribute('aria-disabled', 'true');
+    expect(addEmail()).toBeEnabled();
+    expect(emailsListed()).toEqual(['alice@example.com', 'bob@example.com']);
+  });
 });

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<ReadyEmailSectionModel, 'status' | 'userId'>): 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<HTMLDivElement>(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 || remove.isPending,
     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)),
   };
 }

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<HTMLDivElement>;

packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx

@@ -13,6 +13,7 @@ import type { UserProfileEmailSectionViewProps } from './user-profile-email-sect
 export function UserProfileEmailSectionView({
   emails,
   canAdd,
+  isDisabled,
   verification,
   error,
   rowRef,
@@ -36,6 +37,7 @@ export function UserProfileEmailSectionView({
           color='neutral'
           size='sm'
           variant='outline'
+          disabled={isDisabled}
         />
       }
     >
@@ -60,7 +62,7 @@ export function UserProfileEmailSectionView({
         onRemove={remove ? email => remove.open(email) : undefined}
         onSetPrimary={onSetPrimary}
         onVerify={onVerify}
-        pendingVerifyId={verification.pendingVerifyId}
+        isDisabled={isDisabled}
       >
         <Section.Error>{error}</Section.Error>
       </UserProfileContactListRowView>