From e058751962bc6fc4e07a2d45c63200b73a471009 Mon Sep 17 00:00:00 2001 From: Yaojia Wang Date: Wed, 17 Jun 2026 15:45:18 +0200 Subject: [PATCH] fix(v0.2): tab switching needed multiple clicks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: whole tab was draggable=true, so a click with any micro-movement became a (no-op) drag instead of a click → switch failed. Plus every event (activity/title/status) rebuilt the entire tab bar via replaceChildren, which could destroy the element mid-click. Fix: - switch on pointerdown (fires before drag arms; also works on touch) - build tab DOM once; update classes/dot/label IN PLACE (refreshTab); full rebuild only on structural change (add/close/reorder/rename) - close/rename-input stopPropagation on pointerdown so they don't re-activate Verified: single pointerdown/click switches reliably across tabs; rename + drag still work. 198 tests green. --- public/tabs.ts | 198 ++++++++++++++++++++++++++----------------------- 1 file changed, 105 insertions(+), 93 deletions(-) diff --git a/public/tabs.ts b/public/tabs.ts index 00a9fca..6a4f673 100644 --- a/public/tabs.ts +++ b/public/tabs.ts @@ -3,17 +3,17 @@ * * Each tab owns an independent TerminalSession (own WS + server session). * - * Titles (learned from Warp / VS Code / Arc): - * - auto title from the terminal (xterm onTitleChange = OSC 0/2: cwd/process) - * - double-click a tab to rename inline (Enter commit, Esc cancel) - * - manual title wins over auto; empty rename reverts to auto/"Term N" - * Interaction: - * - middle-click a tab to close (browser/iTerm2 convention) - * - × button (hover on desktop, always shown on touch) - * - long titles ellipsis-truncate with a full-title tooltip + * Titles: auto from the terminal (current folder, via folderFromTitle) → + * double-click to rename inline → falls back to "Term N". Manual wins. + * Interaction: pointerdown to switch (robust with draggable), drag to reorder, + * middle-click / × to close, status dot (connection color), unread dot. + * + * DOM is built once per tab and updated in place. A full rebuild happens ONLY + * on structural changes (add / close / reorder / rename) — never on a plain + * activate or on background output — so clicks are never eaten by a re-render. * * Closing a tab only disconnects its WS (a *detach*); the server PTY keeps - * running and is reclaimed by IDLE_TTL — consistent with session survival. + * running and is reclaimed by IDLE_TTL. */ import { TerminalSession } from './terminal-session.js' @@ -24,15 +24,12 @@ const LEGACY_KEY = 'web-terminal:sessionId' // single-session key from v0.1 interface TabEntry { session: TerminalSession - /** User-set title; null = use auto/fallback. Persisted. */ - customTitle: string | null - /** Latest OSC title from the terminal; null until the shell sets one. */ - autoTitle: string | null - /** True when an inactive tab received output since last viewed (unread dot). */ - hasActivity: boolean + customTitle: string | null // user-set; null = use auto/fallback (persisted) + autoTitle: string | null // current folder from the terminal title + hasActivity: boolean // inactive tab got output since last viewed + el: HTMLDivElement | null // the .tab element (updated in place) } -/** Persisted shape per tab. */ interface StoredTab { id: string | null title: string | null @@ -56,12 +53,11 @@ export class TabApp { return entry.customTitle ?? entry.autoTitle ?? `Term ${idx + 1}` } + /* ── persistence ─────────────────────────────────────────────── */ + private persist(): void { try { - const stored: StoredTab[] = this.tabs.map((t) => ({ - id: t.session.id, - title: t.customTitle, - })) + const stored: StoredTab[] = this.tabs.map((t) => ({ id: t.session.id, title: t.customTitle })) localStorage.setItem(TABS_KEY, JSON.stringify(stored)) localStorage.setItem(ACTIVE_KEY, String(this.activeIndex)) } catch { @@ -77,8 +73,7 @@ export class TabApp { const parsed: unknown = JSON.parse(raw) if (Array.isArray(parsed)) { stored = parsed.map((v): StoredTab => { - // v0.2.0 stored bare ids (string|null); v0.2.1+ stores {id,title}. - if (typeof v === 'string') return { id: v, title: null } + if (typeof v === 'string') return { id: v, title: null } // v0.2.0 format if (v && typeof v === 'object' && 'id' in v) { const o = v as { id: unknown; title?: unknown } return { @@ -95,7 +90,6 @@ export class TabApp { } if (stored.length === 0) { - // First run: seed from the v0.1 single-session key if present. let legacy: string | null = null try { legacy = localStorage.getItem(LEGACY_KEY) @@ -105,9 +99,7 @@ export class TabApp { stored = [{ id: legacy, title: null }] } - for (const s of stored) { - this.createTab(s.id, s.title, false) - } + for (const s of stored) this.addEntry(s.id, s.title) let active = 0 try { @@ -115,19 +107,20 @@ export class TabApp { } catch { active = 0 } - if (!Number.isInteger(active) || active < 0 || active >= this.tabs.length) { - active = 0 - } + if (!Number.isInteger(active) || active < 0 || active >= this.tabs.length) active = 0 + this.rebuild() this.activate(active) - this.render() } - private createTab(sessionId: string | null, customTitle: string | null, activate: boolean): void { + /* ── tab lifecycle ───────────────────────────────────────────── */ + + private addEntry(sessionId: string | null, customTitle: string | null): TabEntry { const entry: TabEntry = { session: null as unknown as TerminalSession, customTitle, autoTitle: null, hasActivity: false, + el: null, } entry.session = new TerminalSession({ sessionId, @@ -135,37 +128,55 @@ export class TabApp { // onActivity only fires for hidden (inactive) panes (see TerminalSession). onActivity: () => { entry.hasActivity = true - this.render() + this.refreshTab(entry) }, onTitle: (title) => { entry.autoTitle = title.trim() || null - this.render() + this.refreshTab(entry) }, - onStatus: () => this.render(), + onStatus: () => this.refreshTab(entry), }) this.paneHost.appendChild(entry.session.el) this.tabs.push(entry) entry.session.connect() - if (activate) this.activate(this.tabs.length - 1) - this.persist() + return entry } newTab(): void { - this.createTab(null, null, true) - this.render() + this.addEntry(null, null) + this.persist() + this.rebuild() + this.activate(this.tabs.length - 1) } activate(i: number): void { if (i < 0 || i >= this.tabs.length) return this.activeIndex = i const entry = this.tabs[i] - if (entry) entry.hasActivity = false // viewing it clears the unread dot + if (entry) entry.hasActivity = false // viewing clears the unread dot this.tabs.forEach((t, idx) => (idx === i ? t.session.show() : t.session.hide())) + this.tabs.forEach((t) => this.refreshTab(t)) // in-place class/text update, no rebuild this.persist() - this.render() } - /** Drag-reorder: move the tab at `from` to position `to`, preserving the active tab. */ + closeTab(i: number): void { + if (i < 0 || i >= this.tabs.length) return + const [entry] = this.tabs.splice(i, 1) + entry?.session.dispose() + if (this.editingIndex === i) this.editingIndex = -1 + if (this.tabs.length === 0) { + this.addEntry(null, null) + this.persist() + this.rebuild() + this.activate(0) + return + } + const next = Math.min(i, this.tabs.length - 1) + this.persist() + this.rebuild() + this.activate(next) + } + private moveTab(from: number, to: number): void { if (from < 0 || to < 0 || from === to || from >= this.tabs.length || to >= this.tabs.length) { return @@ -176,21 +187,7 @@ export class TabApp { this.tabs.splice(to, 0, moved) if (activeEntry) this.activeIndex = this.tabs.indexOf(activeEntry) this.persist() - this.render() - } - - closeTab(i: number): void { - if (i < 0 || i >= this.tabs.length) return - const [entry] = this.tabs.splice(i, 1) - entry?.session.dispose() - if (this.editingIndex === i) this.editingIndex = -1 - if (this.tabs.length === 0) { - this.createTab(null, null, true) - } else { - this.activate(Math.min(i, this.tabs.length - 1)) - } - this.persist() - this.render() + this.rebuild() } private commitRename(i: number, value: string): void { @@ -201,7 +198,7 @@ export class TabApp { } this.editingIndex = -1 this.persist() - this.render() + this.rebuild() } /** Route key-bar input to the currently active tab. */ @@ -209,19 +206,35 @@ export class TabApp { this.tabs[this.activeIndex]?.session.send(data) } - private render(): void { + /* ── rendering ───────────────────────────────────────────────── */ + + /** In-place update of one tab's classes/label/dot — never destroys the DOM. */ + private refreshTab(entry: TabEntry): void { + const el = entry.el + if (!el) return + const idx = this.tabs.indexOf(entry) + el.classList.toggle('active', idx === this.activeIndex) + el.classList.toggle('unread', entry.hasActivity && idx !== this.activeIndex) + const title = this.displayTitle(entry, idx) + el.title = title + const dot = el.querySelector('.tab-dot') + if (dot) dot.className = `tab-dot dot-${entry.session.status}` + const label = el.querySelector('.tab-label') + if (label) label.textContent = title + } + + /** Full rebuild — ONLY for structural changes (add/close/reorder/rename). */ + private rebuild(): void { this.tabBar.replaceChildren() this.tabs.forEach((entry, idx) => { const tabEl = document.createElement('div') - const classes = ['tab'] - if (idx === this.activeIndex) classes.push('active') - if (entry.hasActivity) classes.push('unread') - tabEl.className = classes.join(' ') - const title = this.displayTitle(entry, idx) - tabEl.title = title // full-title tooltip (truncation handled by CSS) + entry.el = tabEl + tabEl.className = 'tab' - // Middle-click anywhere on the tab closes it. + // Switch on pointerdown (fires before any drag; reliable with draggable, works on touch). + tabEl.addEventListener('pointerdown', () => this.activate(idx)) + // Middle-click closes. tabEl.addEventListener('auxclick', (e) => { if (e.button === 1) { e.preventDefault() @@ -229,7 +242,7 @@ export class TabApp { } }) - // Drag-to-reorder (disabled while renaming so the input is usable). + // Drag-to-reorder (disabled while renaming). if (idx !== this.editingIndex) { tabEl.draggable = true tabEl.addEventListener('dragstart', (e) => { @@ -253,7 +266,6 @@ export class TabApp { }) } - // Status dot: color = connection state; brightened when unread. const dot = document.createElement('span') dot.className = `tab-dot dot-${entry.session.status}` tabEl.appendChild(dot) @@ -261,47 +273,47 @@ export class TabApp { if (idx === this.editingIndex) { const input = document.createElement('input') input.className = 'tab-rename' - input.value = title + input.value = this.displayTitle(entry, idx) input.addEventListener('keydown', (e) => { if (e.key === 'Enter') this.commitRename(idx, input.value) else if (e.key === 'Escape') { this.editingIndex = -1 - this.render() + this.rebuild() } e.stopPropagation() }) input.addEventListener('blur', () => this.commitRename(idx, input.value)) + input.addEventListener('pointerdown', (e) => e.stopPropagation()) // don't re-activate tabEl.appendChild(input) - this.tabBar.appendChild(tabEl) - // focus + select after it's in the DOM requestAnimationFrame(() => { input.focus() input.select() }) - return + } else { + const label = document.createElement('span') + label.className = 'tab-label' + label.textContent = this.displayTitle(entry, idx) + label.addEventListener('dblclick', () => { + this.editingIndex = idx + this.rebuild() + }) + tabEl.appendChild(label) + + const close = document.createElement('button') + close.className = 'tab-close' + close.textContent = '×' + close.title = 'Close tab (the shell keeps running on the server)' + close.setAttribute('aria-label', 'Close tab') + close.addEventListener('pointerdown', (e) => e.stopPropagation()) + close.addEventListener('click', (e) => { + e.stopPropagation() + this.closeTab(idx) + }) + tabEl.appendChild(close) } - const label = document.createElement('span') - label.className = 'tab-label' - label.textContent = title - label.addEventListener('click', () => this.activate(idx)) - label.addEventListener('dblclick', () => { - this.editingIndex = idx - this.render() - }) - tabEl.appendChild(label) - - const close = document.createElement('button') - close.className = 'tab-close' - close.textContent = '×' - close.title = 'Close tab (the shell keeps running on the server)' - close.setAttribute('aria-label', 'Close tab') - close.addEventListener('click', (e) => { - e.stopPropagation() - this.closeTab(idx) - }) - tabEl.appendChild(close) - + // Apply current active/unread state. + this.refreshTab(entry) this.tabBar.appendChild(tabEl) })