From b81f06203df81e36b0874ee56e8f4b30a734b7e2 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 12 Jun 2026 12:17:50 -0700 Subject: [PATCH] Defer project header pointer capture until drag threshold is crossed (#5278) This allows simple clicks on the repository header to still reach the inner collapse handler on pointerup. Additionally, body cursor and user-select styles are only applied when actual dragging begins, rather than when the session is armed. --- .../src/components/sidebar/WorktreeList.tsx | 20 ++- .../sidebar/project-header-drag-contract.ts | 13 ++ .../sidebar/project-header-drag-start.test.ts | 78 +++++++++++ .../sidebar/project-header-drag-start.ts | 12 +- .../sidebar/project-header-drag.test.ts | 121 +++++++++++++++++- .../components/sidebar/project-header-drag.ts | 15 ++- 6 files changed, 241 insertions(+), 18 deletions(-) create mode 100644 src/renderer/src/components/sidebar/project-header-drag-start.test.ts diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 92c3dda74..beac56c2d 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -3034,11 +3034,6 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp ? undefined : { transform: getVirtualRowTransform(vItem.start) } } - onPointerDown={ - isDraggableRepoHeader && projectIdForHeader - ? (event) => repoDrag.onHandlePointerDown(event, projectIdForHeader) - : undefined - } >
-
+
repoDrag.onHandlePointerDown(event, projectIdForHeader) + : undefined + } + > {row.label}
diff --git a/src/renderer/src/components/sidebar/project-header-drag-contract.ts b/src/renderer/src/components/sidebar/project-header-drag-contract.ts index 6d823e8db..18662d5f2 100644 --- a/src/renderer/src/components/sidebar/project-header-drag-contract.ts +++ b/src/renderer/src/components/sidebar/project-header-drag-contract.ts @@ -45,9 +45,22 @@ export type ProjectHeaderDragSession = { export const PROJECT_HEADER_DRAG_THRESHOLD_PX = 4 +const REPO_HEADER_DRAG_HANDLE_SELECTOR = '[data-repo-header-drag-handle]' + const REPO_HEADER_ACTION_SELECTOR = '[data-repo-header-action], button, a, input, textarea, select, [contenteditable=""], [contenteditable="true"]' +export function isProjectHeaderDragHandleTarget( + target: EventTarget | null, + currentTarget: HTMLElement +): boolean { + if (!(target instanceof HTMLElement)) { + return false + } + const dragHandle = target.closest(REPO_HEADER_DRAG_HANDLE_SELECTOR) + return dragHandle !== null && currentTarget.contains(dragHandle) +} + export function isRepoHeaderActionTarget( target: EventTarget | null, currentTarget: HTMLElement diff --git a/src/renderer/src/components/sidebar/project-header-drag-start.test.ts b/src/renderer/src/components/sidebar/project-header-drag-start.test.ts new file mode 100644 index 000000000..2d042b3ad --- /dev/null +++ b/src/renderer/src/components/sidebar/project-header-drag-start.test.ts @@ -0,0 +1,78 @@ +// @vitest-environment happy-dom +import { describe, expect, it, vi } from 'vitest' + +import { createProjectHeaderDragSession } from './project-header-drag-start' +import type { Repo } from '../../../../shared/types' + +function createRepo(id: string, projectGroupId: string | null = null): Repo { + return { + id, + path: `/tmp/${id}`, + displayName: id, + badgeColor: '#000000', + addedAt: 0, + projectGroupId, + projectGroupOrder: 0 + } +} + +describe('createProjectHeaderDragSession', () => { + it('does not capture the pointer when arming a drag session', () => { + const handleEl = document.createElement('div') + handleEl.setAttribute('data-repo-header-drag-handle', '') + handleEl.setPointerCapture = vi.fn() + const scrollContainer = document.createElement('div') + document.body.append(scrollContainer, handleEl) + + const repoById = new Map([['repo-a', createRepo('repo-a')]]) + const sidebarRepoHeaderIdsByBucket = new Map([['ungrouped', ['repo-a', 'repo-b']]]) + + const session = createProjectHeaderDragSession({ + event: { + button: 0, + pointerId: 1, + clientX: 10, + clientY: 20, + target: handleEl, + currentTarget: handleEl + } as unknown as React.PointerEvent, + repoId: 'repo-a', + repoById, + sidebarRepoHeaderIdsByBucket, + getScrollContainer: () => scrollContainer + }) + + expect(session).not.toBeNull() + expect(handleEl.setPointerCapture).not.toHaveBeenCalled() + }) + + it('does not arm drag when the pointer starts outside the project name handle', () => { + const header = document.createElement('div') + const handleEl = document.createElement('div') + handleEl.setAttribute('data-repo-header-drag-handle', '') + const chevron = document.createElement('span') + header.append(handleEl, chevron) + const scrollContainer = document.createElement('div') + document.body.append(scrollContainer, header) + + const repoById = new Map([['repo-a', createRepo('repo-a')]]) + const sidebarRepoHeaderIdsByBucket = new Map([['ungrouped', ['repo-a', 'repo-b']]]) + + const session = createProjectHeaderDragSession({ + event: { + button: 0, + pointerId: 1, + clientX: 10, + clientY: 20, + target: chevron, + currentTarget: header + } as unknown as React.PointerEvent, + repoId: 'repo-a', + repoById, + sidebarRepoHeaderIdsByBucket, + getScrollContainer: () => scrollContainer + }) + + expect(session).toBeNull() + }) +}) diff --git a/src/renderer/src/components/sidebar/project-header-drag-start.ts b/src/renderer/src/components/sidebar/project-header-drag-start.ts index f0d96bab4..04c306a86 100644 --- a/src/renderer/src/components/sidebar/project-header-drag-start.ts +++ b/src/renderer/src/components/sidebar/project-header-drag-start.ts @@ -6,6 +6,7 @@ import { type ProjectHeaderDragBucketKey } from './project-header-drop' import { + isProjectHeaderDragHandleTarget, isRepoHeaderActionTarget, type ProjectHeaderDragSession } from './project-header-drag-contract' @@ -21,6 +22,9 @@ export function createProjectHeaderDragSession(args: { if (args.event.button !== 0) { return null } + if (!isProjectHeaderDragHandleTarget(args.event.target, args.event.currentTarget)) { + return null + } if (isRepoHeaderActionTarget(args.event.target, args.event.currentTarget)) { return null } @@ -40,12 +44,8 @@ export function createProjectHeaderDragSession(args: { return null } const handleEl = args.event.currentTarget - try { - handleEl.setPointerCapture(args.event.pointerId) - } catch { - // setPointerCapture can throw if the element is detached; the global - // pointer listeners still fire, so dragging keeps working. - } + // Why: defer setPointerCapture until the drag threshold is crossed so a + // header click still reaches the inner collapse handler on pointerup. return { repoId: args.repoId, bucketKey, diff --git a/src/renderer/src/components/sidebar/project-header-drag.test.ts b/src/renderer/src/components/sidebar/project-header-drag.test.ts index b53a4e868..ff251e7da 100644 --- a/src/renderer/src/components/sidebar/project-header-drag.test.ts +++ b/src/renderer/src/components/sidebar/project-header-drag.test.ts @@ -1,7 +1,32 @@ // @vitest-environment happy-dom -import { describe, expect, it } from 'vitest' +import { act, createElement } from 'react' +import { createRoot } from 'react-dom/client' +import { describe, expect, it, vi } from 'vitest' -import { isRepoHeaderActionTarget } from './project-header-drag' +import { + isProjectHeaderDragHandleTarget, + isRepoHeaderActionTarget, + useRepoHeaderDrag +} from './project-header-drag' +import type { Repo } from '../../../../shared/types' + +function createRepo(id: string, projectGroupId: string | null = null): Repo { + return { + id, + path: `/tmp/${id}`, + displayName: id, + badgeColor: '#000000', + addedAt: 0, + projectGroupId, + projectGroupOrder: 0 + } +} + +function createPointerEvent(type: string, init: MouseEventInit & { pointerId: number }): Event { + const event = new MouseEvent(type, { bubbles: true, ...init }) + Object.defineProperty(event, 'pointerId', { value: init.pointerId }) + return event +} function createHeader(markup: string): HTMLElement { const header = document.createElement('div') @@ -35,3 +60,95 @@ describe('repo header action targets', () => { expect(isRepoHeaderActionTarget(header, header)).toBe(false) }) }) + +describe('project header drag handle targets', () => { + it('accepts pointer events on the project name handle', () => { + const header = createHeader(` + Orca + + `) + + const handle = header.querySelector('#handle') as HTMLElement + expect(isProjectHeaderDragHandleTarget(handle, handle)).toBe(true) + }) + + it('rejects pointer events outside the project name handle', () => { + const header = createHeader(` + Orca + + `) + + expect(isProjectHeaderDragHandleTarget(header.querySelector('#chevron'), header)).toBe(false) + }) +}) + +describe('repo header drag pointer capture', () => { + it('captures the pointer only after crossing the drag threshold', async () => { + const scrollContainer = document.createElement('div') + document.body.appendChild(scrollContainer) + const repoById = new Map([ + ['repo-a', createRepo('repo-a')], + ['repo-b', createRepo('repo-b')] + ]) + const sidebarRepoHeaderIdsByBucket = new Map([['ungrouped', ['repo-a', 'repo-b']]]) + const setPointerCapture = vi.fn() + + function DragHarness(): React.ReactElement { + const repoDrag = useRepoHeaderDrag({ + orderedRepoIds: ['repo-a', 'repo-b'], + sidebarRepoHeaderIdsByBucket, + repoById, + usesProjectGroupOrdering: false, + onCommitRepoOrder: vi.fn(), + onCommitProjectGroupOrder: vi.fn(), + getScrollContainer: () => scrollContainer + }) + + return createElement('div', { + 'data-repo-header-drag-handle': '', + 'data-repo-header-id': 'repo-a', + 'data-repo-header-index': 0, + 'data-repo-header-bucket': 'ungrouped', + onPointerDown: (event: React.PointerEvent) => + repoDrag.onHandlePointerDown(event, 'repo-a'), + ref: (element: HTMLDivElement | null) => { + if (element) { + element.setPointerCapture = setPointerCapture + } + } + }) + } + + const root = createRoot(scrollContainer) + await act(async () => { + root.render(createElement(DragHarness)) + }) + const handle = scrollContainer.querySelector('[data-repo-header-drag-handle]') + expect(handle).not.toBeNull() + + await act(async () => { + handle!.dispatchEvent( + createPointerEvent('pointerdown', { button: 0, clientX: 10, clientY: 10, pointerId: 7 }) + ) + }) + expect(setPointerCapture).not.toHaveBeenCalled() + + await act(async () => { + window.dispatchEvent( + createPointerEvent('pointermove', { clientX: 12, clientY: 12, pointerId: 7 }) + ) + }) + expect(setPointerCapture).not.toHaveBeenCalled() + + await act(async () => { + window.dispatchEvent( + createPointerEvent('pointermove', { clientX: 20, clientY: 20, pointerId: 7 }) + ) + }) + expect(setPointerCapture).toHaveBeenCalledWith(7) + + await act(async () => { + root.unmount() + }) + }) +}) diff --git a/src/renderer/src/components/sidebar/project-header-drag.ts b/src/renderer/src/components/sidebar/project-header-drag.ts index bfc5eb296..0c424ebec 100644 --- a/src/renderer/src/components/sidebar/project-header-drag.ts +++ b/src/renderer/src/components/sidebar/project-header-drag.ts @@ -209,6 +209,12 @@ export function useRepoHeaderDrag({ return } session.promoted = true + try { + session.handleEl.setPointerCapture(session.pointerId) + } catch { + // setPointerCapture can throw if the element is detached; the global + // pointer listeners still fire, so dragging keeps working. + } refreshHeaderRects() setState({ draggingRepoId: session.repoId, dropIndex: null, dropIndicatorY: null }) } @@ -260,7 +266,7 @@ export function useRepoHeaderDrag({ }, [cancelAutoscroll, computeDrop, endDrag, ensureAutoscroll, refreshHeaderRects, sessionArmed]) useEffect(() => { - if (!sessionArmed) { + if (state.draggingRepoId === null) { return } const body = document.body @@ -272,7 +278,7 @@ export function useRepoHeaderDrag({ body.style.cursor = prevCursor body.style.userSelect = prevUserSelect } - }, [sessionArmed]) + }, [state.draggingRepoId]) const onHandlePointerDown = useCallback( (event: React.PointerEvent, repoId: string) => { @@ -295,4 +301,7 @@ export function useRepoHeaderDrag({ return { state, onHandlePointerDown } } -export { isRepoHeaderActionTarget } from './project-header-drag-contract' +export { + isRepoHeaderActionTarget, + isProjectHeaderDragHandleTarget +} from './project-header-drag-contract'