Wait for the drop target's hover in the dragAndDropThought helper

docs/drag-and-drop.md

@@ -68,6 +68,8 @@ Three companion fields ride alongside it:
 
 These are what `DropHover` and other UI components subscribe to.
 
+Each drop target element (the thought rows, `DropChild`, `DropEnd` and `DropUncle`) renders `data-drop-over`: react-dnd's `isOver({ shallow: true })` for that target, which is true even on a target that rejects the drop. It names the target a drop lands on, and it is set in the same commit as the target's `isHovering`, from which `DropChild`, `DropEnd` and `DropUncle` draw their drop hover. A thought row's drop hover also needs `state.hoveringPath`, which the same hover writes first, but only when the pointer has moved (`throttleByMousePosition`). react-dnd itself moves its hover only on a `dragover`, and a drag driven by Puppeteer dispatches `dragover` only when the pointer moves. So when a hover changes the layout under a stationary pointer, e.g. by expanding a thought, the drop target that ends up under the pointer is not hovered until the pointer moves again. The Puppeteer helper [`dragAndDropThought`](../src/e2e/puppeteer/helpers/dragAndDropThought.ts) therefore records `data-drop-over` on the drop target under the pointer as it is rendered, and waits for that record before it holds or drops.
+
 ### Drag sources
 
 There are four drag sources in the app, each implemented as a hook:
@@ -94,7 +96,7 @@ The drop targets in the UI are:
 - [`DropChild`](../src/components/DropChild.tsx) — a `SubthoughtsDrop` zone rendered inside a thought when it's empty or collapsed.
 - [`DropEnd`](../src/components/DropEnd.tsx) — a `SubthoughtsDrop` zone at the end of a subthought list.
 - [`DropCliff`](../src/components/DropCliff.tsx) — renders one `DropEnd` per cliff level when depth decreases by more than 1, so a thought can be dropped at any of the intermediate levels exposed by the cliff.
