fix(tabs): keep deferred divider commits coherent

This commit is contained in:
Brennan Benson
2026-08-03 20:06:20 -07:00
parent 97c038fe5e
commit 48d878b103
4 changed files with 144 additions and 35 deletions
@@ -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(
<TabGroupSplitLayout
layout={{
type: 'split',
@@ -85,24 +115,32 @@ describe('TabGroupSplitLayout divider drag', () => {
expect(handle).not.toBeNull()
firstPane = handle.previousElementSibling as HTMLElement
secondPane = handle.nextElementSibling as HTMLElement
let captured = false
const capturedPointers = new Set<number>()
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<typeof vi.fn>
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()
})
})
@@ -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)
+11 -6
View File
@@ -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()
})
})
+3 -7
View File
@@ -1790,7 +1790,7 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (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<AppState, [], [], TabsSlice> = (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: {