xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(sidebar): move project header grab cursor to title surface only (#11435) 

* fix(sidebar): move project header grab cursor to title surface only

Prevent grab cursor appearing over action buttons (…, +, chevron) which
should show cursor-pointer, not the reorder hand.

- Grab cursor scoped to icon + label surface only
- Row retains data-repo-header-drag-handle for indent/padding drag targets
- Actions excluded via [data-repo-header-actions] selector
- Add lockstep test to keep action selectors synchronized

* fix(sidebar): share project header action selector across drag contracts

Address Greptile feedback: drop the format-sensitive regex lockstep parse and
import one shared REPO_HEADER_ACTION_SELECTOR for repo and group headers.
This commit is contained in:
Jinjing 2026-07-29 17:03:58 -07:00 committed by GitHub
parent 5f7807497e
commit 4c65b42ee2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 380 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/renderer/src/components/sidebar/ProjectHeaderActions.test.tsx
View File

@ -35,5 +35,9 @@ describe('ProjectHeaderActions', () => {
expect(actions?.className).toContain('group-hover:pointer-events-auto')
expect(actions?.className).toContain('has-[:focus-visible]:pointer-events-auto')
expect(actions?.className).toContain('has-[button[data-state=open]]:pointer-events-auto')
// Why: title drag surface is cursor-grab; actions must override so hovering
// … / + never shows the reorder hand.
expect(actions?.className).toContain('cursor-pointer')
expect(actions?.className).toContain('self-stretch')
})
})

5
src/renderer/src/components/sidebar/ProjectHeaderActions.tsx
View File

