fix(v0.2): tab switching needed multiple clicks

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.
This commit is contained in:
Yaojia Wang
2026-06-17 15:45:18 +02:00
parent bf21ac0ab7
commit e058751962

View File

@@ -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)
})