-- [`DropUncle`](../src/components/DropUncle.tsx) — a `ThoughtDrop` zone for inserting before the next hidden uncle, when the immediate parent is hidden by autofocus.
+- [`DropUncle`](../src/components/DropUncle.tsx) — a `SubthoughtsDrop` zone for inserting before the next hidden uncle, when the immediate parent is hidden by autofocus.
 - [`Favorites`](../src/components/Favorites.tsx) — its own zone, handled by `useDragDropFavorites`.
 - [`DropGutter`](../src/components/DropGutter.tsx) — the right-edge invisible quick-delete panel; see [Quick drops](#quick-drops).
 

docs/testing.md

@@ -640,6 +640,7 @@ Integration tests are blackbox, but named helpers may take shortcuts during arra
 | Text paint assertion | Assert | [`getThoughtPixels`](../src/e2e/puppeteer/helpers/getThoughtPixels.ts) | Read pixels within a thought's text bounds through the shared stabilized screenshot helper and an offscreen canvas. Does not change the thought or app state. |
 | Structural assertion | Assert | [`exportThoughts`](../src/e2e/puppeteer/helpers/exportThoughts.ts) | Export the thought tree as plaintext. Do not make additional assertions on Redux state. |
 | Non-visual synchronization | Wait | [`waitForThoughtspaceIdle`](../src/e2e/puppeteer/helpers/waitForThoughtspaceIdle.ts) | Use only when persistence or another prerequisite has no immediate visual signal. `waitForThoughtspaceIdle` waits for every queued persistence write to commit and rejects if one failed; call it only to synchronize before a reload or before asserting that a write landed. `refresh` waits for persisted state to load after a reload, but shared setup and `resetApp` deliberately do not wait for database initialization. Assert the final user-visible result separately. |
+| Drag-and-drop synchronization | Wait | [`dragAndDropThought`](../src/e2e/puppeteer/helpers/dragAndDropThought.ts) | Waits until react-dnd has hovered the drop target under the pointer, read from its `data-drop-over` attribute ([drag-and-drop.md](drag-and-drop.md#state-machine-statelongpress)). For the length of the move it keeps a `MutationObserver` on `window.__dropTargetHoverWatch`, since a Puppeteer drag dispatches `dragover` only on pointer moves, and a hover can change the layout under the stationary pointer, e.g. by hover expansion, before a poll would see it. Tests must not wait on drop hovers themselves. |
 | Flake diagnosis | Arrange | [`withDebugLog`](../src/e2e/puppeteer/helpers/withDebugLog.ts) | Wrap only a test under investigation for an intermittent failure. It records the [Debug Log](debug-log.md) and appends it to the error on failure, where the Puppeteer Flaky workflow's failure report carries it. Logging adds a little synchronous work to every action and input event, so it can make a race rarer; remove the wrapper once the flake is fixed. |
 
 DOM reads are different from backdoors: inline `page.evaluate`/`browser.execute` may read user-visible DOM when no helper exists, though a repeated read should become a named helper. It may not dispatch actions, mutate app state, set test flags, or write to the DOM.
@@ -706,7 +707,7 @@ Puppeteer input is coordinated through the helpers in [`../src/e2e/puppeteer/hel
 | Press a key or shortcut | [`press`](../src/e2e/puppeteer/helpers/press.ts) | Presses a key with optional `alt`, `ctrl`, `meta`, and `shift` modifiers. |
 | Swipe/command gesture | [`gesture`](../src/e2e/puppeteer/helpers/gesture.ts) | Emits `touchStart`, stepped `touchMove` events, and `touchEnd` for the supplied direction path or command gesture. |
 | Long press | [`longPressThought`](../src/e2e/puppeteer/helpers/longPressThought.ts), [`longPressBullet`](../src/e2e/puppeteer/helpers/longPressBullet.ts), [`longPressWord`](../src/e2e/puppeteer/helpers/longPressWord.ts) | Holds a touch on the thought, or the mouse on its bullet, until the bullet reports the long-press highlight, then releases and waits for the body's `data-drag-hold` attribute to return to `false`, since the app ends the long press, and commits the selection, on a timer after the release. Long press is bound to the bullet on every platform but to the thought itself only on touch, so `longPressBullet` is how a thought is long pressed on desktop, without emulating a device. `longPressWord` holds a touch on a word until the highlight, adds Chrome for Android's native long press response (it selects the word, then dispatches a touch `contextmenu`), and leaves the finger down for the test to move or lift. |
-| Drag and drop | [`dragAndDropThought`](../src/e2e/puppeteer/helpers/dragAndDropThought.ts), [`dragAndDropFavorite`](../src/e2e/puppeteer/helpers/dragAndDropFavorite.ts), [`dragAndDrop`](../src/e2e/puppeteer/helpers/dragAndDrop.ts) | Drives real mouse down/move/up input and waits for drag-specific visible conditions. |
+| Drag and drop | [`dragAndDropThought`](../src/e2e/puppeteer/helpers/dragAndDropThought.ts), [`dragAndDropFavorite`](../src/e2e/puppeteer/helpers/dragAndDropFavorite.ts), [`dragAndDrop`](../src/e2e/puppeteer/helpers/dragAndDrop.ts) | Drives real mouse down/move/up input and waits for drag-specific visible conditions. `dragAndDropThought` waits until react-dnd has hovered the drop target under the pointer before it holds or drops, so a test never waits on a drop hover itself. |
 | Scroll | [`scroll`](../src/e2e/puppeteer/helpers/scroll.ts), [`scrollBy`](../src/e2e/puppeteer/helpers/scrollBy.ts), [`scrollIntoView`](../src/e2e/puppeteer/helpers/scrollIntoView.ts), [`scrollTo`](../src/e2e/puppeteer/helpers/scrollTo.ts) | Scrolls the window or a named container; use the narrowest helper that expresses the intent. |
 | Emulate a mobile device | [`deviceEmulation.useForSuite`](../src/e2e/puppeteer/helpers/deviceEmulation.ts) | Selects a Puppeteer device profile at suite scope, which `setup` applies before navigation. There is no mid-session equivalent; see the emulation note above. |
 

src/components/DropChild.tsx

@@ -33,7 +33,7 @@ const DropChild = ({ depth, path, simplePath, cliff, isLastVisible }: DropChildP
   const dropHoverColorValue = useSelector(state => dropHoverColor(state, depth || 0))
   const fontSize = useSelector(state => state.fontSize)
 
-  const { isHovering, dropTarget } = useDragAndDropSubThought({ path, simplePath })
+  const { isHovering, isOver, dropTarget } = useDragAndDropSubThought({ path, simplePath })
 
   // Calculate the height for the child thought over cliff
   // If the thought is at a cliff (cliff < 0) or is the last visible thought, calculate the drop target height
@@ -62,6 +62,7 @@ const DropChild = ({ depth, path, simplePath, cliff, isLastVisible }: DropChildP
           }),
         )}
         ref={dndRef(dropTarget)}
+        data-drop-over={isOver}
         style={{
           height: `${0.5 + dropTargetHeight}em`,
           // Adjust position to compensate for cliff padding on parent thought

src/components/DropEnd.tsx

@@ -58,7 +58,7 @@ const DropEnd = ({
     attributeEquals(state, head(rootedParentOf(state, path)), '=view', 'Table'),
   )
 
-  const { isHovering, dropTarget } = useDragAndDropSubThought({ path })
+  const { isHovering, isOver, dropTarget } = useDragAndDropSubThought({ path })
 
   // a boolean indicating if the drop-hover component is shown
   // true if hovering and the context is not sorted
@@ -128,6 +128,7 @@ const DropEnd = ({
         transform: `translateX(${DROPEND_FINGERSHIFT}em)`,
       }}
       ref={dndRef(dropTarget)}
+      data-drop-over={isOver}
     >
       {testFlags.simulateDrop && (
         <span

src/components/DropUncle.tsx

@@ -31,7 +31,7 @@ const DropUncle = ({
     testFlags.simulateDrop ? getThoughtById(state, head(simplePath))?.value || '' : '',
   )
 
-  const { isHovering, dropTarget } = useDragAndDropThought({
+  const { isHovering, isOver, dropTarget } = useDragAndDropThought({
     path,
     simplePath,
     hoverZone: DropThoughtZone.SubthoughtsDrop,
@@ -54,6 +54,7 @@ const DropUncle = ({
       )}
       style={{ height: `${1.9 + dropTargetHeight}em` }}
       ref={dndRef(dropTarget)}
+      data-drop-over={isOver}
     >
       {testFlags.simulateDrop && (
         <span

src/components/Thought.tsx

@@ -295,6 +295,7 @@ const ThoughtContainer = ({
     dragSourceBullet,
     dragSourceEditable,
     isHovering,
+    isOver,
     dropTarget,
     canDropThought,
     isDeepHovering,
@@ -570,6 +571,7 @@ const ThoughtContainer = ({
     <div
       ref={dndRef(node => dragPreview(dropTarget(node)))}
       aria-label='child'
+      data-drop-over={isOver}
       data-divider={isDivider(value)}
       data-editing={isEditing}
       onClick={isTouch ? undefined : handleMultiselect}

src/e2e/puppeteer/__tests__/drag-and-drop.ts

@@ -21,7 +21,6 @@ import setSelection from '../helpers/setSelection'
 import simulateDragAndDrop from '../helpers/simulateDragAndDrop'
 import waitForAlert from '../helpers/waitForAlert'
 import waitForEditable from '../helpers/waitForEditable'
-import waitUntil from '../helpers/waitUntil'
 import withDebugLog from '../helpers/withDebugLog'
 import { page } from '../session'
 
@@ -134,10 +133,6 @@ describe('drag', () => {
     // hover the drop target directly below aaa, i.e. aaa's own position
     await dragAndDropThought('aaa', 'aaa', { hold: true, position: 'after' })
 
-    // the drop hover is still shown at the thought's own position; only the drop is cancelled
-    // (.drop-hover is the class that dropHoverRecipe gives every drop hover bar)
-    await waitUntil(() => !!document.querySelector('.drop-hover'), { timeout: 6000 })
-
     // release on the same drop target
     await dragAndDropThought('aaa', 'aaa', { position: 'after', skipMouseDown: true })
 

src/e2e/puppeteer/helpers/dragAndDropThought.ts

@@ -5,7 +5,77 @@ import hide from './hide'
 import showMousePointer from './showMousePointer'
 import waitUntil from './waitUntil'
 
-/** Performs Drag and Drop functionality on a thought in Puppeteer browser. */
+/** The page-side record of whether react-dnd has hovered the drop target under the drop point. */
+interface DropTargetHoverWatch {
+  /** Returns true if react-dnd hovers the drop target under the drop point now. */
+  check: () => boolean
+  observer: MutationObserver
+  hovered: boolean
+}
+
+declare global {
+  interface Window {
+    /** Set by dragAndDropThought for the length of a move. */
+    __dropTargetHoverWatch?: DropTargetHoverWatch
+  }
+}
+
+/**
+ * Starts recording whether react-dnd hovers the drop target under the given point: the innermost element with
+ * data-drop-over, which every drop target element renders from react-dnd's isOver({ shallow: true }). That is the
+ * target a drop there lands on. It is set in the same commit as the target's isHovering, from which DropChild, DropEnd
+ * and DropUncle draw their drop hover. A thought row's drop hover also needs state.hoveringPath, which the same hover
+ * writes before it whenever the pointer has moved. It is true even on a target that rejects the drop, so this also
+ * covers a drop that shows no drop hover.
+ *
+ * The hover is recorded as it is rendered rather than polled for. A drag driven by Puppeteer dispatches dragover only
+ * when the pointer moves, so when a hover changes the layout under the stationary pointer, e.g. by expanding a
+ * thought, nothing hovers the drop target that is under it afterwards, and a poll would wait for it forever.
+ */
+const watchDropTargetHover = (point: { x: number; y: number }) =>
+  page.evaluate(({ x, y }) => {
+    window.__dropTargetHoverWatch?.observer.disconnect()
+
+    /** Returns true if react-dnd hovers the drop target under the point. */
+    const check = () =>
+      document.elementFromPoint(x, y)?.closest('[data-drop-over]')?.getAttribute('data-drop-over') === 'true'
+
+    const watch: DropTargetHoverWatch = {
+      check,
+      observer: new MutationObserver(() => {
+        watch.hovered = watch.hovered || check()
+      }),
+      hovered: false,
+    }
+    watch.observer.observe(document.body, { attributes: true, attributeFilter: ['data-drop-over'], subtree: true })
+    window.__dropTargetHoverWatch = watch
+  }, point)
+
+/** Waits until react-dnd has hovered the drop target under the point since watchDropTargetHover, or hovers it now, which covers a target that was already hovered before the move. Throws if it never does. */
+const waitForDropTargetHover = async (point: { x: number; y: number }) => {
+  try {
+    await page.waitForFunction(
+      () => !!window.__dropTargetHoverWatch?.hovered || !!window.__dropTargetHoverWatch?.check(),
+      { timeout: 5000 },
+    )
+  } catch (e) {
+    const targetFound = await page.evaluate(
+      ({ x, y }) => !!document.elementFromPoint(x, y)?.closest('[data-drop-over]'),
+      point,
+    )
+    throw new Error(
+      targetFound
+        ? 'The drop target under the pointer was never hovered.'
+        : `There is no drop target under the pointer at (${point.x}, ${point.y}).`,
+      { cause: e },
+    )
+  }
+}
+
+/** Stops recording the hover. */
+const unwatchDropTargetHover = () => page.evaluate(() => window.__dropTargetHoverWatch?.observer.disconnect())
+
+/** Performs Drag and Drop functionality on a thought in Puppeteer browser. Waits for the drop target under the pointer to render its hover before holding or dropping, so callers do not need to synchronize with drag-and-drop themselves. */
 const dragAndDropThought = async (
   sourceValue: string,
   /** The value of the thought to drop at. Use the position absolute to control whether to drop before, after, etc. If null, only initiates the drag and move the mouse. */
@@ -92,12 +162,22 @@ const dragAndDropThought = async (
       y: dropUncle ? dragEnd.y + dragEnd.height : dragEnd.y + dragEnd.height / 2 + yOffset,
     }
 
-    // A single mousemove is not registered by react-dnd when continuing an existing drag (skipMouseDown), so issue a second move one pixel away first.
-    // Both events land on the drop target, so no other drop targets are hovered in between.
-    if (skipMouseDown) {
-      await page.mouse.move(dropPosition.x, dropPosition.y - 1)
+    await watchDropTargetHover(dropPosition)
+    try {
+      // A single mousemove is not registered by react-dnd when continuing an existing drag (skipMouseDown), so issue a second move one pixel away first.
+      // Both events land on the drop target, so no other drop targets are hovered in between.
+      if (skipMouseDown) {
+        await page.mouse.move(dropPosition.x, dropPosition.y - 1)
+      }
+      await page.mouse.move(dropPosition.x, dropPosition.y)
+
+      // The drop targets other than the thought rows (DropChild, DropEnd, DropUncle) mount only once the drag is in
+      // progress, so the drop target under the pointer is only known after it.
+      await page.locator('[data-drag-in-progress="true"]').wait()
+      await waitForDropTargetHover(dropPosition)
+    } finally {
+      await unwatchDropTargetHover()
     }
-    await page.mouse.move(dropPosition.x, dropPosition.y)
   } else {
     // Trigger drag state when no explicit drop target is provided.
     // Without this move, drag-and-drop alert visibility is flaky in CI.

src/hooks/useDragAndDropSubThought.tsx

@@ -282,8 +282,8 @@ const drop = (props: DroppableSubthoughts, monitor: DropTargetMonitor) => {
 /** Creates the props for drop. */
 const dropCollect = (monitor: DropTargetMonitor) => ({
   isHovering: monitor.isOver({ shallow: true }) && monitor.canDrop(),
-  // is being hovered over current thought irrespective of whether the given item is droppable
-  isBeingHoveredOver: monitor.isOver({ shallow: true }),
+  // hovered irrespective of whether the dragged thought can be dropped here
+  isOver: monitor.isOver({ shallow: true }),
   isDeepHovering: monitor.isOver(),
   canDropThought: monitor.canDrop(),
 })
@@ -292,7 +292,7 @@ const dropCollect = (monitor: DropTargetMonitor) => ({
 const useDragAndDropSubThought = (props: DroppableSubthoughts) => {
   const dispatch = useDispatch()
 
-  const [{ isHovering, isDeepHovering, canDropThought }, dropTarget] = useDrop({
+  const [{ isHovering, isOver, isDeepHovering, canDropThought }, dropTarget] = useDrop({
     accept: [DragAndDropType.Thought, NativeTypes.FILE],
     canDrop: (item, monitor) => !dropRejection(props, monitor),
     drop: (item, monitor) => drop(props, monitor),
@@ -338,7 +338,7 @@ const useDragAndDropSubThought = (props: DroppableSubthoughts) => {
 
   const isHoveringPinned = usePinDropHover(isHovering)
 
-  return { isHovering: isHoveringPinned, isDeepHovering, dropTarget }
+  return { isHovering: isHoveringPinned, isOver, isDeepHovering, dropTarget }
 }
 
 export default useDragAndDropSubThought

src/hooks/useDragAndDropThought.tsx

@@ -355,6 +355,8 @@ const dragCollect = (monitor: DragSourceMonitor) => ({
 /** Collects props from the DropTarget. */
 const dropCollect = (monitor: DropTargetMonitor) => ({
   isHovering: monitor.isOver({ shallow: true }) && monitor.canDrop(),
+  // hovered irrespective of whether the dragged thought can be dropped here
+  isOver: monitor.isOver({ shallow: true }),
   isDeepHovering: monitor.isOver(),
   canDropThought: monitor.canDrop(),
 })
@@ -380,7 +382,7 @@ const useDragAndDropThought = (props: Partial<ThoughtContainerProps> & { hoverZo
     collect: dragCollect,
   })
 
-  const [{ isHovering, isDeepHovering, canDropThought }, dropTarget] = useDrop({
+  const [{ isHovering, isOver, isDeepHovering, canDropThought }, dropTarget] = useDrop({
     accept: [DragAndDropType.Thought, NativeTypes.FILE],
     canDrop: (item, monitor) => !dropRejection(propsTypes, monitor),
     drop: (item, monitor) => drop(propsTypes, monitor),
@@ -439,6 +441,7 @@ const useDragAndDropThought = (props: Partial<ThoughtContainerProps> & { hoverZo
     dragSourceEditable,
     dragPreview,
     isHovering: isHoveringPinned,
+    isOver,
     isDeepHovering,
     canDropThought,
     dropTarget,