diff --git a/src/renderer/src/components/sidebar/WorktreeCard.tsx b/src/renderer/src/components/sidebar/WorktreeCard.tsx index 398e469da1c..966963eccc7 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.tsx @@ -759,7 +759,7 @@ const WorktreeCard = React.memo(function WorktreeCard({ )} - {lineageChildren &&
{lineageChildren}
} + {lineageChildren &&
{lineageChildren}
} ) diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts b/src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts new file mode 100644 index 00000000000..e909eea3dee --- /dev/null +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from 'vitest' +import { + shouldIgnoreNestedWorktreeContextMenuScope, + shouldSuppressContextMenuFollowUpClick +} from './WorktreeContextMenu' + +describe('shouldIgnoreNestedWorktreeContextMenuScope', () => { + it('allows the context menu scope that owns the event target', () => { + const currentScope = {} as EventTarget + const target = { + closest: () => currentScope + } as unknown as EventTarget + + expect(shouldIgnoreNestedWorktreeContextMenuScope(currentScope, target)).toBe(false) + }) + + it('ignores context menu events owned by a nested scope', () => { + const currentScope = {} as EventTarget + const nestedScope = {} as Element + const target = { + closest: () => nestedScope + } as unknown as EventTarget + + expect(shouldIgnoreNestedWorktreeContextMenuScope(currentScope, target)).toBe(true) + }) + + it('ignores context menu events from text nodes inside a nested scope', () => { + const currentScope = {} as EventTarget + const nestedScope = {} as Element + const target = { + parentElement: { + closest: () => nestedScope + } + } as unknown as EventTarget + + expect(shouldIgnoreNestedWorktreeContextMenuScope(currentScope, target)).toBe(true) + }) + + it('allows events from unscoped targets', () => { + const currentScope = {} as EventTarget + const target = { + closest: () => null + } as unknown as EventTarget + + expect(shouldIgnoreNestedWorktreeContextMenuScope(currentScope, target)).toBe(false) + }) +}) + +describe('shouldSuppressContextMenuFollowUpClick', () => { + it('suppresses the click emitted immediately after opening a context menu', () => { + expect(shouldSuppressContextMenuFollowUpClick(1_000, 1_050)).toBe(true) + }) + + it('does not suppress later unrelated clicks', () => { + expect(shouldSuppressContextMenuFollowUpClick(1_000, 1_700)).toBe(false) + }) + + it('does not suppress clicks that predate the context menu timestamp', () => { + expect(shouldSuppressContextMenuFollowUpClick(1_000, 999)).toBe(false) + }) +}) diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx index e3830d57824..b05c47b6881 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx @@ -1,4 +1,5 @@ -import React, { useCallback, useEffect, useMemo, useState } from 'react' +/* eslint-disable max-lines -- Why: this menu keeps row targeting, batch actions, and ctrl-click event guards together so nested worktree menus share one event policy. */ +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { DropdownMenu, DropdownMenuContent, @@ -42,6 +43,30 @@ type Props = { const CLOSE_ALL_CONTEXT_MENUS_EVENT = 'orca-close-all-context-menus' const WORKTREE_CONTEXT_MENU_SCOPE_ATTR = 'data-worktree-context-menu-scope' +const CONTEXT_MENU_CLICK_SUPPRESSION_MS = 500 + +function shouldIgnoreNestedWorktreeContextMenuScope( + currentTarget: EventTarget, + target: EventTarget | null +): boolean { + const maybeScopedTarget = target as { + closest?: (selector: string) => Element | null + parentElement?: { closest?: (selector: string) => Element | null } + } | null + const scopeSelector = `[${WORKTREE_CONTEXT_MENU_SCOPE_ATTR}]` + const closestScope = + maybeScopedTarget?.closest?.(scopeSelector) ?? + maybeScopedTarget?.parentElement?.closest?.(scopeSelector) + // Why: lineage child previews live inside the parent card DOM but own their + // context menu target. The parent must ignore only those nested scopes. + return closestScope != null && closestScope !== currentTarget +} + +function shouldSuppressContextMenuFollowUpClick(contextMenuOpenedAt: number, now: number): boolean { + return ( + now - contextMenuOpenedAt >= 0 && now - contextMenuOpenedAt <= CONTEXT_MENU_CLICK_SUPPRESSION_MS + ) +} const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ worktree, @@ -61,13 +86,13 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ const isFolder = repo ? isFolderRepo(repo) : false const repoMap = useRepoMap() const worktreeMap = useWorktreeMap() - const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) const worktreeLineageById = useAppStore((s) => s.worktreeLineageById) const updateWorktreeLineage = useAppStore((s) => s.updateWorktreeLineage) const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) const ptyIdsByTabId = useAppStore((s) => s.ptyIdsByTabId) const browserTabsByWorktree = useAppStore((s) => s.browserTabsByWorktree) const deleteStateByWorktreeId = useAppStore((s) => s.deleteStateByWorktreeId) + const contextMenuOpenedAtRef = useRef(null) const activeContextWorktrees = menuOpen ? contextWorktrees : selectedWorktrees const isMultiContext = activeContextWorktrees.length > 1 const sleepableWorktrees = useMemo( @@ -109,8 +134,6 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ ) const validParentWorktreeId = lineageInfo.state === 'valid' ? lineageInfo.parent.id : null const hasAnyContextLineage = activeContextWorktrees.some((item) => worktreeLineageById[item.id]) - const canGroupUnderActive = - activeWorktreeId != null && activeContextWorktrees.every((item) => item.id !== activeWorktreeId) useEffect(() => { const closeMenu = (): void => setMenuOpen(false) @@ -231,46 +254,53 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ } }, [validParentWorktreeId]) - const handleGroupUnderActive = useCallback(() => { - if (!activeWorktreeId) { - return - } - void Promise.all( - activeContextWorktrees.map((item) => - updateWorktreeLineage(item.id, { parentWorktreeId: activeWorktreeId }) - ) - ) - }, [activeContextWorktrees, activeWorktreeId, updateWorktreeLineage]) - const handleRemoveParentLink = useCallback(() => { void Promise.all( activeContextWorktrees.map((item) => updateWorktreeLineage(item.id, { noParent: true })) ) }, [activeContextWorktrees, updateWorktreeLineage]) - return ( - <> -
{ - const target = event.target - if ( - target instanceof Element && - target.closest(`[${WORKTREE_CONTEXT_MENU_SCOPE_ATTR}]`) - ) { - return - } - event.preventDefault() - window.dispatchEvent(new Event(CLOSE_ALL_CONTEXT_MENUS_EVENT)) - setContextWorktrees(onContextMenuSelect?.(event) ?? selectedWorktrees) - const bounds = event.currentTarget.getBoundingClientRect() - setMenuPoint({ x: event.clientX - bounds.left, y: event.clientY - bounds.top }) - setMenuOpen(true) - }} - > - {children} -
+ const suppressOpeningPointerEvent = useCallback((event: React.SyntheticEvent) => { + const contextMenuOpenedAt = contextMenuOpenedAtRef.current + if ( + contextMenuOpenedAt == null || + !shouldSuppressContextMenuFollowUpClick(contextMenuOpenedAt, Date.now()) + ) { + if (contextMenuOpenedAt != null) { + contextMenuOpenedAtRef.current = null + } + return + } + // Why: macOS ctrl-click can release over the just-opened menu, selecting + // the item under the cursor unless the opening pointer sequence is ignored. + event.preventDefault() + event.stopPropagation() + if (event.type === 'click') { + contextMenuOpenedAtRef.current = null + } + }, []) + return ( +
{ + if (shouldIgnoreNestedWorktreeContextMenuScope(event.currentTarget, event.target)) { + return + } + event.preventDefault() + contextMenuOpenedAtRef.current = Date.now() + window.dispatchEvent(new Event(CLOSE_ALL_CONTEXT_MENUS_EVENT)) + setContextWorktrees(onContextMenuSelect?.(event) ?? selectedWorktrees) + const bounds = event.currentTarget.getBoundingClientRect() + setMenuPoint({ x: event.clientX - bounds.left, y: event.clientY - bounds.top }) + setMenuOpen(true) + }} + onClickCapture={(event) => { + suppressOpeningPointerEvent(event) + }} + > + {children}
) }) export default WorktreeContextMenu -export { CLOSE_ALL_CONTEXT_MENUS_EVENT, WORKTREE_CONTEXT_MENU_SCOPE_ATTR } +export { + CLOSE_ALL_CONTEXT_MENUS_EVENT, + WORKTREE_CONTEXT_MENU_SCOPE_ATTR, + shouldSuppressContextMenuFollowUpClick, + shouldIgnoreNestedWorktreeContextMenuScope +} diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 9b709a8ce7d..97352e8cc8d 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -1,7 +1,7 @@ /* eslint-disable max-lines */ import React, { useMemo, useCallback, useRef, useState, useEffect, useLayoutEffect } from 'react' import { useVirtualizer } from '@tanstack/react-virtual' -import { ChevronDown, CircleX, Plus } from 'lucide-react' +import { ChevronDown, CircleX, Plus, Workflow } from 'lucide-react' import { useAppStore } from '@/store' import { getAllWorktreesFromState, @@ -40,6 +40,7 @@ import { } from './visible-worktrees' import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { useRepoHeaderDrag } from './repo-header-drag' +import WorktreeContextMenu from './WorktreeContextMenu' import { areWorktreeSelectionsEqual, getWorktreeSelectionIntent, @@ -78,7 +79,7 @@ function getWorktreeOptionId(worktreeId: string): string { return `worktree-list-option-${encodeURIComponent(worktreeId)}` } -const LINEAGE_INDENT = 30 +const LINEAGE_INDENT = 24 type VirtualizedWorktreeViewportProps = { rows: Row[] @@ -671,52 +672,106 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp return (
event.stopPropagation()} + style={{ paddingLeft: `${Math.max(0, child.depth - 1) * LINEAGE_INDENT}px` }} > - -
-
- {child.worktree.displayName} -
-
- {child.repo && groupBy !== 'repo' ? ( - - - - {child.repo.displayName} - - - ) : null} - - {branchDisplayName(child.worktree.branch)} + onContextMenuSelect(event, child.worktree)} + > +
event.stopPropagation()} + > + + -
- {child.worktree.linkedIssue || child.worktree.comment ? ( -
- {child.worktree.linkedIssue ? ( - - #{child.worktree.linkedIssue} +
+
+ {child.worktree.displayName} +
+
+ {child.repo && groupBy !== 'repo' ? ( + + + + {child.repo.displayName} + + + ) : null} + + {branchDisplayName(child.worktree.branch)} +
+ {child.worktree.linkedIssue || child.worktree.comment ? ( +
+ {child.worktree.linkedIssue ? ( + + #{child.worktree.linkedIssue} + + ) : null} + {child.worktree.linkedIssue && child.worktree.comment ? ' ' : null} + {child.worktree.comment} +
+ ) : null} + {child.lineageChildCount > 0 && child.lineageGroupKey ? ( +
+ + + + + + {child.lineageCollapsed + ? 'Show child workspaces' + : 'Hide child workspaces'} + + +
) : null} - {child.worktree.linkedIssue && child.worktree.comment ? ' ' : null} - {child.worktree.comment}
- ) : null} -
+
+
) } diff --git a/tests/e2e/worktree-lineage.spec.ts b/tests/e2e/worktree-lineage.spec.ts index 55bbcad8d09..201965723b8 100644 --- a/tests/e2e/worktree-lineage.spec.ts +++ b/tests/e2e/worktree-lineage.spec.ts @@ -33,6 +33,9 @@ async function seedLineageScenario(page: Page): Promise { } const [parent, child] = worktrees + if (!parent.instanceId || !child.instanceId) { + throw new Error('Worktree lineage E2E needs instance-stamped worktrees') + } store.setState((current) => ({ worktreesByRepo: Object.fromEntries( Object.entries(current.worktreesByRepo).map(([repoId, repoWorktrees]) => [ @@ -47,7 +50,19 @@ async function seedLineageScenario(page: Page): Promise { return worktree }) ]) - ) + ), + worktreeLineageById: { + ...current.worktreeLineageById, + [child.id]: { + worktreeId: child.id, + worktreeInstanceId: child.instanceId, + parentWorktreeId: parent.id, + parentWorktreeInstanceId: parent.instanceId, + origin: 'manual', + capture: { source: 'manual-action', confidence: 'explicit' }, + createdAt: Date.now() + } + } })) store.getState().setActiveWorktree(parent.id) @@ -61,9 +76,7 @@ test.describe('Worktree Lineage', () => { await waitForActiveWorktree(orcaPage) }) - test('groups a child under the active workspace from the sidebar context menu', async ({ - orcaPage - }) => { + test('renders existing child lineage in the sidebar', async ({ orcaPage }) => { const { parentId, childId } = await seedLineageScenario(orcaPage) const parentRow = worktreeOption(orcaPage, parentId) const childRow = worktreeOption(orcaPage, childId) @@ -73,9 +86,6 @@ test.describe('Worktree Lineage', () => { await expect(parentRow).toHaveAttribute('aria-current', 'page') await expect(childRow).toContainText('E2E lineage child') - await childRow.click({ button: 'right' }) - await orcaPage.getByRole('menuitem', { name: 'Group under Active Workspace' }).click() - const childToggle = parentRow.getByRole('button', { name: 'Hide 1 child workspace' }) await expect(childToggle).toBeVisible({ timeout: 10_000 }) await expect(childRow).toBeVisible() @@ -102,5 +112,16 @@ test.describe('Worktree Lineage', () => { await childToggle.click() await expect(parentRow.getByRole('button', { name: 'Show 1 child workspace' })).toBeVisible() await expect(childRow).toBeHidden() + + await parentRow.getByRole('button', { name: 'Show 1 child workspace' }).click() + await childRow.click({ button: 'right' }) + await orcaPage.getByRole('menuitem', { name: 'Remove from Parent' }).click() + await expect(parentRow.getByRole('button', { name: /child workspace/ })).toHaveCount(0) + await expect(childRow).toBeVisible() + + await parentRow.click({ button: 'right' }) + await expect( + orcaPage.getByRole('menuitem', { name: 'Group under Active Workspace' }) + ).toHaveCount(0) }) })