From 48d878b1031dec4f96dec973d99e82b159bdde47 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 3 Aug 2026 20:06:20 -0700 Subject: [PATCH] fix(tabs): keep deferred divider commits coherent --- .../TabGroupSplitLayout.drag.test.tsx | 122 ++++++++++++++++-- .../tab-group/TabGroupSplitLayout.tsx | 30 +++-- src/renderer/src/store/slices/tabs.test.ts | 17 ++- src/renderer/src/store/slices/tabs.ts | 10 +- 4 files changed, 144 insertions(+), 35 deletions(-) diff --git a/src/renderer/src/components/tab-group/TabGroupSplitLayout.drag.test.tsx b/src/renderer/src/components/tab-group/TabGroupSplitLayout.drag.test.tsx index 50aa211d343..7303b9f4f33 100644 --- a/src/renderer/src/components/tab-group/TabGroupSplitLayout.drag.test.tsx +++ b/src/renderer/src/components/tab-group/TabGroupSplitLayout.drag.test.tsx @@ -54,19 +54,49 @@ function pointerEvent(type: string, init: { pointerId?: number; clientX?: number describe('TabGroupSplitLayout divider drag', () => { let container: HTMLDivElement - let root: Root + let root: Root | null let handle: HTMLElement let firstPane: HTMLElement let secondPane: HTMLElement + let containerRect: DOMRect + let resizeObserverCallback: ResizeObserverCallback | null + const resizeObserverDisconnectMock = vi.fn() beforeEach(async () => { setTabGroupSplitRatioMock.mockClear() recordFeatureInteractionMock.mockClear() + resizeObserverDisconnectMock.mockClear() + resizeObserverCallback = null + vi.stubGlobal( + 'ResizeObserver', + class { + private readonly callback: ResizeObserverCallback + private tracksSplitContainer = false + + constructor(callback: ResizeObserverCallback) { + this.callback = callback + } + + observe(target: Element): void { + if (target === handle?.parentElement) { + this.tracksSplitContainer = true + resizeObserverCallback = this.callback + } + } + unobserve(): void {} + disconnect(): void { + if (this.tracksSplitContainer) { + resizeObserverDisconnectMock() + } + } + } + ) container = document.createElement('div') document.body.appendChild(container) - root = createRoot(container) + const mountedRoot = createRoot(container) + root = mountedRoot await act(async () => { - root.render( + mountedRoot.render( { expect(handle).not.toBeNull() firstPane = handle.previousElementSibling as HTMLElement secondPane = handle.nextElementSibling as HTMLElement - let captured = false + const capturedPointers = new Set() Object.assign(handle, { - setPointerCapture: () => { - captured = true + setPointerCapture: (pointerId: number) => { + capturedPointers.add(pointerId) }, - releasePointerCapture: () => { - captured = false + releasePointerCapture: (pointerId: number) => { + capturedPointers.delete(pointerId) }, - hasPointerCapture: () => captured + hasPointerCapture: (pointerId: number) => capturedPointers.has(pointerId) }) const splitContainer = handle.parentElement as HTMLElement - splitContainer.getBoundingClientRect = () => - ({ left: 0, top: 0, width: 1000, height: 500, right: 1000, bottom: 500 }) as DOMRect + containerRect = { + left: 0, + top: 0, + width: 1000, + height: 500, + right: 1000, + bottom: 500 + } as DOMRect + splitContainer.getBoundingClientRect = vi.fn(() => containerRect) }) afterEach(() => { - act(() => root.unmount()) + act(() => root?.unmount()) container.remove() + vi.unstubAllGlobals() }) it('writes pane styles per move and commits the store once on release', async () => { @@ -145,4 +183,64 @@ describe('TabGroupSplitLayout divider drag', () => { expect(setTabGroupSplitRatioMock).toHaveBeenCalledTimes(1) expect(setTabGroupSplitRatioMock).toHaveBeenCalledWith('wt-1', '', 0.15) }) + + it('refreshes drag geometry only when the split container resizes', async () => { + const getRectMock = handle.parentElement?.getBoundingClientRect as ReturnType + await act(async () => { + handle.dispatchEvent(pointerEvent('pointerdown', { pointerId: 1 })) + handle.dispatchEvent(pointerEvent('pointermove', { pointerId: 1, clientX: 250 })) + }) + expect(firstPane.style.flex).toBe('0.25 1 0%') + expect(getRectMock).toHaveBeenCalledOnce() + + containerRect = { ...containerRect, width: 500, right: 500 } as DOMRect + resizeObserverCallback?.([], {} as ResizeObserver) + await act(async () => { + handle.dispatchEvent(pointerEvent('pointermove', { pointerId: 1, clientX: 250 })) + handle.dispatchEvent(pointerEvent('pointerup', { pointerId: 1 })) + }) + + expect(firstPane.style.flex).toBe('0.5 1 0%') + expect(getRectMock).toHaveBeenCalledTimes(2) + expect(setTabGroupSplitRatioMock).toHaveBeenCalledWith('wt-1', '', 0.5) + expect(resizeObserverDisconnectMock).toHaveBeenCalledOnce() + }) + + it('ignores events from pointers that do not own the drag', async () => { + await act(async () => { + handle.dispatchEvent(pointerEvent('pointerdown', { pointerId: 1 })) + handle.dispatchEvent(pointerEvent('pointermove', { pointerId: 2, clientX: 300 })) + handle.dispatchEvent(pointerEvent('pointerup', { pointerId: 2 })) + }) + expect(firstPane.style.flex).toBe('0.5 1 0%') + expect(setTabGroupSplitRatioMock).not.toHaveBeenCalled() + + await act(async () => { + handle.dispatchEvent(pointerEvent('pointermove', { pointerId: 1, clientX: 350 })) + handle.dispatchEvent(pointerEvent('pointerup', { pointerId: 1 })) + }) + expect(setTabGroupSplitRatioMock).toHaveBeenCalledWith('wt-1', '', 0.35) + }) + + it.each(['pointercancel', 'lostpointercapture'])('commits on %s', async (eventType) => { + await act(async () => { + handle.dispatchEvent(pointerEvent('pointerdown', { pointerId: 1 })) + handle.dispatchEvent(pointerEvent('pointermove', { pointerId: 1, clientX: 400 })) + handle.dispatchEvent(pointerEvent(eventType, { pointerId: 1 })) + }) + expect(setTabGroupSplitRatioMock).toHaveBeenCalledWith('wt-1', '', 0.4) + expect(resizeObserverDisconnectMock).toHaveBeenCalledOnce() + }) + + it('commits and releases drag resources on unmount', async () => { + await act(async () => { + handle.dispatchEvent(pointerEvent('pointerdown', { pointerId: 1 })) + handle.dispatchEvent(pointerEvent('pointermove', { pointerId: 1, clientX: 450 })) + }) + act(() => root?.unmount()) + root = null + + expect(setTabGroupSplitRatioMock).toHaveBeenCalledWith('wt-1', '', 0.45) + expect(resizeObserverDisconnectMock).toHaveBeenCalledOnce() + }) }) diff --git a/src/renderer/src/components/tab-group/TabGroupSplitLayout.tsx b/src/renderer/src/components/tab-group/TabGroupSplitLayout.tsx index 8c0c200a3b9..254e1f1c46c 100644 --- a/src/renderer/src/components/tab-group/TabGroupSplitLayout.tsx +++ b/src/renderer/src/components/tab-group/TabGroupSplitLayout.tsx @@ -48,13 +48,16 @@ function ResizeHandle({ onResizeStart() setDragging(true) handle.setPointerCapture(event.pointerId) - // Why: measure once — the container can't resize mid-drag, and per-move - // getBoundingClientRect forces a reflow against freshly written styles. - const rect = container.getBoundingClientRect() + // Why: measure outside pointermove so pane writes never force a readback. + let rect = container.getBoundingClientRect() + const resizeObserver = new ResizeObserver(() => { + rect = container.getBoundingClientRect() + }) + resizeObserver.observe(container) let draggedRatio: number | null = null const onPointerMove = (moveEvent: PointerEvent): void => { - if (!handle.hasPointerCapture(event.pointerId)) { + if (moveEvent.pointerId !== event.pointerId || !handle.hasPointerCapture(event.pointerId)) { return } const ratio = isHorizontal @@ -75,6 +78,7 @@ function ResizeHandle({ return } cleaned = true + resizeObserver.disconnect() if (draggedRatio !== null) { onRatioChange(draggedRatio) } @@ -97,16 +101,22 @@ function ResizeHandle({ } } - const onPointerUp = (): void => { - cleanup() + const onPointerUp = (upEvent: PointerEvent): void => { + if (upEvent.pointerId === event.pointerId) { + cleanup() + } } - const onPointerCancel = (): void => { - cleanup() + const onPointerCancel = (cancelEvent: PointerEvent): void => { + if (cancelEvent.pointerId === event.pointerId) { + cleanup() + } } - const onLostPointerCapture = (): void => { - cleanup() + const onLostPointerCapture = (lostEvent: PointerEvent): void => { + if (lostEvent.pointerId === event.pointerId) { + cleanup() + } } handle.addEventListener('pointermove', onPointerMove) diff --git a/src/renderer/src/store/slices/tabs.test.ts b/src/renderer/src/store/slices/tabs.test.ts index 16005a2bc79..c7bab81ff81 100644 --- a/src/renderer/src/store/slices/tabs.test.ts +++ b/src/renderer/src/store/slices/tabs.test.ts @@ -962,18 +962,23 @@ describe('TabsSlice', () => { } } }) - const before = store.getState().layoutByWorktree + const beforeState = store.getState() + const beforeLayout = beforeState.layoutByWorktree + const subscriber = vi.fn() + const unsubscribe = store.subscribe(subscriber) // Why: an unchanged commit must not mint fresh root state — every store // subscriber wakes on the new reference (STA-3328). store.getState().setTabGroupSplitRatio(WT, '', 0.5) - expect(store.getState().layoutByWorktree).toBe(before) + expect(store.getState()).toBe(beforeState) + expect(store.getState().layoutByWorktree).toBe(beforeLayout) + expect(subscriber).not.toHaveBeenCalled() store.getState().setTabGroupSplitRatio(WT, '', 0.5004) - expect(store.getState().layoutByWorktree).toBe(before) - - store.getState().setTabGroupSplitRatio(WT, '', 0.7) - expect(store.getState().layoutByWorktree).not.toBe(before) + expect(store.getState().layoutByWorktree).not.toBe(beforeLayout) + expect(subscriber).toHaveBeenCalledOnce() + expect((store.getState().layoutByWorktree[WT] as { ratio: number }).ratio).toBe(0.5004) + unsubscribe() }) }) diff --git a/src/renderer/src/store/slices/tabs.ts b/src/renderer/src/store/slices/tabs.ts index f4bb58c54c8..e4abdd85d57 100644 --- a/src/renderer/src/store/slices/tabs.ts +++ b/src/renderer/src/store/slices/tabs.ts @@ -1790,7 +1790,7 @@ export const createTabsSlice: StateCreator = (set, set((state) => { const currentLayout = state.layoutByWorktree[worktreeId] if (!currentLayout) { - return {} + return state } // Why: an unchanged ratio must not mint fresh root state — every store // subscriber wakes on the new reference (STA-3328). @@ -1801,12 +1801,8 @@ export const createTabsSlice: StateCreator = (set, ? targetNode[segment] : undefined } - if ( - targetNode && - targetNode.type === 'split' && - Math.abs((targetNode.ratio ?? 0.5) - ratio) < 0.0005 - ) { - return {} + if (!targetNode || targetNode.type !== 'split' || (targetNode.ratio ?? 0.5) === ratio) { + return state } return { layoutByWorktree: {