From 3aeb592863950675f424ee46b5180a694859ec79 Mon Sep 17 00:00:00 2001 From: abundantbeing Date: Sat, 15 Aug 2026 01:56:54 +0700 Subject: [PATCH] fix(desktop): stop tabs double-click-hiding the tab strip; body double-tap reveals it The synthesized double-tap that hides a zone's tab strip rode every tab's pointerdown (generic pane drag and each pane's tabDrag), so a routine double-click on a tab (select a title, retry a click) vanished the whole bar and stranded the zone with no tab, no close X, and no way back but a right-click. Keep the documented hide gesture on the strip background only, and add its inverse as recovery: double-tap a hidden zone's body restores the strip. Regression tests pin both sides of the grammar. --- apps/desktop/src/app/chat/session-drag.ts | 4 +- apps/desktop/src/app/chat/session-tile.tsx | 6 +- apps/desktop/src/app/contrib/surfaces.tsx | 5 +- .../pane-shell/tree/renderer/drag-session.ts | 2 +- .../tree/renderer/tab-strip-hide.test.tsx | 133 ++++++++++++++++++ .../pane-shell/tree/renderer/tree-group.tsx | 76 ++++++++-- 6 files changed, 202 insertions(+), 24 deletions(-) create mode 100644 apps/desktop/src/components/pane-shell/tree/renderer/tab-strip-hide.test.tsx diff --git a/apps/desktop/src/app/chat/session-drag.ts b/apps/desktop/src/app/chat/session-drag.ts index 8c2c5fbe2b..b283b07458 100644 --- a/apps/desktop/src/app/chat/session-drag.ts +++ b/apps/desktop/src/app/chat/session-drag.ts @@ -95,8 +95,8 @@ function tileZoneHost(groupId: string): { chat: boolean; pane: string } | null { /** * Begin dragging a session — a sidebar row OR a tile's own tab (same drop * language either way: stack, split, or composer link). Sub-threshold releases - * stay ordinary clicks, so `opts.onTap` (activate the tile) and `opts.double` - * (hide the tab bar) ride the tab's gestures; Esc aborts instantly. A stack/ + * stay ordinary clicks, so `opts.onTap` (activate the tile) rides the tab's + * gesture; Esc aborts instantly. A stack/ * split commits through `openSessionTile`, which OPENS a new tile from a sidebar * row and MOVES the existing one when its tab is the drag source. */ diff --git a/apps/desktop/src/app/chat/session-tile.tsx b/apps/desktop/src/app/chat/session-tile.tsx index 88432e9b54..0cb0c807a8 100644 --- a/apps/desktop/src/app/chat/session-tile.tsx +++ b/apps/desktop/src/app/chat/session-tile.tsx @@ -616,9 +616,9 @@ export const watchSessionTiles = paneMirror({ ), // A tile's tab drags like a sidebar row — stack / split / drop-to-link — with - // its tap (activate) + double-tap (hide bar) preserved. Always takes the drag. - tabDrag: (storedSessionId, event, onTap, double) => { - startSessionDrag(tileDragPayload(storedSessionId), event, { double, onTap }) + // its tap (activate) preserved. Always takes the drag. + tabDrag: (storedSessionId, event, onTap) => { + startSessionDrag(tileDragPayload(storedSessionId), event, { onTap }) return true }, diff --git a/apps/desktop/src/app/contrib/surfaces.tsx b/apps/desktop/src/app/contrib/surfaces.tsx index d2108a751f..e191e40011 100644 --- a/apps/desktop/src/app/contrib/surfaces.tsx +++ b/apps/desktop/src/app/contrib/surfaces.tsx @@ -147,9 +147,8 @@ export const ChatRoutesSurface = memo(function ChatRoutesSurface({ /> ) - // FULL-PAGE views (not chat) mark the zone body `data-zone-no-header`: a - // page is not a tab-able surface, so the zone's double-click header toggle - // stands down while one is showing (see onZoneDoubleClick). + // FULL-PAGE views (not chat): a page is not a tab-able surface, so the + // zone's tab strip stands down while one is showing (paneChrome.headerVeto). const page = (view: ReactNode) => (
{view} diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/drag-session.ts b/apps/desktop/src/components/pane-shell/tree/renderer/drag-session.ts index 826f15d4b9..6066bb0eab 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/drag-session.ts +++ b/apps/desktop/src/components/pane-shell/tree/renderer/drag-session.ts @@ -158,7 +158,7 @@ const sameHint = (a: DropHint | null, b: DropHint | null) => /** Double-tap detection for drag handles. Pane handles preventDefault * pointerdown, which suppresses native `dblclick` — so rapid same-handle * taps are detected here instead. */ -const DOUBLE_TAP_MS = 400 +export const DOUBLE_TAP_MS = 400 let lastTap: { key: string; time: number } | null = null export interface DoubleTapContext { diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/tab-strip-hide.test.tsx b/apps/desktop/src/components/pane-shell/tree/renderer/tab-strip-hide.test.tsx new file mode 100644 index 0000000000..4851e3ffdf --- /dev/null +++ b/apps/desktop/src/components/pane-shell/tree/renderer/tab-strip-hide.test.tsx @@ -0,0 +1,133 @@ +import { useStore } from '@nanostores/react' +import { cleanup, fireEvent, render } from '@testing-library/react' +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +import { registry } from '@/contrib/registry' + +import { group, split } from '../model' +import { $layoutTree, markCollapsePane, registerPaneCloser } from '../store' + +import { TreeGroup } from './tree-group' + +/** TreeGroup reads its node from props; subscribe so store writes re-render. */ +function LiveTreeGroup() { + useStore($layoutTree) + + return +} + +// Pins the tab-strip hide/recovery grammar: hiding the strip is an EXPLICIT +// verb (zone menu / main tab menu) — a double-click on a tab must NOT vanish +// the bar (it used to, stranding the zone with no ✕), and a double-tap on the +// zone body restores a strip that WAS hidden explicitly. + +class TestResizeObserver { + observe() {} + unobserve() {} + disconnect() {} +} + +beforeAll(() => { + vi.stubGlobal('ResizeObserver', TestResizeObserver) + // jsdom lacks CSS.escape, which tab-strip-scroll uses in a layout effect. + vi.stubGlobal('CSS', { ...globalThis.CSS, escape: (value: string) => value }) + Element.prototype.hasPointerCapture ??= () => false + Element.prototype.setPointerCapture ??= () => undefined + Element.prototype.releasePointerCapture ??= () => undefined + HTMLElement.prototype.scrollIntoView ??= () => undefined +}) + +const disposers: (() => void)[] = [] + +beforeEach(async () => { + window.localStorage.clear() + + const { $dismissedPanes, $hiddenTreePanes } = await import('../store') + $dismissedPanes.set(new Set()) + $hiddenTreePanes.set(new Set()) + + for (const [id, data] of [ + ['workspace', { placement: 'main', uncloseable: true }], + ['terminal', { placement: 'bottom' }] + ] as const) { + disposers.push(registry.register({ area: 'panes', data, id, render: () => null, title: id })) + } + + markCollapsePane('terminal') + registerPaneCloser('terminal', () => undefined) +}) + +afterEach(() => { + cleanup() + disposers.splice(0).forEach(dispose => dispose()) +}) + +const zoneAt = (index: number) => { + const node = $layoutTree.get()! + + return (node.type === 'split' ? node.children[index] : node) as never +} + +const groupNode = () => { + const node = $layoutTree.get()! + + return (node.type === 'split' ? node.children[0] : node) as { headerHidden?: boolean; panes: string[] } +} + +const tablist = () => document.querySelector('[role="tablist"]') + +const doublePointerDown = (target: Element) => { + fireEvent.pointerDown(target, { button: 0, pointerType: 'mouse' }) + fireEvent.pointerDown(target, { button: 0, pointerType: 'mouse' }) +} + +describe('tab strip hide/recovery grammar', () => { + it('double-clicking a tab does NOT hide the strip', () => { + // $layoutTree.set, not declareDefaultTree — the latter only adopts into an + // existing tree, and the store is module state that survives between tests. + $layoutTree.set( + split('column', [group(['workspace', 'terminal'], { active: 'terminal', id: 'grp-main' })]) + ) + render() + + const tab = document.querySelector('[data-tree-tab="terminal"]') + expect(tab).toBeTruthy() + + doublePointerDown(tab!) + + // The strip is still there and the tree never recorded a hide. + expect(tablist()).toBeTruthy() + expect(groupNode().headerHidden).not.toBe(true) + }) + + it('double-tapping the zone body restores an explicitly hidden strip', () => { + $layoutTree.set( + split('column', [group(['terminal'], { active: 'terminal', headerHidden: true, id: 'grp-tools' })]) + ) + render() + + // Hidden strip: no tablist, no ✕ anywhere in the zone. + expect(tablist()).toBeNull() + + const body = document.querySelector('[data-tree-group="grp-tools"]') + expect(body).toBeTruthy() + + doublePointerDown(body!) + + expect(tablist()).toBeTruthy() + expect(groupNode().headerHidden).toBe(false) + }) + + it('a single body tap does not toggle the strip back', () => { + $layoutTree.set( + split('column', [group(['terminal'], { active: 'terminal', headerHidden: true, id: 'grp-tools' })]) + ) + render() + + const body = document.querySelector('[data-tree-group="grp-tools"]')! + fireEvent.pointerDown(body, { button: 0, pointerType: 'mouse' }) + + expect(tablist()).toBeNull() + expect(groupNode().headerHidden).toBe(true) + }) +}) diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx b/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx index 481dfd02e5..94bae355a7 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx +++ b/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx @@ -10,7 +10,15 @@ */ import { useStore } from '@nanostores/react' -import { type CSSProperties, Fragment, type ReactNode, type RefObject, useRef, useState } from 'react' +import { + type CSSProperties, + Fragment, + type ReactNode, + type PointerEvent as ReactPointerEvent, + type RefObject, + useRef, + useState +} from 'react' import { ActionsContextMenu, type MenuKit, renderActionItem } from '@/components/ui/actions-menu' import { Codicon } from '@/components/ui/codicon' @@ -71,7 +79,7 @@ import { toggleTabSelected } from '../tab-selection' -import { type DoubleTapContext, startPaneDrag } from './drag-session' +import { DOUBLE_TAP_MS, type DoubleTapContext, startPaneDrag } from './drag-session' import { forceLoneHeaderForPanes } from './lone-header' import { useActiveTabVisible } from './tab-strip-scroll' import { paneChrome } from './track-model' @@ -291,9 +299,14 @@ export function TreeGroup({ tabCount: shown.length }) - // Drag handles preventDefault pointerdown (no native dblclick), so the - // header + chips share a synthesized double-tap: restore if collapsed - // (undoing the first tap's minimize toggle) and hide the chrome. + // The STRIP background keeps the synthesized double-tap hide — it is the + // documented explicit gesture (model.ts) and the open PR #84458 reveal edge + // is its recovery for that surface. TABS must not carry it: a tab is where + // people double-click for mundane reasons (select a title, retry a click), + // and every tab drag forwarded the gesture, so a routine double-click + // vanished the whole bar and stranded the zone with no ✕ and no way back + // but a right-click. The body below carries the inverse gesture as the + // recovery path when a strip IS hidden. const hideHeaderDoubleTap: DoubleTapContext = { key: `hide-header-${node.id}`, onDoubleTap: () => { @@ -302,6 +315,38 @@ export function TreeGroup({ } } + // Recovery for an explicitly hidden strip: double-tap the zone BODY brings + // the bar back. The body is otherwise inert (panes own their clicks; drags + // engage past a movement threshold), so the gesture can't fire by accident. + const showHeaderDoubleTap: DoubleTapContext = { + key: `show-header-${node.id}`, + onDoubleTap: () => setTreeGroupHeaderHidden(node.id, false) + } + + const bodyTapRef = useRef<{ time: number; x: number; y: number } | null>(null) + + const onBodyPointerDown = (event: ReactPointerEvent) => { + if (event.button !== 0 || !headerHidden || node.minimized || editMode) { + return + } + + const now = Date.now() + const last = bodyTapRef.current + + if ( + last && + now - last.time < DOUBLE_TAP_MS && + Math.hypot(event.clientX - last.x, event.clientY - last.y) < 24 + ) { + bodyTapRef.current = null + showHeaderDoubleTap.onDoubleTap() + + return + } + + bodyTapRef.current = { time: now, x: event.clientX, y: event.clientY } + } + // Zone-menu close targets read the layout tree, but this component must NOT // subscribe to it: `useStore($layoutTree)` here wires every zone — and // therefore every mounted pane and its whole transcript — to the entire @@ -355,11 +400,10 @@ export function TreeGroup({ targetPane } - // NO body double-click toggle: virtualized content (the thread) recreates - // its nodes between clicks, so the gesture was hopelessly unreliable. The - // bar's lifecycle is explicit instead — gaining a tab sticky-shows it - // (insertAtGroup pins headerHidden false), the main tab's context menu - // hides it, and full-page views veto it via paneChrome.headerVeto. + // The bar's lifecycle is explicit: gaining a tab sticky-shows it + // (insertAtGroup pins headerHidden false), the zone menu (and the main + // tab's menu) hide it, full-page views veto it via paneChrome.headerVeto, + // and a double-tap on the body restores it while hidden. return (
{ setMenuPane((e.target as HTMLElement).closest('[data-tree-tab]')?.getAttribute('data-tree-tab') ?? undefined) }} + onPointerDown={onBodyPointerDown} ref={ref} style={wcOverlap ? { paddingTop: wcOverlap.y + wcOverlap.height } : undefined} > @@ -442,8 +487,9 @@ export function TreeGroup({ listRef={tabsRef} onPointerDown={e => // Tap the header to collapse to it / expand back — the DetailPane - // / sidebar-section gesture (never for the main zone). Double-tap - // hides the header entirely. Drag still moves the pane. + // / sidebar-section gesture (never for the main zone). The + // double-tap hide rides the strip background below, not the tabs. + // Drag still moves the pane. startPaneDrag( activeId, e, @@ -545,7 +591,7 @@ export function TreeGroup({ e, onTap, stripRef.current ? { groupId: node.id, strip: stripRef.current } : undefined, - hideHeaderDoubleTap, + undefined, t.zones.tabCount(dragSelection.length), dragSelection ) @@ -557,13 +603,13 @@ export function TreeGroup({ // session drop language — link/stack/split); `false` defers // to the generic pane move (the workspace tab on a fresh // draft has no session to link). - if (!chrome.tabDrag?.(e, onTap, hideHeaderDoubleTap)) { + if (!chrome.tabDrag?.(e, onTap)) { startPaneDrag( paneId, e, onTap, stripRef.current ? { groupId: node.id, strip: stripRef.current } : undefined, - hideHeaderDoubleTap, + undefined, title ) }