@ -3,8 +3,11 @@ import { cn } from '@/lib/utils'
// Why: desktop hover actions should not permanently reserve project-title width;
// touch devices keep them in normal flow because there is no hover reveal.
// Why: title surface uses cursor-grab; force pointer here so … / + / chevron
// (and gaps) never show the reorder hand. self-stretch fills the row height in
// flow so the gutter beside buttons is still an action hit target, not a drag arm.
export const PROJECT_HEADER_ACTIONS_CLASS_NAME = cn(
'flex shrink-0 items-center gap-0.5',
'flex shrink-0 cursor-pointer items-center gap-0.5 self-stretch',
'can-hover:absolute can-hover:right-1 can-hover:top-1/2 can-hover:z-10 can-hover:-translate-y-1/2',
'can-hover:rounded-md can-hover:bg-worktree-sidebar can-hover:pl-1',
'can-hover:pointer-events-none can-hover:opacity-0 can-hover:transition-opacity',

82
src/renderer/src/components/sidebar/WorktreeList.tsx
View File

@ -4254,6 +4254,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
? repoHeaderSectionEndByRepoId.get(projectIdForHeader)
: undefined
}
// Why: row keeps handle attrs so indent/padding still arms drag; grab
// cursor lives only on the title surface so … / + never inherit it.
data-repo-header-drag-handle={isDraggableRepoHeader ? '' : undefined}
data-project-group-header-id={projectGroupIdForHeader}
data-project-group-header-index={projectGroupHeaderIndex}
@ -4270,10 +4272,10 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
data-workspace-status={headerWorkspaceStatus ?? undefined}
data-workspace-pin-drop-target={isPinnedHeader ? '' : undefined}
className={cn(
// Why: no row-level grab — only the title surface below shows the hand;
// actions use cursor-pointer so … / + never look reorderable.
'group relative flex h-7 w-full items-center gap-1.5 pr-2 text-left transition-all',
isDraggableRepoHeader || isDraggableProjectGroupHeader
? 'cursor-grab active:cursor-grabbing'
: 'cursor-pointer',
!(isDraggableRepoHeader || isDraggableProjectGroupHeader) && 'cursor-pointer',
highlightedRevealRowKey === row.key &&
'rounded-md bg-worktree-sidebar-accent ring-1 ring-worktree-sidebar-ring/50',
(isDraggingThis || isDraggingThisProjectGroup) &&
@ -4330,39 +4332,48 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
}
}}
>
{row.icon ? (
<div
data-repo-header-drag-handle={isDraggableRepoHeader ? '' : undefined}
data-project-group-header-drag-handle={
isDraggableProjectGroupHeader ? '' : undefined
}
className={cn(
'flex size-4 shrink-0 items-center justify-center rounded-[4px]',
repoHeaderColor ? 'text-muted-foreground' : row.tone,
(isDraggableRepoHeader || isDraggableProjectGroupHeader) &&
'hover:cursor-grab active:cursor-grabbing'
)}
>
{row.repo ? (
<RepoIconGlyph
repoIcon={row.repo.repoIcon}
color={repoHeaderColor}
className="size-4"
iconClassName="size-3.5"
/>
) : (
<row.icon className="size-3" />
)}
</div>
) : null}
<div className="min-w-0 flex-1">
<div className="flex min-w-0 items-center gap-1.5">
<div className="min-w-0 truncate text-[13px] font-semibold leading-none">
{row.label}
{/* Why: grab cursor on icon+title only. Row still has handle attrs so
indent/padding can arm drag; actions are excluded via data-repo-header-actions.
self-stretch fills h-7 so grab matches the full title column height. */}
<div
data-repo-header-drag-handle={isDraggableRepoHeader ? '' : undefined}
data-project-group-header-drag-handle={
isDraggableProjectGroupHeader ? '' : undefined
}
className={cn(
'flex min-w-0 flex-1 items-center gap-1.5 self-stretch',
(isDraggableRepoHeader || isDraggableProjectGroupHeader) &&
'cursor-grab active:cursor-grabbing'
)}
>
{row.icon ? (
<div
className={cn(
'flex size-4 shrink-0 items-center justify-center rounded-[4px]',
repoHeaderColor ? 'text-muted-foreground' : row.tone
)}
>
{row.repo ? (
<RepoIconGlyph
repoIcon={row.repo.repoIcon}
color={repoHeaderColor}
className="size-4"
iconClassName="size-3.5"
/>
) : (
<row.icon className="size-3" />
)}
</div>
) : null}
<div className="min-w-0 flex-1">
<div className="flex min-w-0 items-center gap-1.5">
<div className="min-w-0 truncate text-[13px] font-semibold leading-none">
{row.label}
</div>
<RepoForkIndicator upstream={row.repo?.upstream} />
<FolderPathStatusIndicator status={projectGroupPathStatus} />
</div>
<RepoForkIndicator upstream={row.repo?.upstream} />
<FolderPathStatusIndicator status={projectGroupPathStatus} />
</div>
</div>
@ -4705,6 +4716,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
)
}
onKeyDown={stopRepoHeaderKeyboardToggle}
onPointerDown={handleRepoHeaderActionPointerDown}
onClick={(event) => {
event.preventDefault()
event.stopPropagation()

25
src/renderer/src/components/sidebar/project-group-header-dom.test.ts
View File

@ -22,4 +22,29 @@ describe('Project Group header drag DOM source', () => {
expect(source).toContain('const updateProjectGroup = useAppStore((s) => s.updateProjectGroup)')
expect(source).toContain('void updateProjectGroup(groupId, { tabOrder })')
})
it('keeps grab cursor on the title surface and dual handle attrs on row + surface', () => {
// Why: lock the cursor/hit-test split so a cleanup does not put grab back on the whole row
// or drop row-level handle attrs that arm drag from indent/padding.
const source = readWorktreeListSource()
const headerBlockStart = source.indexOf('data-repo-header-id={projectIdForHeader}')
const headerBlockEnd = source.indexOf('<ProjectHeaderActions>', headerBlockStart)
expect(headerBlockStart).toBeGreaterThan(-1)
expect(headerBlockEnd).toBeGreaterThan(headerBlockStart)
const headerBlock = source.slice(headerBlockStart, headerBlockEnd)
// Dual handle placement: row (indent/padding) + title surface.
expect(headerBlock.match(/data-repo-header-drag-handle=/g)?.length).toBe(2)
expect(headerBlock.match(/data-project-group-header-drag-handle=/g)?.length).toBe(2)
// Surface owns grab; outer row only gets cursor-pointer when not draggable.
expect(headerBlock).toContain(
"!(isDraggableRepoHeader || isDraggableProjectGroupHeader) && 'cursor-pointer'"
)
expect(headerBlock).toContain("'flex min-w-0 flex-1 items-center gap-1.5 self-stretch'")
expect(headerBlock).toContain("'cursor-grab active:cursor-grabbing'")
// Row-level ternary grab must stay gone.
expect(headerBlock).not.toMatch(
/isDraggableRepoHeader \|\| isDraggableProjectGroupHeader\s*\?\s*'cursor-grab/
)
})
})

8
src/renderer/src/components/sidebar/project-group-header-drag-contract.ts
View File

