Only adjust the drag hoverCount on an actual hover transition

docs/drag-and-drop.md

@@ -135,6 +135,8 @@ When the press ends, `useLongPress` defers `onLongPressEnd` by 10 ms so that the
 
 [`useDragLeave`](../src/hooks/useDragLeave.ts) tracks how many drop targets are currently being deep-hovered (a module-level `hoverCount`). When the count drops to zero, it debounces a 50 ms clear of `state.hoveringPath`. This prevents flicker when the cursor briefly leaves one drop zone before entering an adjacent one.
 
+Because `hoverCount` is shared across every drop target, only a change in `isDeepHovering` may adjust it. The hook's effect also re-runs on mount and when `canDropThought` or `hoverZone` change, and treating those as hover transitions would let a thought mounting mid-drag decrement the count to zero and blank the drop indicator while a target is still hovered. A separate unmount-only effect releases a target's contribution to the count, so a thought the layout unmounts mid-drag doesn't leak one.
+
 ### `useDropHoverColor`
 
 [`useDropHoverColor`](../src/hooks/useDropHoverColor.ts) — small UI hook that maps the drop zone's depth to its hover color. Used by the various Drop* components.

src/hooks/__tests__/useDragLeave.ts

@@ -0,0 +1,88 @@
+import { renderHook } from '@testing-library/react'
+import { act, createElement } from 'react'
+import { Provider } from 'react-redux'
+import { importTextActionCreator as importText } from '../../actions/importText'
+import { updateHoveringPathActionCreator as updateHoveringPath } from '../../actions/updateHoveringPath'
+import contextToPath from '../../selectors/contextToPath'
+import store from '../../stores/app'
+import initStore from '../../test-helpers/initStore'
+import useDragLeave from '../useDragLeave'
+
+/** Renders useDragLeave against the app store. */
+const renderDragLeave = (props: { isDeepHovering: boolean; canDropThought: boolean }) =>
+  renderHook((propsNew: { isDeepHovering: boolean; canDropThought: boolean }) => useDragLeave(propsNew), {
+    initialProps: props,
+    wrapper: ({ children }) => createElement(Provider, { store, children }),
+  })
+
+/** Imports two thoughts and sets hoveringPath to the first, as if a drag were in progress over it. */
+const startHovering = () => {
+  store.dispatch(importText({ text: '- a\n- b' }))
+  store.dispatch(updateHoveringPath({ path: contextToPath(store.getState(), ['a'])! }))
+}
+
+/** Advances past the hook's 50ms debounce. */
+const flushDebounce = () => act(() => vi.advanceTimersByTimeAsync(100))
+
+beforeEach(() => {
+  initStore()
+  vi.useFakeTimers()
+})
+
+afterEach(() => {
+  vi.useRealTimers()
+})
+
+it('keeps hoveringPath while a drop target is hovered and an unrelated thought mounts', async () => {
+  startHovering()
+
+  // the cursor enters a drop target
+  const target = renderDragLeave({ isDeepHovering: false, canDropThought: true })
+  target.rerender({ isDeepHovering: true, canDropThought: true })
+
+  // an unrelated thought mounts while the drag is still over the target
+  renderDragLeave({ isDeepHovering: false, canDropThought: true })
+
+  await flushDebounce()
+
+  expect(store.getState().hoveringPath).toBeDefined()
+})
+
+// Guards against over-correcting the hover count: ignoring mounts must not leave the count stuck above zero, or
+// hoveringPath would never be cleared.
+it('still clears hoveringPath after an unrelated thought mounts and the cursor leaves', async () => {
+  startHovering()
+
+  const target = renderDragLeave({ isDeepHovering: false, canDropThought: true })
+  target.rerender({ isDeepHovering: true, canDropThought: true })
+  renderDragLeave({ isDeepHovering: false, canDropThought: true })
+  target.rerender({ isDeepHovering: false, canDropThought: true })
+
+  await flushDebounce()
+
+  expect(store.getState().hoveringPath).toBeUndefined()
+})
+
+it('clears hoveringPath once the cursor leaves the drop target', async () => {
+  startHovering()
+
+  const target = renderDragLeave({ isDeepHovering: false, canDropThought: true })
+  target.rerender({ isDeepHovering: true, canDropThought: true })
+  target.rerender({ isDeepHovering: false, canDropThought: true })
+
+  await flushDebounce()
+
+  expect(store.getState().hoveringPath).toBeUndefined()
+})
+
+it('clears hoveringPath when a hovered drop target unmounts', async () => {
+  startHovering()
+
+  const target = renderDragLeave({ isDeepHovering: false, canDropThought: true })
+  target.rerender({ isDeepHovering: true, canDropThought: true })
+  target.unmount()
+
+  await flushDebounce()
+
+  expect(store.getState().hoveringPath).toBeUndefined()
+})

