Merge pull request #35 from refactoringhq/task/drag-drop-tab-order

fix: drag-and-drop tab order persistence

src/components/TabBar.test.tsx

@@ -132,6 +132,71 @@ describe('TabBar', () => {
     expect(screen.getAllByTestId('tab-modified-indicator')).toHaveLength(3)
   })
 
+  it('does not reorder on drag cancel (dragEnd without drop)', () => {
+    const onReorderTabs = vi.fn()
+    const tabs = makeTabs(['Alpha', 'Beta', 'Gamma'])
+    render(
+      <TabBar
+        tabs={tabs}
+        activeTabPath={tabs[0].entry.path}
+        {...defaultProps}
+        onReorderTabs={onReorderTabs}
+      />
+    )
+
+    const alphaTab = screen.getByText('Alpha').closest('[draggable]')!
+    const betaTab = screen.getByText('Beta').closest('[draggable]')!
+
+    fireEvent.dragStart(alphaTab, {
+      dataTransfer: { effectAllowed: 'move', setData: vi.fn() },
+    })
+
+    const rect = betaTab.getBoundingClientRect()
+    fireEvent.dragOver(betaTab, {
+      clientX: rect.left + rect.width * 0.75,
+      dataTransfer: { dropEffect: 'move' },
+    })
+
+    // Cancel via dragEnd (Escape or release outside tab bar)
+    fireEvent.dragEnd(alphaTab)
+
+    expect(onReorderTabs).not.toHaveBeenCalled()
+  })
+
+  it('reorders from last toward first position', () => {
+    const onReorderTabs = vi.fn()
+    const tabs = makeTabs(['Alpha', 'Beta', 'Gamma'])
+    render(
+      <TabBar
+        tabs={tabs}
+        activeTabPath={tabs[2].entry.path}
+        {...defaultProps}
+        onReorderTabs={onReorderTabs}
+      />
+    )
+
+    const gammaTab = screen.getByText('Gamma').closest('[draggable]')!
+    const alphaTab = screen.getByText('Alpha').closest('[draggable]')!
+
+    fireEvent.dragStart(gammaTab, {
+      dataTransfer: { effectAllowed: 'move', setData: vi.fn() },
+    })
+
+    // jsdom returns zero-sized rects, so clientX always hits "right half"
+    // (insert after index 0 → insert index 1). This still validates
+    // that dragging the last tab toward the front produces a reorder.
+    const rect = alphaTab.getBoundingClientRect()
+    fireEvent.dragOver(alphaTab, {
+      clientX: rect.left + rect.width * 0.75,
+      dataTransfer: { dropEffect: 'move' },
+    })
+
+    fireEvent.drop(alphaTab, { dataTransfer: {} })
+
+    // Gamma (2) dragged onto Alpha (0) → reorder from 2 to 1
+    expect(onReorderTabs).toHaveBeenCalledWith(2, 1)
+  })
+
   it('switches tab on click', () => {
     const onSwitchTab = vi.fn()
     const tabs = makeTabs(['Alpha', 'Beta'])

src/components/TabBar.tsx

@@ -100,16 +100,25 @@ function computeInsertIndex(e: React.DragEvent<HTMLDivElement>, index: number):
 function useTabDrag(onReorderTabs?: (from: number, to: number) => void) {
   const [dragIndex, setDragIndex] = useState<number | null>(null)
   const [dropIndex, setDropIndex] = useState<number | null>(null)
+  // Refs mirror state so event handlers always read the latest values,
+  // avoiding stale closures when dragover and drop fire in the same frame.
+  const dragIndexRef = useRef<number | null>(null)
+  const dropIndexRef = useRef<number | null>(null)
   const dragNodeRef = useRef<HTMLDivElement | null>(null)
+  const onReorderRef = useRef(onReorderTabs)
+  useEffect(() => { onReorderRef.current = onReorderTabs })
 
   const resetDrag = useCallback(() => {
     if (dragNodeRef.current) dragNodeRef.current.style.opacity = ''
     dragNodeRef.current = null
+    dragIndexRef.current = null
+    dropIndexRef.current = null
     setDragIndex(null)
     setDropIndex(null)
   }, [])
 
   const handleDragStart = useCallback((e: React.DragEvent<HTMLDivElement>, index: number) => {
+    dragIndexRef.current = index
     setDragIndex(index)
     e.dataTransfer.effectAllowed = 'move'
     e.dataTransfer.setData('text/plain', String(index))
@@ -122,20 +131,32 @@ function useTabDrag(onReorderTabs?: (from: number, to: number) => void) {
   const handleDragOver = useCallback((e: React.DragEvent<HTMLDivElement>, index: number) => {
     e.preventDefault()
     e.dataTransfer.dropEffect = 'move'
-    if (dragIndex === null || dragIndex === index) { setDropIndex(null); return }
-    setDropIndex(computeInsertIndex(e, index))
-  }, [dragIndex])
+    const currentDrag = dragIndexRef.current
+    if (currentDrag === null || currentDrag === index) {
+      dropIndexRef.current = null
+      setDropIndex(null)
+      return
+    }
+    const idx = computeInsertIndex(e, index)
+    dropIndexRef.current = idx
+    setDropIndex(idx)
+  }, [])
 
   const handleDrop = useCallback((e: React.DragEvent<HTMLDivElement>) => {
     e.preventDefault()
-    const toIndex = computeDropTarget(dragIndex, dropIndex)
-    if (toIndex !== null && onReorderTabs) onReorderTabs(dragIndex!, toIndex)
+    const toIndex = computeDropTarget(dragIndexRef.current, dropIndexRef.current)
+    if (toIndex !== null && onReorderRef.current) {
+      onReorderRef.current(dragIndexRef.current!, toIndex)
+    }
     resetDrag()
-  }, [dragIndex, dropIndex, onReorderTabs, resetDrag])
+  }, [resetDrag])
 
   const handleBarDragLeave = useCallback((e: React.DragEvent<HTMLDivElement>) => {
     const related = e.relatedTarget as HTMLElement | null
-    if (!e.currentTarget.contains(related)) setDropIndex(null)
+    if (!e.currentTarget.contains(related)) {
+      dropIndexRef.current = null
+      setDropIndex(null)
+    }
   }, [])
 
   return { dragIndex, dropIndex, handleDragStart, handleDragEnd: resetDrag, handleDragOver, handleDrop, handleBarDragLeave }

src/hooks/useTabManagement.test.ts

@@ -205,6 +205,28 @@ describe('useTabManagement', () => {
       expect(result.current.tabs.map(t => t.entry.title)).toEqual(['C', 'A', 'B'])
     })
 
+    it('preserves active tab after reorder', async () => {
+      const { result } = renderHook(() => useTabManagement())
+
+      await act(async () => {
+        await result.current.handleSelectNote(makeEntry({ path: '/vault/a.md', title: 'A' }))
+      })
+      await act(async () => {
+        await result.current.handleSelectNote(makeEntry({ path: '/vault/b.md', title: 'B' }))
+      })
+      await act(async () => {
+        await result.current.handleSelectNote(makeEntry({ path: '/vault/c.md', title: 'C' }))
+      })
+
+      // C is active (last opened). Move it to the front.
+      act(() => {
+        result.current.handleReorderTabs(2, 0)
+      })
+
+      expect(result.current.tabs.map(t => t.entry.title)).toEqual(['C', 'A', 'B'])
+      expect(result.current.activeTabPath).toBe('/vault/c.md')
+    })
+
     it('persists tab order to localStorage', async () => {
       const { result } = renderHook(() => useTabManagement())