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.
This commit is contained in:
committed by
Brooklyn Nicholson
parent
2584b7c4ec
commit
3aeb592863
@@ -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.
|
||||
*/
|
||||
|
||||
@@ -616,9 +616,9 @@ export const watchSessionTiles = paneMirror<SessionTile>({
|
||||
</SessionTabMenu>
|
||||
),
|
||||
// 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
|
||||
},
|
||||
|
||||
@@ -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) => (
|
||||
<div className="contents" data-zone-no-header>
|
||||
<Suspense fallback={null}>{view}</Suspense>
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 <TreeGroup node={zoneAt(0)} parentAxis="column" />
|
||||
}
|
||||
|
||||
// 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(<LiveTreeGroup />)
|
||||
|
||||
const tab = document.querySelector<HTMLElement>('[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(<LiveTreeGroup />)
|
||||
|
||||
// Hidden strip: no tablist, no ✕ anywhere in the zone.
|
||||
expect(tablist()).toBeNull()
|
||||
|
||||
const body = document.querySelector<HTMLElement>('[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(<LiveTreeGroup />)
|
||||
|
||||
const body = document.querySelector<HTMLElement>('[data-tree-group="grp-tools"]')!
|
||||
fireEvent.pointerDown(body, { button: 0, pointerType: 'mouse' })
|
||||
|
||||
expect(tablist()).toBeNull()
|
||||
expect(groupNode().headerHidden).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -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<HTMLDivElement>) => {
|
||||
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 (
|
||||
<div
|
||||
@@ -376,6 +420,7 @@ export function TreeGroup({
|
||||
onContextMenu={e => {
|
||||
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
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user