fix: wip

packages/app-tests/e2e/envelope-editor-v2/envelope-recipient-groups.spec.ts

@@ -5,6 +5,9 @@ import {
   clickAddSignerButton,
   dragGroupCardOntoCard,
   dragRecipientRowToGap,
+  getRecipientEmailInputs,
+  getRecipientStepCards,
+  moveGroupCardUp,
   openDocumentEnvelopeEditor,
   openTemplateEnvelopeEditor,
   setRecipientEmail,
@@ -55,7 +58,7 @@ const runGroupingFlow = async (surface: TEnvelopeEditorSurface) => {
   // Drag carol's card onto bob's card to merge them into one group.
   await dragGroupCardOntoCard(root, 2, 1);
 
-  await expect(root.getByText('2 signers · any order')).toBeVisible();
+  await expect(root.getByText('2 recipients · any order')).toBeVisible();
   await expect(root.getByTestId('ungroup-step-button')).toBeVisible();
   await expect(root.getByText('Group 3', { exact: true })).not.toBeVisible();
 
@@ -67,12 +70,12 @@ const runGroupingFlow = async (surface: TEnvelopeEditorSurface) => {
 
   // Groups survive a reload (grouped normalization on load).
   await root.reload();
-  await expect(root.getByText('2 signers · any order')).toBeVisible();
+  await expect(root.getByText('2 recipients · any order')).toBeVisible();
 
   // Ungroup dissolves back into sequential groups.
   await root.getByTestId('ungroup-step-button').click();
 
-  await expect(root.getByText('2 signers · any order')).not.toBeVisible();
+  await expect(root.getByText('2 recipients · any order')).not.toBeVisible();
   await expect(root.getByText('Group 3', { exact: true })).toBeVisible();
 
   await expectRecipientOrders(surface, [
@@ -97,6 +100,32 @@ test.describe('document editor', () => {
 
     await runGroupingFlow(surface);
   });
+
+  test('documents: reordered group cards can still be dragged', async ({ page }) => {
+    const surface = await openDocumentEnvelopeEditor(page);
+    const { root } = surface;
+
+    await setRecipientEmail(root, 0, 'alice@example.com');
+    await clickAddSignerButton(root);
+    await setRecipientEmail(root, 1, 'bob@example.com');
+
+    await toggleSigningOrder(root, true);
+    await expect(getRecipientStepCards(root)).toHaveCount(2);
+
+    // Move bob's card into position 1.
+    await moveGroupCardUp(root, 1);
+
+    await expect(getRecipientEmailInputs(root).nth(0)).toHaveValue('bob@example.com');
+    await expect(getRecipientEmailInputs(root).nth(1)).toHaveValue('alice@example.com');
+
+    // Regression: after a reorder, the card moved into position 2 must still
+    // be draggable — positional drag-and-drop ids used to go stale on mounted
+    // cards, silently killing their drag handles. Prove it by completing a
+    // merge with the repositioned card.
+    await dragGroupCardOntoCard(root, 1, 0);
+
+    await expect(root.getByText('2 recipients · any order')).toBeVisible();
+  });
 });
 
 test.describe('template editor', () => {

packages/app-tests/e2e/envelope-editor-v2/envelope-recipients.spec.ts

@@ -112,6 +112,12 @@ const runRecipientFlow = async (surface: TEnvelopeEditorSurface): Promise<Recipi
   await setRecipientRole(surface.root, 1, 'Needs to approve');
   await setRecipientRole(surface.root, 2, 'Receives copy');
 
+  // The role selects must reflect the change immediately, without requiring a
+  // navigation or reload (regression: leaf controllers going stale after a
+  // root-level signers array update).
+  await assertRecipientRole(surface.root, 1, 'Needs to approve');
+  await assertRecipientRole(surface.root, 2, 'Receives copy');
+
   await getRecipientRemoveButtons(surface.root).nth(2).click();
   await expect(getRecipientEmailInputs(surface.root)).toHaveCount(2);
 

packages/app-tests/e2e/fixtures/envelope-editor.ts

@@ -395,32 +395,48 @@ export const dragHandleToTarget = async (
     return Boolean(className?.includes(activeClass));
   };
 
-  // Crawl-and-drop: drop-target geometry is captured at drag start and can
-  // drift from the live layout (and auto-scrolling invalidates any cached
-  // coordinates), so precise aiming is unreliable. Instead, approach from
-  // well above the target and crawl downward in small increments, dropping
-  // the moment the target reports the drag as over it — the highlight class
-  // is rendered from the library's own drag state, so it cannot disagree
-  // with where the drop will land.
+  // The highlight class is rendered from the library's own drag state, so it
+  // cannot disagree with where a drop will land — both phases below only drop
+  // once the target reports the drag as over it AND that state survives a
+  // short confirmation dwell (it can flicker while crossing a card's
+  // reorder/combine boundary).
   //
-  // The cursor is clamped inside the viewport: moving outside the window
-  // cancels the drag (pointercancel), and holding near the bottom edge lets
-  // the library auto-scroll the target up to the cursor instead.
+  // The cursor is always clamped inside the viewport: moving outside the
+  // window cancels the drag (pointercancel), and holding near the bottom edge
+  // lets the library auto-scroll the target up to the cursor instead.
   const viewportHeight = root.viewportSize()?.height ?? 720;
   const maxCursorY = viewportHeight - 40;
 
+  const confirmAndDrop = async () => {
+    if (!(await hasBecomeActive())) {
+      return false;
+    }
+
+    await root.waitForTimeout(150);
+
+    if (!(await hasBecomeActive())) {
+      return false;
+    }
+
+    await root.mouse.up();
+
+    return true;
+  };
+
+  let hasDropped = false;
+
+  // Crawl-and-drop: approach from above and inch downward through the
+  // corridor. Captured drop-target geometry can drift a few pixels from the
+  // live layout for small targets, so a slow traversal is the reliable way to
+  // hit them.
   const crawlX = targetBox.x + targetBox.width / 2 - itemOffsetX;
   const crawlStartY = Math.min(targetBox.y + targetBox.height / 2 - itemOffsetY - 140, maxCursorY);
 
   await root.mouse.move(crawlX, crawlStartY, { steps: 15 });
   await root.waitForTimeout(150);
 
-  let hasDropped = false;
-
   for (let step = 1; step <= 80; step += 1) {
-    if (await hasBecomeActive()) {
-      await root.mouse.up();
-
+    if (await confirmAndDrop()) {
       hasDropped = true;
 
       break;
@@ -446,16 +462,65 @@ export const getStepDragHandles = (root: Page) => root.locator('[data-testid="st
 export const getRecipientRowDragHandles = (root: Page) => root.locator('[data-testid="recipient-row-drag-handle"]');
 
 /**
- * Drags a whole group card onto another card's centre, merging the two groups.
+ * Drags a whole group card onto another card, merging the two groups.
+ *
+ * Uses @hello-pangea/dnd's keyboard drag mode: mouse-emulated combines are
+ * unreliable because approaching a card traverses its reorder edge, which
+ * displaces the target away from the cursor. Keyboard drags step through
+ * positions (including combine states) deterministically.
  */
 export const dragGroupCardOntoCard = async (root: Page, sourceCardIndex: number, targetCardIndex: number) => {
-  await dragHandleToTarget(
-    root,
-    getStepDragHandles(root).nth(sourceCardIndex),
-    getRecipientStepCards(root).nth(targetCardIndex),
-    // The combine/join highlight on the target card.
-    { activeClass: 'ring-primary' },
-  );
+  const handle = getStepDragHandles(root).nth(sourceCardIndex);
+  const target = getRecipientStepCards(root).nth(targetCardIndex);
+
+  await handle.scrollIntoViewIfNeeded();
+  await handle.focus();
+
+  // Lift.
+  await root.keyboard.press('Space');
+  await root.waitForTimeout(250);
+
+  const direction = targetCardIndex < sourceCardIndex ? 'ArrowUp' : 'ArrowDown';
+
+  for (let press = 0; press < 4; press += 1) {
+    await root.keyboard.press(direction);
+    await root.waitForTimeout(250);
+
+    const targetClassName = await target.getAttribute('class');
+
+    if (targetClassName?.includes('ring-primary')) {
+      // Drop while the target reports the combine state.
+      await root.keyboard.press('Space');
+      await root.waitForTimeout(400);
+
+      return;
+    }
+  }
+
+  await root.keyboard.press('Escape');
+
+  throw new Error('Combine drag did not reach the target card');
+};
+
+/**
+ * Moves a group card one position up via keyboard drag. With combining
+ * enabled, the first ArrowUp enters the combine state with the card above and
+ * the second moves above it.
+ */
+export const moveGroupCardUp = async (root: Page, cardIndex: number) => {
+  const handle = getStepDragHandles(root).nth(cardIndex);
+
+  await handle.scrollIntoViewIfNeeded();
+  await handle.focus();
+
+  await root.keyboard.press('Space');
+  await root.waitForTimeout(250);
+  await root.keyboard.press('ArrowUp');
+  await root.waitForTimeout(250);
+  await root.keyboard.press('ArrowUp');
+  await root.waitForTimeout(250);
+  await root.keyboard.press('Space');
+  await root.waitForTimeout(400);
 };
 
 /**
@@ -467,8 +532,8 @@ export const dragRecipientRowToGap = async (root: Page, rowIndex: number, gapInd
     root,
     getRecipientRowDragHandles(root).nth(rowIndex),
     getRecipientStepGaps(root).nth(gapIndex),
-    // The drag-over highlight on the gap drop-zone.
-    { activeClass: 'border-primary' },
+    // The marker class applied to a gap drop-zone while dragged over.
+    { activeClass: 'gap-active' },
   );
 };
 

packages/lib/client-only/hooks/use-editor-recipients.ts

@@ -88,6 +88,49 @@ export const ZEditorRecipientsFormSchema = z
 
 export type TEditorRecipientsFormSchema = z.infer<typeof ZEditorRecipientsFormSchema>;
 
+/**
+ * Replaces the signers array while keeping controlled inputs in sync.
+ *
+ * Rows are rendered with stable `formId` keys (required for drag and drop),
+ * so react-hook-form `Controller`s never remount and their leaf
+ * subscriptions are NOT re-notified by a root-level array `setValue`. Any
+ * value that changes while a signer keeps its index (e.g. a role change)
+ * must be leaf-set first so the controlled input actually re-renders.
+ */
+export const updateEditorSigners = (
+  form: UseFormReturn<TEditorRecipientsFormSchema>,
+  updatedSigners: TEditorRecipientsFormSchema['signers'],
+) => {
+  const previousSigners = form.getValues('signers');
+
+  updatedSigners.forEach((signer, index) => {
+    const previousSigner = previousSigners[index];
+
+    // Only slot-stable signers need leaf notifications — moved signers get a
+    // new field name and re-subscribe with fresh values on their own.
+    if (!previousSigner || previousSigner.formId !== signer.formId) {
+      return;
+    }
+
+    if (previousSigner.role !== signer.role) {
+      form.setValue(`signers.${index}.role`, signer.role, { shouldDirty: true });
+    }
+
+    if (previousSigner.email !== signer.email) {
+      form.setValue(`signers.${index}.email`, signer.email, { shouldDirty: true });
+    }
+
+    if (previousSigner.name !== signer.name) {
+      form.setValue(`signers.${index}.name`, signer.name, { shouldDirty: true });
+    }
+  });
+
+  form.setValue('signers', updatedSigners, {
+    shouldValidate: true,
+    shouldDirty: true,
+  });
+};
+
 type EditorRecipientsProps = {
   envelope: TEditorEnvelope;
 };