@ -4,6 +4,7 @@ import type {
ProjectGroupHeaderDragBucketKey,
ProjectGroupHeaderDragRect
} from './project-group-header-drop'
import { REPO_HEADER_ACTION_SELECTOR } from './project-header-drag-contract'
import type { ProjectGroup } from '../../../../shared/types'
export type ProjectGroupDragState = {
@ -50,9 +51,6 @@ export const PROJECT_GROUP_HEADER_DRAG_THRESHOLD_PX = 4
const PROJECT_GROUP_HEADER_DRAG_HANDLE_SELECTOR = '[data-project-group-header-drag-handle]'
const PROJECT_GROUP_HEADER_ACTION_SELECTOR =
'[data-repo-header-action], [data-repo-header-collapse-affordance], button, a, input, textarea, select, [contenteditable=""], [contenteditable="true"]'
export function isProjectGroupHeaderDragHandleTarget(
target: EventTarget | null,
currentTarget: HTMLElement
@ -76,7 +74,5 @@ export function isProjectGroupHeaderActionTarget(
if (!(target instanceof Element) || target === currentTarget) {
return false
}
return (
currentTarget.contains(target) && target.closest(PROJECT_GROUP_HEADER_ACTION_SELECTOR) !== null
)
return currentTarget.contains(target) && target.closest(REPO_HEADER_ACTION_SELECTOR) !== null
}

135
src/renderer/src/components/sidebar/project-group-header-drag-start.test.ts
View File

@ -87,4 +87,139 @@ describe('createProjectGroupHeaderDragSession', () => {
expect(session).toBeNull()
})
it('arms from the group icon svg (SVGElement target)', () => {
const header = document.createElement('div')
header.setAttribute('data-project-group-header-drag-handle', '')
const iconSvg = document.createElementNS('http://www.w3.org/2000/svg', 'svg')
header.append(iconSvg)
const scrollContainer = document.createElement('div')
document.body.append(scrollContainer, header)
const projectGroupById = new Map<string, ProjectGroup>([
['group-a', group('group-a')],
['group-b', group('group-b')]
])
const sidebarProjectGroupHeaderIdsByBucket = new Map([['root', ['group-a', 'group-b']]])
const session = createProjectGroupHeaderDragSession({
event: {
button: 0,
pointerId: 1,
clientX: 10,
clientY: 20,
target: iconSvg,
currentTarget: header
} as unknown as React.PointerEvent<HTMLElement>,
groupId: 'group-a',
projectGroupById,
sidebarProjectGroupHeaderIdsByBucket,
getScrollContainer: () => scrollContainer
})
expect(session?.groupId).toBe('group-a')
})
it('does not arm when pressing an svg icon inside an action button', () => {
// Why: row stays the handle for hit-testing; action targets must be filtered.
const header = document.createElement('div')
header.setAttribute('data-project-group-header-drag-handle', '')
const actionButton = document.createElement('button')
actionButton.setAttribute('data-repo-header-action', '')
const actionIcon = document.createElementNS('http://www.w3.org/2000/svg', 'svg')
actionButton.append(actionIcon)
header.append(actionButton)
const scrollContainer = document.createElement('div')
document.body.append(scrollContainer, header)
const projectGroupById = new Map<string, ProjectGroup>([
['group-a', group('group-a')],
['group-b', group('group-b')]
])
const sidebarProjectGroupHeaderIdsByBucket = new Map([['root', ['group-a', 'group-b']]])
const session = createProjectGroupHeaderDragSession({
event: {
button: 0,
pointerId: 1,
clientX: 10,
clientY: 20,
target: actionIcon,
currentTarget: header
} as unknown as React.PointerEvent<HTMLElement>,
groupId: 'group-a',
projectGroupById,
sidebarProjectGroupHeaderIdsByBucket,
getScrollContainer: () => scrollContainer
})
expect(session).toBeNull()
})
it('does not arm from the actions overlay even if the row is the drag handle', () => {
// Why: row keeps the handle attr for indent/padding; overlay gaps must still be excluded.
const header = document.createElement('div')
header.setAttribute('data-project-group-header-drag-handle', '')
const dragSurface = document.createElement('div')
const label = document.createElement('span')
dragSurface.append(label)
const actions = document.createElement('div')
actions.setAttribute('data-repo-header-actions', '')
header.append(dragSurface, actions)
const scrollContainer = document.createElement('div')
document.body.append(scrollContainer, header)
const projectGroupById = new Map<string, ProjectGroup>([
['group-a', group('group-a')],
['group-b', group('group-b')]
])
const sidebarProjectGroupHeaderIdsByBucket = new Map([['root', ['group-a', 'group-b']]])
const sessionFromActions = createProjectGroupHeaderDragSession({
event: {
button: 0,
pointerId: 1,
clientX: 10,
clientY: 20,
target: actions,
currentTarget: header
} as unknown as React.PointerEvent<HTMLElement>,
groupId: 'group-a',
projectGroupById,
sidebarProjectGroupHeaderIdsByBucket,
getScrollContainer: () => scrollContainer
})
const sessionFromLabel = createProjectGroupHeaderDragSession({
event: {
button: 0,
pointerId: 2,
clientX: 10,
clientY: 20,
target: label,
currentTarget: header
} as unknown as React.PointerEvent<HTMLElement>,
groupId: 'group-a',
projectGroupById,
sidebarProjectGroupHeaderIdsByBucket,
getScrollContainer: () => scrollContainer
})
const sessionFromRowPadding = createProjectGroupHeaderDragSession({
event: {
button: 0,
pointerId: 3,
clientX: 10,
clientY: 20,
target: header,
currentTarget: header
} as unknown as React.PointerEvent<HTMLElement>,
groupId: 'group-a',
projectGroupById,
sidebarProjectGroupHeaderIdsByBucket,
getScrollContainer: () => scrollContainer
})
expect(sessionFromActions).toBeNull()
expect(sessionFromLabel?.groupId).toBe('group-a')
expect(sessionFromRowPadding?.groupId).toBe('group-a')
})
})

