From de37a4cbe874e46f55d4274a58c85951621b9564 Mon Sep 17 00:00:00 2001 From: alex <40974140+alxojy@users.noreply.github.com> Date: Sat, 24 Oct 2026 10:32:00 +0800 Subject: [PATCH] fix(workspace-board): close the board when a right-sidebar tab is clicked (#30063) Co-authored-by: Claude Opus 5.5 Co-authored-by: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Co-authored-by: Neil --- .../right-sidebar/activity-bar-buttons.tsx | 2 + ...e-workspace-kanban-outside-dismiss.test.ts | 167 ++++++++++++------ .../use-workspace-kanban-outside-dismiss.ts | 30 +++- ...ce-board-right-sidebar-tab-dismiss.spec.ts | 40 +++++ 4 files changed, 182 insertions(+), 57 deletions(-) create mode 100644 tests/e2e/workspace-board-right-sidebar-tab-dismiss.spec.ts diff --git a/src/renderer/src/components/right-sidebar/activity-bar-buttons.tsx b/src/renderer/src/components/right-sidebar/activity-bar-buttons.tsx index 2284ac53ac4..7362f66a68f 100644 --- a/src/renderer/src/components/right-sidebar/activity-bar-buttons.tsx +++ b/src/renderer/src/components/right-sidebar/activity-bar-buttons.tsx @@ -100,6 +100,7 @@ export function TopActivityOverflowMenu({ onSelect(item.id)} + data-workspace-board-dismiss="" className={cn(active && 'bg-accent text-accent-foreground')} aria-current={active ? 'page' : undefined} aria-label={activityItemAriaLabel(item, item.statusIndicator)} @@ -150,6 +151,7 @@ export function ActivityBarButton({ active ? 'text-foreground' : 'text-muted-foreground/60 hover:text-muted-foreground' )} onClick={onClick} + data-workspace-board-dismiss="" aria-label={activityItemAriaLabel(item, effectiveStatus)} > diff --git a/src/renderer/src/components/sidebar/use-workspace-kanban-outside-dismiss.test.ts b/src/renderer/src/components/sidebar/use-workspace-kanban-outside-dismiss.test.ts index 0908a2966fe..e8b145a2e9f 100644 --- a/src/renderer/src/components/sidebar/use-workspace-kanban-outside-dismiss.test.ts +++ b/src/renderer/src/components/sidebar/use-workspace-kanban-outside-dismiss.test.ts @@ -1,61 +1,128 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { isWorkspaceBoardKeepOpenTarget } from './use-workspace-kanban-outside-dismiss' +// @vitest-environment happy-dom -class FakeNode { - parentElement: FakeElement | null = null +import { afterEach, describe, expect, it, vi } from 'vitest' +import { cleanup, renderHook } from '@testing-library/react' +import { + isWorkspaceBoardKeepOpenTarget, + useWorkspaceKanbanOutsideDismiss +} from './use-workspace-kanban-outside-dismiss' + +afterEach(() => { + cleanup() + document.body.replaceChildren() +}) + +function mountBoardListener({ + open = true, + preserveOpenForMenu = false +}: { open?: boolean; preserveOpenForMenu?: boolean } = {}) { + const sheet = document.createElement('div') + sheet.setAttribute('data-slot', 'sheet-content') + const board = document.createElement('div') + sheet.append(board) + document.body.append(sheet) + vi.spyOn(sheet, 'getBoundingClientRect').mockReturnValue(new DOMRect(100, 36, 800, 600)) + const onOpenChange = vi.fn() + const hook = renderHook(() => + useWorkspaceKanbanOutsideDismiss({ + open, + boardRef: { current: board }, + preserveOpenForMenu, + onOpenChange + }) + ) + return { board, onOpenChange, unmount: hook.unmount } } -class FakeElement extends FakeNode { - private readonly attributes: ReadonlySet - - constructor(attributes: readonly string[] = [], parentElement: FakeElement | null = null) { - super() - this.attributes = new Set(attributes) - this.parentElement = parentElement - } - - closest(selector: string): FakeElement | null { - if (this.matches(selector)) { - return this - } - return this.parentElement?.closest(selector) ?? null - } - - private matches(selector: string): boolean { - return selector - .split(',') - .map((part) => part.trim()) - .some((part) => this.attributes.has(part)) - } +function sidebarTab(): HTMLButtonElement { + const tab = document.createElement('button') + tab.setAttribute('data-workspace-board-dismiss', '') + document.body.append(tab) + return tab } describe('workspace kanban outside dismiss keep-open targets', () => { - beforeEach(() => { - vi.stubGlobal('Node', FakeNode) - vi.stubGlobal('Element', FakeElement) - }) - - afterEach(() => { - vi.unstubAllGlobals() - }) - - it('keeps the board open when a Sonner toast action is clicked', () => { - const toast = new FakeElement(['[data-sonner-toast]']) - const action = new FakeElement([], toast) - - expect(isWorkspaceBoardKeepOpenTarget(action as unknown as EventTarget)).toBe(true) - }) - - it('keeps the board open when the contextual tour panel is clicked', () => { - const panel = new FakeElement(['[data-contextual-tour-panel]']) - const nextButton = new FakeElement([], panel) - - expect(isWorkspaceBoardKeepOpenTarget(nextButton as unknown as EventTarget)).toBe(true) - }) + it.each(['data-sonner-toast', 'data-contextual-tour-panel'])( + 'keeps the board open for a child of %s', + (attribute) => { + const panel = document.createElement('div') + panel.setAttribute(attribute, '') + const action = document.createElement('button') + panel.append(action) + expect(isWorkspaceBoardKeepOpenTarget(action)).toBe(true) + } + ) it('does not keep the board open for generic outside content', () => { - const target = new FakeElement() + expect(isWorkspaceBoardKeepOpenTarget(document.createElement('div'))).toBe(false) + }) +}) - expect(isWorkspaceBoardKeepOpenTarget(target as unknown as EventTarget)).toBe(false) +describe('workspace kanban outside dismiss', () => { + it.each(['pointerdown', 'click'])( + 'closes from a sidebar tab inside a portalled menu on %s', + (eventType) => { + const { onOpenChange } = mountBoardListener() + const menu = document.createElement('div') + menu.setAttribute('data-slot', 'dropdown-menu-content') + const tab = sidebarTab() + const icon = document.createElement('span') + tab.append(icon) + menu.append(tab) + document.body.append(menu) + + icon.dispatchEvent(new MouseEvent(eventType, { bubbles: true })) + + expect(onOpenChange).toHaveBeenCalledWith(false) + } + ) + + it('dismisses for an explicit sidebar tab while a board menu is open', () => { + const { onOpenChange } = mountBoardListener({ preserveOpenForMenu: true }) + + sidebarTab().dispatchEvent(new PointerEvent('pointerdown', { bubbles: true })) + + expect(onOpenChange).toHaveBeenCalledWith(false) + }) + + it('preserves board content, unrelated menus and the existing outside bounds', () => { + const { board, onOpenChange } = mountBoardListener() + const markerInsideBoard = sidebarTab() + board.append(markerInsideBoard) + markerInsideBoard.click() + const menu = document.createElement('div') + menu.setAttribute('data-slot', 'dropdown-menu-content') + document.body.append(menu) + menu.dispatchEvent( + new PointerEvent('pointerdown', { bubbles: true, clientX: 950, clientY: 80 }) + ) + document.body.dispatchEvent( + new PointerEvent('pointerdown', { bubbles: true, clientX: 500, clientY: 18 }) + ) + expect(onOpenChange).not.toHaveBeenCalled() + + document.body.dispatchEvent( + new PointerEvent('pointerdown', { bubbles: true, clientX: 950, clientY: 80 }) + ) + expect(onOpenChange).toHaveBeenCalledWith(false) + }) + + it('preserves an open board menu for ordinary outside pointer events', () => { + const { onOpenChange } = mountBoardListener({ preserveOpenForMenu: true }) + document.body.dispatchEvent( + new PointerEvent('pointerdown', { bubbles: true, clientX: 950, clientY: 80 }) + ) + expect(onOpenChange).not.toHaveBeenCalled() + }) + + it('stops listening after unmount and while the board is closed', () => { + const mounted = mountBoardListener() + mounted.unmount() + const closed = mountBoardListener({ open: false }) + const tab = sidebarTab() + tab.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true })) + tab.click() + expect(mounted.onOpenChange).not.toHaveBeenCalled() + expect(closed.onOpenChange).not.toHaveBeenCalled() }) }) diff --git a/src/renderer/src/components/sidebar/use-workspace-kanban-outside-dismiss.ts b/src/renderer/src/components/sidebar/use-workspace-kanban-outside-dismiss.ts index 0c9d156c88c..9a39b3e2826 100644 --- a/src/renderer/src/components/sidebar/use-workspace-kanban-outside-dismiss.ts +++ b/src/renderer/src/components/sidebar/use-workspace-kanban-outside-dismiss.ts @@ -19,9 +19,12 @@ const WORKSPACE_BOARD_KEEP_OPEN_SELECTOR = [ '[role="menu"][data-state="open"]' ].join(', ') +function eventTargetElement(target: EventTarget | null): Element | null { + return target instanceof Element ? target : target instanceof Node ? target.parentElement : null +} + export function isWorkspaceBoardKeepOpenTarget(target: EventTarget | null): boolean { - const element = - target instanceof Element ? target : target instanceof Node ? target.parentElement : null + const element = eventTargetElement(target) // Why: board-owned menus and confirmation dialogs portal to document.body, // so DOM containment alone would treat their clicks/focus as board exits. return Boolean(element?.closest(WORKSPACE_BOARD_KEEP_OPEN_SELECTOR)) @@ -40,15 +43,24 @@ export function useWorkspaceKanbanOutsideDismiss(params: { return } - const handlePointerDown = (event: PointerEvent): void => { + const handleOutsideInteraction = (event: MouseEvent): void => { const content = boardRef.current?.closest('[data-slot="sheet-content"]') - if (!content || preserveOpenForMenu) { + if (!content) { return } if (event.target instanceof Node && content.contains(event.target)) { return } - if (isWorkspaceBoardKeepOpenTarget(event.target)) { + // Why: titlebar tabs switch content the board covers, so it must close to show it. + if (eventTargetElement(event.target)?.closest('[data-workspace-board-dismiss]')) { + onOpenChange(false) + return + } + if ( + preserveOpenForMenu || + event.type === 'click' || + isWorkspaceBoardKeepOpenTarget(event.target) + ) { return } const rect = content.getBoundingClientRect() @@ -57,7 +69,11 @@ export function useWorkspaceKanbanOutsideDismiss(params: { } } - document.addEventListener('pointerdown', handlePointerDown, true) - return () => document.removeEventListener('pointerdown', handlePointerDown, true) + document.addEventListener('pointerdown', handleOutsideInteraction, true) + document.addEventListener('click', handleOutsideInteraction, true) + return () => { + document.removeEventListener('pointerdown', handleOutsideInteraction, true) + document.removeEventListener('click', handleOutsideInteraction, true) + } }, [boardRef, onOpenChange, open, preserveOpenForMenu]) } diff --git a/tests/e2e/workspace-board-right-sidebar-tab-dismiss.spec.ts b/tests/e2e/workspace-board-right-sidebar-tab-dismiss.spec.ts new file mode 100644 index 00000000000..4ab3761d4c4 --- /dev/null +++ b/tests/e2e/workspace-board-right-sidebar-tab-dismiss.spec.ts @@ -0,0 +1,40 @@ +import { test, expect } from './helpers/orca-app' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' + +// Why: on narrow windows the titlebar tab strip switches panels the board covers. +test.describe('Workspace board right-sidebar tab dismiss', () => { + test.skip(process.platform !== 'darwin', 'Linux and Windows draw the tab strip under the board') + + test.beforeEach(async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + }) + + test('clicking a right-sidebar tab closes the board and shows that tab', async ({ + electronApp, + orcaPage + }) => { + await electronApp.evaluate(({ BrowserWindow }) => { + BrowserWindow.getAllWindows()[0].setSize(1000, 800) + }) + await orcaPage.evaluate(() => { + const state = window.__store?.getState() + if (!state) { + throw new Error('window.__store is not available') + } + state.setSidebarOpen(true) + state.setRightSidebarOpen(true) + state.setRightSidebarTab('explorer') + }) + + await orcaPage.getByRole('button', { name: 'Workspace board' }).click() + const board = orcaPage.locator('[data-workspace-board-sheet]') + await expect(board).toBeVisible() + await board.evaluate((el) => Promise.all(el.getAnimations().map((a) => a.finished))) + + await orcaPage.getByRole('button', { name: /^Source Control/ }).click() + + await expect(board).toBeHidden() + await expect(orcaPage.getByTestId('source-control-filter-toggle')).toBeVisible() + }) +})