src/hooks/useDragLeave.ts

@@ -21,7 +21,9 @@ const clearHoveringPath: Thunk = (dispatch, getState) => {
 const useDragLeave = ({ isDeepHovering, canDropThought }: { isDeepHovering: boolean; canDropThought: boolean }) => {
   const dispatch = useDispatch()
   const hoverZone = useSelector(state => state.hoverZone)
-  const prevIsDeepHoveringRef = useRef(isDeepHovering)
+  // Whether this drop target is currently counted in hoverCount. Starts false so that mounting contributes nothing
+  // until the cursor actually enters.
+  const isCountedRef = useRef(false)
   const prevHoverZone = useRef(hoverZone)
 
   // Initialize the debounced function if it hasn't been already
@@ -46,34 +48,46 @@ const useDragLeave = ({ isDeepHovering, canDropThought }: { isDeepHovering: bool
       return
     }
 
-    if (isDeepHovering && !prevIsDeepHoveringRef.current) {
-      // Cursor has entered a drop target, increase hover count
-      hoverCount += 1
-
-      // Cancel any pending debounce since we're over a drop target
-      debouncedSetHoveringPath?.cancel()
-    } else {
-      // Cursor has left a drop target, decrease hover count
-      hoverCount = Math.max(hoverCount - 1, 0)
-      if (hoverCount === 0) {
-        // No drop targets are being hovered over; start debounce
-        debouncedSetHoveringPath?.()
-      }
-    }
-
-    prevIsDeepHoveringRef.current = isDeepHovering
-    prevHoverZone.current = hoverZone
-
-    return () => {
-      // Cleanup on unmount
+    // Only a change in isDeepHovering means the cursor entered or left this drop target. The effect also re-runs when
+    // the component mounts and when canDropThought changes, and those must not touch the shared hoverCount: otherwise
+    // any thought mounting mid-drag decrements the count to zero and clears hoveringPath while a target is still
+    // hovered, dropping the drop indicator.
+    if (isDeepHovering !== isCountedRef.current) {
+      isCountedRef.current = isDeepHovering
       if (isDeepHovering) {
+        // Cursor has entered a drop target, increase hover count
+        hoverCount += 1
+
+        // Cancel any pending debounce since we're over a drop target
+        debouncedSetHoveringPath?.cancel()
+      } else {
+        // Cursor has left a drop target, decrease hover count
+        hoverCount = Math.max(hoverCount - 1, 0)
         if (hoverCount === 0) {
-          // Start debounce when unmounting and no more drop targets are hovered
+          // No drop targets are being hovered over; start debounce
           debouncedSetHoveringPath?.()
         }
       }
     }
+
+    prevHoverZone.current = hoverZone
   }, [isDeepHovering, dispatch, hoverZone, canDropThought])
+
+  // Release this drop target's contribution to hoverCount when it unmounts mid-drag, e.g. when the layout unmounts a
+  // thought that the cursor is over. Empty deps are load-bearing: React runs an effect's cleanup before every re-run,
+  // not only on unmount, so this cannot be folded into the effect above. It closes over refs and module state only,
+  // so it never goes stale.
+  useEffect(
+    () => () => {
+      if (!isCountedRef.current) return
+      isCountedRef.current = false
+      hoverCount = Math.max(hoverCount - 1, 0)
+      if (hoverCount === 0) {
+        debouncedSetHoveringPath?.()
+      }
+    },
+    [],
+  )
 }
 
 export default useDragLeave