54
src/renderer/src/components/sidebar/project-group-header-drag.test.ts Normal file
View File

@ -0,0 +1,54 @@
// @vitest-environment happy-dom
import { describe, expect, it } from 'vitest'
import { isProjectGroupHeaderActionTarget } from './project-group-header-drag'
function createHeader(markup: string): HTMLElement {
const header = document.createElement('div')
header.setAttribute('data-project-group-header-id', 'group-1')
header.innerHTML = markup
document.body.appendChild(header)
return header
}
describe('project group header action targets', () => {
it('ignores explicit project action wrappers', () => {
const header = createHeader(`
<span data-repo-header-action="" tabindex="0">
<span id="icon"></span>
</span>
`)
expect(isProjectGroupHeaderActionTarget(header.querySelector('#icon'), header)).toBe(true)
})
it('ignores the project header actions overlay (including gaps between icons)', () => {
const header = createHeader(`
<div data-repo-header-actions="">
<button type="button" data-repo-header-action=""><span id="icon"></span></button>
</div>
`)
expect(
isProjectGroupHeaderActionTarget(header.querySelector('[data-repo-header-actions]'), header)
).toBe(true)
expect(isProjectGroupHeaderActionTarget(header.querySelector('#icon'), header)).toBe(true)
})
it('ignores the hover collapse affordance', () => {
const header = createHeader(`
<div data-repo-header-collapse-affordance="">
<span id="chevron"></span>
</div>
`)
expect(isProjectGroupHeaderActionTarget(header.querySelector('#chevron'), header)).toBe(true)
})
it('does not ignore plain header text or the header itself', () => {
const header = createHeader('<span id="label">Group</span>')
expect(isProjectGroupHeaderActionTarget(header.querySelector('#label'), header)).toBe(false)
expect(isProjectGroupHeaderActionTarget(header, header)).toBe(false)
})
})

25
src/renderer/src/components/sidebar/project-header-action-selector-lockstep.test.ts Normal file
View File

@ -0,0 +1,25 @@
// @vitest-environment happy-dom
import { describe, expect, it } from 'vitest'
import { isProjectGroupHeaderActionTarget } from './project-group-header-drag-contract'
import {
isRepoHeaderActionTarget,
REPO_HEADER_ACTION_SELECTOR
} from './project-header-drag-contract'
describe('project header action selectors', () => {
it('includes the actions overlay for both repo and group helpers', () => {
// Why: both helpers must share REPO_HEADER_ACTION_SELECTOR so overlay gaps
// never arm drag/toggle on either header kind.
expect(REPO_HEADER_ACTION_SELECTOR).toContain('[data-repo-header-actions]')
const header = document.createElement('div')
const actions = document.createElement('div')
actions.setAttribute('data-repo-header-actions', '')
header.append(actions)
document.body.append(header)
expect(isRepoHeaderActionTarget(actions, header)).toBe(true)
expect(isProjectGroupHeaderActionTarget(actions, header)).toBe(true)
})
})

5
src/renderer/src/components/sidebar/project-header-drag-contract.ts
View File

@ -47,8 +47,9 @@ 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], [data-repo-header-collapse-affordance], button, a, input, textarea, select, [contenteditable=""], [contenteditable="true"]'
// Shared with project-group headers: both reuse ProjectHeaderActions markup.
export const REPO_HEADER_ACTION_SELECTOR =
'[data-repo-header-actions], [data-repo-header-action], [data-repo-header-collapse-affordance], button, a, input, textarea, select, [contenteditable=""], [contenteditable="true"]'
export function isProjectHeaderDragHandleTarget(
target: EventTarget | null,

68
src/renderer/src/components/sidebar/project-header-drag-start.test.ts
View File

@ -107,6 +107,8 @@ describe('createProjectHeaderDragSession', () => {
})
it('does not arm a drag session when pressing an svg icon inside an action button', () => {
// Why: the row is still the drag handle for hit-testing; action targets must
// be filtered even though closest(drag-handle) would match the row.
const header = document.createElement('div')
header.setAttribute('data-repo-header-drag-handle', '')
const actionButton = document.createElement('button')
@ -137,4 +139,70 @@ describe('createProjectHeaderDragSession', () => {
expect(session).toBeNull()
})
it('does not arm a drag session when pressing the actions overlay even if the row is the drag handle', () => {
// Why: the row keeps data-repo-header-drag-handle so indent/padding can arm
// drag; actions must still be excluded via data-repo-header-actions.
const header = document.createElement('div')
header.setAttribute('data-repo-header-drag-handle', '')
const dragSurface = document.createElement('div')
const label = document.createElement('span')
dragSurface.append(label)
const actions = document.createElement('div')
actions.setAttribute('data-repo-header-actions', '')
header.append(dragSurface, actions)
const scrollContainer = document.createElement('div')
document.body.append(scrollContainer, header)
const repoById = new Map<string, Repo>([['repo-a', createRepo('repo-a')]])
const sidebarRepoHeaderIdsByBucket = new Map([['ungrouped', ['repo-a', 'repo-b']]])
const sessionFromActions = createProjectHeaderDragSession({
event: {
button: 0,
pointerId: 1,
clientX: 10,
clientY: 20,
target: actions,
currentTarget: header
} as unknown as React.PointerEvent<HTMLElement>,
repoId: 'repo-a',
repoById,
sidebarRepoHeaderIdsByBucket,
getScrollContainer: () => scrollContainer
})
const sessionFromLabel = createProjectHeaderDragSession({
event: {
button: 0,
pointerId: 2,
clientX: 10,
clientY: 20,
target: label,
currentTarget: header
} as unknown as React.PointerEvent<HTMLElement>,
repoId: 'repo-a',
repoById,
sidebarRepoHeaderIdsByBucket,
getScrollContainer: () => scrollContainer
})
const sessionFromRowPadding = createProjectHeaderDragSession({
event: {
button: 0,
pointerId: 3,
clientX: 10,
clientY: 20,
target: header,
currentTarget: header
} as unknown as React.PointerEvent<HTMLElement>,
repoId: 'repo-a',
repoById,
sidebarRepoHeaderIdsByBucket,
getScrollContainer: () => scrollContainer
})
expect(sessionFromActions).toBeNull()
expect(sessionFromLabel?.repoId).toBe('repo-a')
// Why: target === currentTarget is treated as the empty row (not an action).
expect(sessionFromRowPadding?.repoId).toBe('repo-a')
})
})

13
src/renderer/src/components/sidebar/project-header-drag.test.ts
View File

@ -44,4 +44,17 @@ describe('repo header action targets', () => {
expect(isRepoHeaderActionTarget(header.querySelector('#chevron'), header)).toBe(true)
})
it('ignores the project header actions overlay (including gaps between icons)', () => {
const header = createHeader(`
<div data-repo-header-actions="">
<button type="button" data-repo-header-action=""><span id="icon"></span></button>
</div>
`)
expect(
isRepoHeaderActionTarget(header.querySelector('[data-repo-header-actions]'), header)
).toBe(true)
expect(isRepoHeaderActionTarget(header.querySelector('#icon'), header)).toBe(true)
})
})