diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 472c868a..7fe8e59c 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -303,9 +303,9 @@ So: `_confirmIdle()` (session.ts) requires the pane to go quiet, and then asks t - **Collapse is per-device** (`codeman:tab-groups-collapsed` in localStorage, ids of deleted groups garbage-collected on adoption). A store that throws means all-expanded; a malformed stored VALUE reads as empty and is rewritten, so it can never disable collapse on that device for good. A collapsed group still SHOWS the active row, and `_updateActiveTabImmediate` falls through to a full render whenever the structure key changes, since a class toggle cannot reveal a hidden row. - **A collapsed header carries the most urgent alert it hides** (`hiddenGroupAlerts()`, applied by `_syncTabGroupHeaderAlerts` on BOTH render paths, since alerts change without a rebuild), in the tab alert language: `tab-alert-action` red, `tab-alert-idle` yellow. A permission prompt behind a collapse must never be invisible. - **Lineage arcs to a collapse-hidden session anchor to its group header** (`lineage-line--proxied`); two endpoints proxied to one header draw nothing. -- **The upstream HTML5 drag stays off in the grouped rail**: a flat-order drop cannot express a group move, and the server re-ranks within the old group. The grouped rail has its OWN pointer drag instead (`_bindTabLayoutPointerDrag`, mouse/pen only, bound once on the container): rows move before/after a row or into a group, a header drag reorders groups, and the drop maps to ONE operation through the pure `dropOperation()`. Escape cancels a drag, and the click that ends one is swallowed. The flat rail and the header strip keep the HTML5 drag untouched. For the same reason Ctrl+Shift+{ / } only swaps with a neighbour in the active session's own section (`_canSwapActiveTabWith`, reading the projection's `sectionByRef`): a cross-group swap moves nothing on the server, gets no `session:orderChanged` back, and would leave this client's `sessionOrder` and Alt+N targets out of step with every other device. -- **Edits are named operations, saved serially.** `createEditCoordinator` applies `createGroup` / `renameGroup` / `deleteGroup` / `reorderGroup` / `moveRef` to the rail at once, then sends ONE `PUT /api/tab-layout {baseVersion, layout}` at a time; edits made meanwhile wait and go out on the version that write returns. A 409 carries the server's layout: the in-flight operations are replayed onto it (an operation that no longer applies is dropped and reported) and re-sent, at most `maxAttempts` times; a 400 re-reads first; anything else reports and re-reads. A `moveRef` moves the session together with the sessions that still follow it and marks a hand-moved child `placement: 'manual'`, mirroring the server's `moveRef`, and `normalizeLayout` keeps `placement` because whole layouts are written back. `_onTabLayoutChanged` / `_applyTabLayout` defer a read while a write is in flight and rebase unsaved edits onto a read otherwise. On `pagehide`, unconfirmed operations go out in a `keepalive` PUT AND into sessionStorage; after reload they replay onto the fresh layout, which is a no-op when the keepalive landed. -- **Every way in has a keyboard path.** The session row menu (Shift+F10 in the tree, the rail's overflow button) gains Move up/down, Move to , Move to Ungrouped and Move to new group in the vertical rail (only "new group" before the first group exists; nothing on the strip). A group header opens its menu with Shift+F10 / ContextMenu, right-click or its hover glyph (a non-focusable, `aria-hidden` span: a treeitem holds no interactive children), and F2 renames it inline. The menu closes on Escape (which it consumes before the global Escape handler), a pointer outside, Tab, focus leaving it, a resize, a second open and any full re-render. The inline group editor shares `_activeRename` with the session rename, so only the CURRENT editor may release `_inlineRenameActive`. +- **The upstream HTML5 drag stays off in the grouped rail**: a flat-order drop cannot express a group move, and the server re-ranks within the old group. The grouped rail has its OWN pointer drag instead (`_bindTabLayoutPointerDrag`, mouse/pen only, bound once on the container): rows move before/after a row or into a group, a header drag reorders groups, and the drop maps to ONE operation through the pure `dropOperation()`. Escape cancels a drag, and the click that ends one is swallowed. ⚠️ A press is only captured once it moves 6 px, so its release can land outside the rail: `pointerup`/`pointercancel` are heard on `window` while a press is pending, a move with the primary button up cancels it, and a new press cancels any previous one. Without that a stale press became a phantom drag on the next hover, and a replaced drag left its capture-phase Escape listener behind, swallowing every Escape before the terminal saw it. The flat rail and the header strip keep the HTML5 drag untouched. For the same reason Ctrl+Shift+{ / } only swaps with a neighbour in the active session's own section (`_canSwapActiveTabWith`, reading the projection's `sectionByRef`): a cross-group swap moves nothing on the server, gets no `session:orderChanged` back, and would leave this client's `sessionOrder` and Alt+N targets out of step with every other device. +- **Edits are named operations, saved serially.** `createEditCoordinator` applies `createGroup` / `renameGroup` / `deleteGroup` / `reorderGroup` / `moveRef` to the rail at once, then sends ONE `PUT /api/tab-layout {baseVersion, layout}` at a time; edits made meanwhile wait and go out on the version that write returns. A 409 carries the server's layout: the in-flight operations are replayed onto it (an operation that no longer applies is dropped and reported) and re-sent, at most `maxAttempts` times; a 400 re-reads once (a second 400 is reported as a failed save, not as a race); anything else reports and re-reads. A `moveRef` moves the session together with the sessions that still follow it and marks a hand-moved child `placement: 'manual'`, mirroring the server's `moveRef`, and `normalizeLayout` keeps `placement` because whole layouts are written back. `_onTabLayoutChanged` / `_applyTabLayout` defer a read while a write is in flight and rebase unsaved edits onto a read otherwise. ⚠️ A FAILED read (`_applyTabLayout(null)`) while edits are pending keeps the held layout and the editor and re-reads after the write settles: disposing there would orphan the in-flight write, whose 409 then never gets its rebase. Any path that does drop unsaved work says so in a toast. On `pagehide`, unconfirmed operations go out in a `keepalive` PUT AND into sessionStorage as `{ owner, baseVersion, savedAt, operations }`; after reload they replay onto the fresh layout (a no-op when the keepalive landed), but only for the same owner, within `TAB_LAYOUT_PENDING_MAX_AGE_MS` (60 s), and never onto a layout older than the copy's base. A "Move to " with no anchor carries no `index`, so a replay still puts the row last. +- **Every way in has a keyboard path.** The session row menu (Shift+F10 in the tree, the rail's overflow button) gains Move up/down, Move to , Move to Ungrouped and Move to new group in the vertical rail (only "new group" before the first group exists; nothing on the strip). A group header opens its menu with Shift+F10 / ContextMenu, right-click or its hover glyph (a non-focusable, `aria-hidden` span: a treeitem holds no interactive children), and F2 renames it inline (Enter commits and refocuses the header; a commit by BLUR leaves focus where it went, since refocusing from inside the blur handler overrides the user's click). The glyph stays visible under `@media (hover: none)`: a touch tablet has no hover and no long-press `contextmenu`. Group names in "Move to" labels are quoted, so a group named "New group" or "ungrouped" cannot read (or translate, case-insensitively) like the fixed entries. The menu closes on Escape (which it consumes before the global Escape handler), a pointer outside, Tab, focus leaving it, a resize, a second open and any full re-render. It borrows the `.tab-rail-action-menu` class for its look only: `closeTabRailActionMenu()` excludes `.tab-layout-group-action-menu`, so closing the row menu (every `session:deleted` does) cannot strand the group menu's listeners. The inline group editor shares `_activeRename` with the session rename, so only the CURRENT editor may release `_inlineRenameActive`. - **Only the grouped rail is a tree.** `#sessionTabs` ships as `role=tablist` with `role=tab` rows, and the header strip, sidebar and flat rail keep exactly that. While grouped, `_applyTabListRole` makes it `role=tree` (and restores `tablist` + its label when grouping ends), named-group headers are level-1 `treeitem`s that `aria-owns` their rows' `role=group` (rows sit beside the header, not inside it), and ungrouped rows plus a collapsed group's kept selection are level-1 items. A collapsed header owns nothing, a group with no open rows is a leaf (no `aria-expanded`, no owned group), and the "Ungrouped" heading is `aria-hidden`. Rows are re-roled in the DOM by `_applyTabTreeSemantics` after render, never by rewriting their markup, so a grouped row's content stays the flat row's. - **One tab stop in the tree.** Exactly one treeitem carries `tabindex=0` (the focused or selected item); every control inside a row drops to `-1`, which is why Shift+F10 / ContextMenu open a row's actions from the keyboard. Focus survives a full re-render by identity (`group:`/`session:`/`webview:`; a row a collapse just hid hands focus to its header), but only when focus was already inside the rail. The tree walk (`_tabTreeItems`) follows painted order WITHIN each group when the rail is sorted; the flat list keeps its own whole-list computed-order walk. `aria-posinset`/`aria-setsize` follow painted order too, so the incremental render path re-runs `_applyTabTreePositions` after it re-sorts rows in place. ⚠️ `_handleTabTreeKeydown` acts only when the key lands on the treeitem ITSELF: a key on a focused in-row control (close, overflow, the rename input) is that control's, or Enter on the overflow button re-selects the row instead of reopening its menu. ⚠️ The roving `tabindex=-1` also hides every item but the stop from the keyboard-dismiss selector's `[tabindex]` arm, which is why `MOBILE_KEYBOARD_DISMISS_EXEMPT_SELECTOR` lists `[role="treeitem"]` (see Dismissing the on-screen keyboard). diff --git a/src/web/public/app.js b/src/web/public/app.js index 00fc2ec4..b2688793 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -577,6 +577,13 @@ const SIDEBAR_RICH_CLOCK_MS = 20000; */ const URL_SESSION_WAIT_MS = 30000; +/** + * How old the sessionStorage copy of unsaved tab-group edits may be when the + * next page replays it (see _restorePendingTabLayoutEdits). A reload takes + * seconds; an older copy is from a tab that sat closed or a different visit. + */ +const TAB_LAYOUT_PENDING_MAX_AGE_MS = 60000; + class CodemanApp { constructor() { this.sessions = new Map(); @@ -6335,7 +6342,7 @@ class CodemanApp { } // ═══════════════════════════════════════════════════════════════ - // Owner tab layout: grouped vertical rail (read-only) + // Owner tab layout: grouped vertical rail (reading and drawing) // ═══════════════════════════════════════════════════════════════ // // The server owns named tab groups (GET /api/tab-layout, tab-layout*.ts) and @@ -6343,7 +6350,7 @@ class CodemanApp { // Ctrl+Tab and every other order consumer are untouched here. This layer only // decides how the VERTICAL rail draws rows: in sections, with per-device // collapse. With no groups (or any read failure) the rail is the flat list it - // has always been. + // has always been. Editing the groups is the next block. _ensureTabLayoutCoordinator() { if (this._tabLayoutCoordinator) return this._tabLayoutCoordinator; @@ -6403,6 +6410,13 @@ class CodemanApp { if (next && this.tabLayout && next.version < this.tabLayout.version) return; const editor = this._tabLayoutEditor; if (editor) { + // A failed read says nothing about the layout, and dropping the editor now + // would lose the edit outright: a write in flight would never get its 409 + // rebased. Keep the held layout and the editor; read again once it settles. + if (!next && editor.hasPending()) { + this._tabLayoutReloadPending = true; + return; + } if (next && editor.isWriting()) { // The write's own response decides; read again after it. this._tabLayoutReloadPending = true; @@ -6411,6 +6425,7 @@ class CodemanApp { // Unsaved edits are rebased onto the read (adoptExternal repaints); with // none, the editor is simply rebuilt from the new layout on next use. if (next && editor.hasPending() && editor.adoptExternal(next)) return; + if (editor.hasPending()) this.showToast?.('Your tab group edit was not saved.', 'error'); editor.dispose(); this._tabLayoutEditor = null; } @@ -6608,7 +6623,11 @@ class CodemanApp { const groups = this.tabLayout?.groups || []; const index = groups.findIndex((group) => group.id === groupId); if (index < 0) return false; - if (!window.confirm(`Delete group "${groups[index].name}"? Its tabs move to Ungrouped.`)) return false; + if (!window.confirm(`Delete group "${groups[index].name}"? Its tabs move to Ungrouped.`)) { + // The menu that asked is gone; put the keyboard back on the group. + this.$('sessionTabs')?.querySelector(`[data-tab-group-header="${CSS.escape(groupId)}"]`)?.focus(); + return false; + } const neighbour = groups[index + 1] || groups[index - 1]; return this.editTabLayout({ type: 'deleteGroup', groupId }, neighbour ? `group:${neighbour.id}` : null); } @@ -6667,7 +6686,10 @@ class CodemanApp { } for (const group of this.tabLayout.groups) { if (group.id === location.groupId) continue; - actions.push({ label: `Move to ${group.name}`, run: () => this.moveTabRef(ref, group.id) }); + // Quoted: a group may be NAMED "New group" or "ungrouped", which unquoted + // would read (and, case-insensitively, translate) exactly like the + // fixed "Move to new group" / "Move to Ungrouped" entries next to it. + actions.push({ label: `Move to "${group.name}"`, run: () => this.moveTabRef(ref, group.id) }); } if (location.groupId !== null) actions.push({ label: 'Move to Ungrouped', run: () => this.moveTabRef(ref, null) }); actions.push({ label: 'Move to new group', run: () => this.createTabGroup({ ref }) }); @@ -6842,7 +6864,10 @@ class CodemanApp { let settled = false; const handle = { groupId, cancel: () => settle(false) }; - const settle = (commit) => { + // `fromBlur`: focus already moved somewhere the user chose (the terminal, + // another control). Pulling it back to the header from inside the blur + // handler wins over that move, so a blur commit never asks for refocus. + const settle = (commit, { fromBlur = false } = {}) => { if (settled) return; settled = true; const name = input.value.trim(); @@ -6851,11 +6876,14 @@ class CodemanApp { this._activeRename = null; this._inlineRenameActive = false; const current = this.tabLayout?.groups?.find((candidate) => candidate.id === groupId); + const focusIdentity = fromBlur ? null : `group:${groupId}`; if (commit && current && name && name !== current.name) { - if (this.editTabLayout({ type: 'renameGroup', groupId, name }, `group:${groupId}`)) return; + if (this.editTabLayout({ type: 'renameGroup', groupId, name }, focusIdentity)) return; + } + if (focusIdentity) { + this._tabFocusIdentity = focusIdentity; + this._tabRefocusAfterEdit = true; } - this._tabFocusIdentity = `group:${groupId}`; - this._tabRefocusAfterEdit = true; this._fullRenderSessionTabs(); }; this._activeRename = handle; @@ -6870,7 +6898,7 @@ class CodemanApp { settle(false); } }); - input.addEventListener('blur', () => settle(true)); + input.addEventListener('blur', () => settle(true, { fromBlur: true })); input.focus(); input.select(); return true; @@ -6896,6 +6924,11 @@ class CodemanApp { } _onTabLayoutPointerDown(e, container) { + // A press whose release never reached us (let go outside the rail, or + // outside the window) must not survive into this one: a stale pending + // press turned into a phantom drag on the next hover, and a replaced drag + // left its Escape listener behind for good. + if (this._tabLayoutDrag) this._cancelTabLayoutPointerDrag(container); if (e.button !== 0 || e.pointerType === 'touch' || !container.classList.contains('session-tabs--grouped')) return; if (this._inlineRenameActive || !this._tabLayoutEditable()) return; // Controls keep their own click; only the row body or the header drags. @@ -6907,7 +6940,14 @@ class CodemanApp { else if (row?.dataset.webviewId) source = { type: 'ref', ref: { kind: 'webview', id: row.dataset.webviewId } }; else if (row?.dataset.id) source = { type: 'ref', ref: { kind: 'session', id: row.dataset.id } }; if (!source) return; - this._tabLayoutDrag = { pointerId: e.pointerId, x: e.clientX, y: e.clientY, source, origin: header || row, active: false, target: null }; + const drag = { pointerId: e.pointerId, x: e.clientX, y: e.clientY, source, origin: header || row, active: false, target: null }; + // Capture is only taken once the press becomes a drag, so until then the + // release can land outside the rail: hear it on window. + drag.windowUp = (upEvent) => this._finishTabLayoutPointerDrag(upEvent, container); + drag.windowCancel = () => this._cancelTabLayoutPointerDrag(container); + window.addEventListener('pointerup', drag.windowUp, true); + window.addEventListener('pointercancel', drag.windowCancel, true); + this._tabLayoutDrag = drag; } /** What a pointer at (x, y) would drop onto, from the rendered rail. */ @@ -6938,6 +6978,11 @@ class CodemanApp { _onTabLayoutPointerMove(e, container) { const drag = this._tabLayoutDrag; if (!drag || drag.pointerId !== e.pointerId) return; + // The primary button is up, so the release went somewhere we never heard. + if ((e.buttons & 1) === 0) { + this._cancelTabLayoutPointerDrag(container); + return; + } if (!drag.active) { if (Math.hypot(e.clientX - drag.x, e.clientY - drag.y) < 6) return; drag.active = true; @@ -6948,6 +6993,7 @@ class CodemanApp { try { container.setPointerCapture(e.pointerId); } catch {} + if (this._tabLayoutDragKeydown) document.removeEventListener('keydown', this._tabLayoutDragKeydown, true); this._tabLayoutDragKeydown = (keyEvent) => { if (keyEvent.key !== 'Escape') return; keyEvent.preventDefault(); @@ -6969,6 +7015,8 @@ class CodemanApp { const drag = this._tabLayoutDrag; if (!drag) return; this._tabLayoutDrag = null; + if (drag.windowUp) window.removeEventListener('pointerup', drag.windowUp, true); + if (drag.windowCancel) window.removeEventListener('pointercancel', drag.windowCancel, true); drag.origin?.classList.remove('tab-layout-dragging'); container?.classList.remove('tab-layout-drag-active'); if (container) this._clearTabLayoutDropMarks(container); @@ -7010,7 +7058,10 @@ class CodemanApp { const operations = editor?.pendingOperations?.() || []; if (!operations.length) return false; try { - sessionStorage.setItem('codeman:tab-layout-pending', JSON.stringify({ operations })); + sessionStorage.setItem( + 'codeman:tab-layout-pending', + JSON.stringify({ owner: this._tabLayoutOwnerKey(), baseVersion: editor.baseVersion(), savedAt: Date.now(), operations }) + ); } catch {} try { const layout = editor.getLayout(); @@ -7024,17 +7075,53 @@ class CodemanApp { return true; } + /** Whose layout this page edits, as the server keys it (`@single` without multi-user). */ + _tabLayoutOwnerKey() { + const me = window.__codemanUser; + if (!me) return null; + return me.multiUser ? me.username : '@single'; + } + + /** + * Replay the previous page's unsaved edits, but only that page's: the copy is + * ignored when it belongs to another owner (a different login in this tab), + * is older than a reload could explain, or names a layout newer than the one + * just read (a different server behind the same origin). + */ _restorePendingTabLayoutEdits() { - let operations; + let saved; try { const raw = sessionStorage.getItem('codeman:tab-layout-pending'); if (!raw) return false; - sessionStorage.removeItem('codeman:tab-layout-pending'); - operations = JSON.parse(raw)?.operations; + saved = JSON.parse(raw); } catch { return false; } + const owner = this._tabLayoutOwnerKey(); + if (owner === null) { + // Who we are is not known yet (/api/me still loading): decide once it is. + if (!this._tabLayoutRestoreWaiting) { + this._tabLayoutRestoreWaiting = true; + document.addEventListener( + 'codeman:me', + () => { + this._tabLayoutRestoreWaiting = false; + this._restorePendingTabLayoutEdits(); + }, + { once: true } + ); + } + return false; + } + try { + sessionStorage.removeItem('codeman:tab-layout-pending'); + } catch {} + const operations = saved?.operations; if (!Array.isArray(operations) || !operations.length || !this.tabLayout || !window.CodemanTabLayout) return false; + if (saved.owner !== owner) return false; + const age = Date.now() - saved.savedAt; + if (!Number.isFinite(age) || age < 0 || age > TAB_LAYOUT_PENDING_MAX_AGE_MS) return false; + if (!Number.isSafeInteger(saved.baseVersion) || saved.baseVersion > this.tabLayout.version) return false; return this._ensureTabLayoutEditor().restore(operations); } @@ -7049,9 +7136,9 @@ class CodemanApp { // affordance instead of lying about it — `tabRailSort: 'manual'` is the way // back to drag-reordering, and Alt+N / Ctrl+Shift+{ } still walk the strip // order this list is no longer showing. - // The grouped rail is read-only for now: a flat-order drag cannot express - // "move into that group", and the server would re-rank it within its old - // group anyway. Grouped editing comes with its own drag model. + // The grouped rail opts out too: a flat-order drag cannot express "move + // into that group", and the server would re-rank it within its old group + // anyway. It has its own pointer drag (_bindTabLayoutPointerDrag). if (this.isTabRailSorted() || container.classList.contains('session-tabs--grouped')) { tabs.forEach((tab) => tab.setAttribute('draggable', 'false')); return; diff --git a/src/web/public/i18n.js b/src/web/public/i18n.js index 3b749394..cee646c8 100644 --- a/src/web/public/i18n.js +++ b/src/web/public/i18n.js @@ -85,6 +85,7 @@ 'Tab groups changed elsewhere; part of your edit no longer applies.': '标签分组已在别处更改;你的部分编辑已不再适用。', 'Tab groups kept changing elsewhere; your edit was not saved.': '标签分组在别处持续更改;你的编辑未保存。', + 'Your tab group edit was not saved.': '你的标签分组编辑未保存。', 'Open session manager': '打开会话管理器', Attachments: '附件', 'Open attachment history': '打开附件历史', @@ -952,7 +953,7 @@ [/^Selected: (.+)$/, (_m, value) => `已选择:${value}`], [/^Failed to (.+)$/, (_m, action) => `操作失败:${action}`], // Group names are user text: they pass through untranslated. - [/^Move to (.+)$/, (_m, group) => `移到 ${group}`], + [/^Move to "(.+)"$/, (_m, group) => `移到“${group}”`], [ /^Delete group "(.+)"\? Its tabs move to Ungrouped\.$/, (_m, group) => `删除分组“${group}”?其中的标签将移到未分组。`, diff --git a/src/web/public/styles.css b/src/web/public/styles.css index b69ba0bc..6dc388b3 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -752,6 +752,14 @@ html[data-tab-orientation='vertical'] .tab-rail .tab-layout-group-header:focus-v opacity: 1; } +/* No hover on a touch tablet, and a long press there is not a contextmenu + event: the glyph is the only way into the group menu, so keep it shown. */ +@media (hover: none) { + html[data-tab-orientation='vertical'] .tab-rail .tab-layout-group-menu { + opacity: 1; + } +} + html[data-tab-orientation='vertical'] .tab-rail .tab-layout-group-menu:hover { color: var(--text); background: var(--bg-tertiary, var(--bg-hover)); diff --git a/src/web/public/tab-layout-browser.js b/src/web/public/tab-layout-browser.js index c14fdb4e..6aa48939 100644 --- a/src/web/public/tab-layout-browser.js +++ b/src/web/public/tab-layout-browser.js @@ -427,9 +427,11 @@ const layout = normalizeLayout(layoutInput); const block = lineageBlock(layout, ref, parents); const remaining = containerRefs(layout, groupId).filter((candidate) => !block.has(refKey(candidate))); - if (!anchor) return { groupId, index: remaining.length }; + // No anchor means "at the end", and an operation with no index keeps + // meaning that when it is replayed onto a layout that has changed since. + if (!anchor) return { groupId }; const at = remaining.findIndex((candidate) => refKey(candidate) === refKey(anchor)); - if (at < 0) return { groupId, index: remaining.length }; + if (at < 0) return { groupId }; return { groupId, index: placement === 'after' ? at + 1 : at }; } @@ -482,7 +484,7 @@ type: 'moveRef', ref: { kind: source.ref.kind, id: source.ref.id }, groupId: destination.groupId, - index: destination.index, + ...(destination.index === undefined ? {} : { index: destination.index }), parents: parents || {}, }; return contentKey(applyOperation(layout, operation)) === contentKey(layout) ? null : operation; @@ -613,6 +615,7 @@ pending = []; let failed = false; let reportedDrop = false; + let rereadFor400 = false; try { for (let attempt = 0; attempt < maxAttempts && inFlight.length; attempt++) { const desired = replayOperations(authoritative, inFlight); @@ -633,7 +636,10 @@ inFlight = []; } else if (response?.status === 409 && response.layout) { authoritative = normalizeLayout(response.layout); - } else if (response?.status === 400 && options.fetchLayout) { + } else if (response?.status === 400 && options.fetchLayout && !rereadFor400) { + // Maybe our base was stale in a way the server reports as invalid: + // re-read once. A 400 that survives that is a refusal, not a race. + rereadFor400 = true; authoritative = normalizeLayout(await options.fetchLayout()); if (disposed) return; } else { diff --git a/src/web/public/tab-rail-resize.js b/src/web/public/tab-rail-resize.js index 51429675..ae906641 100644 --- a/src/web/public/tab-rail-resize.js +++ b/src/web/public/tab-rail-resize.js @@ -298,7 +298,9 @@ Object.assign(CodemanApp.prototype, { }, closeTabRailActionMenu(options = {}) { - const menu = document.querySelector('.tab-rail-action-menu'); + // The group menu borrows this class for its look but has its own owner + // (closeTabGroupMenu); removing its DOM here would strand its listeners. + const menu = document.querySelector('.tab-rail-action-menu:not(.tab-layout-group-action-menu)'); const trigger = this._tabRailActionMenuTrigger; menu?.remove(); if (this._tabRailActionMenuOutside) { diff --git a/test/i18n-branding.test.ts b/test/i18n-branding.test.ts index 45f95908..8ffff762 100644 --- a/test/i18n-branding.test.ts +++ b/test/i18n-branding.test.ts @@ -91,6 +91,18 @@ describe('custom display name and browser localization', () => { dom.window.close(); }); + it('keeps a quoted group name apart from the fixed "Move to" entries in zh-CN', () => { + const dom = makeDom(''); + const api = dom.window.CodemanI18n; + api.configure({ language: 'zh-CN' }); + const labels = ['Move to "New group"', 'Move to new group', 'Move to "ungrouped"', 'Move to Ungrouped'].map((label) => + api.t(label) + ); + expect(labels).toEqual(['移到“New group”', '移到新分组', '移到“ungrouped”', '移到未分组']); + expect(new Set(labels).size).toBe(4); + dom.window.close(); + }); + it('renders hostile-looking names as text rather than HTML', () => { const dom = makeDom(''); const api = dom.window.CodemanI18n; diff --git a/test/tab-layout-editing.browser.test.ts b/test/tab-layout-editing.browser.test.ts index 7cd53531..3e1f66da 100644 --- a/test/tab-layout-editing.browser.test.ts +++ b/test/tab-layout-editing.browser.test.ts @@ -65,6 +65,7 @@ describe('grouped rail editing in Chromium', () => {
+
`); @@ -124,6 +125,11 @@ describe('grouped rail editing in Chromium', () => { }; w.__setApp(app); w.__app = app; + // Escapes that reach the stand-in terminal (xterm listens on its textarea). + w.__termEscapes = 0; + document.getElementById('term')!.addEventListener('keydown', (e) => { + if (e.key === 'Escape') w.__termEscapes++; + }); }); }); @@ -151,6 +157,7 @@ describe('grouped rail editing in Chromium', () => { w.__app._applyTabLayout(layout); w.__activation = null; w.__panelsClosed = false; + w.__termEscapes = 0; }, LAYOUT); await page.mouse.move(1, 1); }); @@ -218,6 +225,104 @@ describe('grouped rail editing in Chromium', () => { expect(await page.evaluate(() => (window as any).__activation)).toBe('session:two'); }); + it('a press released outside the rail leaves nothing behind, and Escape still reaches the terminal', async () => { + const a = await box('.session-tab[data-id="one"]'); + // Press on a row and flick out of the rail in ONE move, releasing out there: + // the rail never sees the move or the release. + await page.mouse.move(a.x + 20, a.y + a.height / 2); + await page.mouse.down(); + await page.mouse.move(760, 600); + await page.mouse.up(); + // The release was heard on window: the press is gone before any hover. + expect(await page.evaluate(() => (window as any).__app._tabLayoutDrag)).toBeNull(); + // Hovering back with no button down must not turn into a phantom drag. + const b = await box('[data-tab-group-header="gy"]'); + await page.mouse.move(b.x + 30, b.y + b.height / 2, { steps: 6 }); + expect(await page.locator('.tab-layout-dragging, .tab-layout-drop-into, .tab-layout-drag-active').count()).toBe(0); + expect(await page.evaluate(() => (window as any).__app._tabLayoutDrag)).toBeNull(); + + // A real drag after that, cancelled with Escape, then one more completed. + await page.mouse.move(a.x + 20, a.y + a.height / 2); + await page.mouse.down(); + await page.mouse.move(b.x + 30, b.y + b.height / 2, { steps: 6 }); + await page.keyboard.press('Escape'); + await page.mouse.up(); + await drag('.session-tab[data-id="one"]', '[data-tab-group-header="gy"]'); + await settled(); + expect(puts).toHaveLength(1); + + // No drag listener is left in document capture swallowing Escape. + await page.locator('#term').focus(); + await page.keyboard.press('Escape'); + await page.keyboard.press('Escape'); + expect(await page.evaluate(() => (window as any).__termEscapes)).toBe(2); + expect(await page.evaluate(() => (window as any).__app._tabLayoutDragKeydown)).toBeNull(); + }); + + it('each guard holds on its own: buttons-up move, a second press, no stacked Escape listener', async () => { + // Synthetic pointer events reach the cases a real mouse cannot isolate (a + // release outside the WINDOW never reaches any listener of ours). + const result = await page.evaluate(() => { + const w = window as any; + const rail = document.getElementById('sessionTabs')!; + const at = (el: Element) => { + const r = el.getBoundingClientRect(); + return { clientX: r.left + 20, clientY: r.top + r.height / 2 }; + }; + const fire = (target: Element, type: string, init: PointerEventInit) => + target.dispatchEvent( + new PointerEvent(type, { bubbles: true, cancelable: true, pointerId: 7, pointerType: 'mouse', ...init }) + ); + const one = rail.querySelector('.session-tab[data-id="one"] .tab-name')!; + const three = rail.querySelector('.session-tab[data-id="three"] .tab-name')!; + const out: Record = {}; + + // 1. A pending press, then a move with no button down: cancelled, no drag. + fire(one, 'pointerdown', { button: 0, buttons: 1, ...at(one) }); + fire(three, 'pointermove', { buttons: 0, ...at(three) }); + out.afterButtonsUp = w.__app._tabLayoutDrag; + out.dragClass = rail.querySelectorAll('.tab-layout-dragging').length; + + // 2. An active drag whose release never arrived, then a new press. + fire(one, 'pointerdown', { button: 0, buttons: 1, ...at(one) }); + fire(three, 'pointermove', { buttons: 1, ...at(three) }); + out.firstActive = w.__app._tabLayoutDrag?.active === true; + const firstListener = w.__app._tabLayoutDragKeydown; + fire(three, 'pointerdown', { button: 0, buttons: 1, ...at(three) }); + out.staleOrigin = rail.querySelectorAll('.tab-layout-dragging').length; + out.firstListenerKept = w.__app._tabLayoutDragKeydown === firstListener; + fire(one, 'pointermove', { buttons: 1, ...at(one) }); + fire(one, 'pointerup', { button: 0, buttons: 0, ...at(one) }); + out.leftover = w.__app._tabLayoutDragKeydown; + return out; + }); + expect(result.afterButtonsUp).toBeNull(); + expect(result.dragClass).toBe(0); + expect(result.firstActive).toBe(true); + expect(result.staleOrigin).toBe(0); + expect(result.firstListenerKept).toBe(false); + expect(result.leftover).toBeNull(); + await page.locator('#term').focus(); + await page.keyboard.press('Escape'); + expect(await page.evaluate(() => (window as any).__termEscapes)).toBe(1); + await settled(); + }); + + it('committing a group rename by clicking elsewhere leaves focus where the click put it', async () => { + await page.evaluate(() => (window as any).__app.startTabGroupRename('gy')); + const input = page.locator('.tab-layout-group-rename-input'); + await expect.poll(() => input.evaluate((el) => el === document.activeElement)).toBe(true); + await page.keyboard.press('Control+A'); + await page.keyboard.type('Elsewhere'); + await page.locator('#term').click(); + await settled(); + expect(puts.at(-1).layout.groups[1].name).toBe('Elsewhere'); + expect(await page.evaluate(() => document.activeElement?.id)).toBe('term'); + // So Enter goes to the terminal, not to the header (which would collapse it). + await page.keyboard.press('Enter'); + expect(await page.evaluate(() => (window as any).__app.collapsedTabGroupIds.size)).toBe(0); + }); + it('paints the inline group editor as you type, then saves the trimmed name', async () => { await page.evaluate(() => (window as any).__app.startTabGroupRename('gx')); const input = page.locator('.tab-layout-group-rename-input'); diff --git a/test/tab-layout-editing.test.ts b/test/tab-layout-editing.test.ts index b612709e..2e0182db 100644 --- a/test/tab-layout-editing.test.ts +++ b/test/tab-layout-editing.test.ts @@ -136,17 +136,17 @@ describe('drop -> operation', () => { groupId: 'g1', index: 1, }); - expect(h.dropOperation(base(), { type: 'ref', ref: w('web') }, { type: 'group', groupId: 'g1' }, {})).toMatchObject( - { - ref: w('web'), - groupId: 'g1', - index: 2, - } - ); - expect(h.dropOperation(base(), { type: 'ref', ref: s('a') }, { type: 'ungrouped' }, {})).toMatchObject({ - groupId: null, - index: 2, - }); + // Onto a header or Ungrouped means "at the end": no index, so a replay onto + // a layout that gained rows meanwhile still lands it last. + const onHeader = h.dropOperation(base(), { type: 'ref', ref: w('web') }, { type: 'group', groupId: 'g1' }, {}); + expect(onHeader).toMatchObject({ ref: w('web'), groupId: 'g1' }); + expect(onHeader).not.toHaveProperty('index'); + const onUngrouped = h.dropOperation(base(), { type: 'ref', ref: s('a') }, { type: 'ungrouped' }, {}); + expect(onUngrouped).toMatchObject({ groupId: null }); + expect(onUngrouped).not.toHaveProperty('index'); + const crowded = { ...base(), groups: [{ ...base().groups[0], refs: [...base().groups[0].refs, s('late')] }, base().groups[1]] }; + const replayed = h.applyOperation(crowded, onHeader); + expect(replayed.groups[0].refs.at(-1)).toEqual(w('web')); }); it('maps a group dropped on another group (or a row in it) to a reorder', () => { @@ -364,6 +364,26 @@ describe('edit coordinator', () => { expect(onFailure).toHaveBeenCalledTimes(1); }); + it('reports a 400 that survives the re-read as a failed save, not as a race', async () => { + const { put, calls } = controlledPut(); + const fetchLayout = vi.fn(async () => base(11)); + const reportError = vi.fn(); + const onFailure = vi.fn(); + const { editor } = makeEditor(put, { fetchLayout, onFailure, reportError, maxAttempts: 3 }); + editor.enqueue({ type: 'renameGroup', groupId: 'g1', name: 'Mine' }); + await settle(); + calls[0].resolve({ ok: false, status: 400, layout: null }); + await settle(); + await settle(); + calls[1].resolve({ ok: false, status: 400, layout: null }); + await settle(); + expect(calls).toHaveLength(2); + expect(fetchLayout).toHaveBeenCalledTimes(1); + expect(reportError).toHaveBeenCalledWith('Could not save tab groups.'); + expect(reportError).not.toHaveBeenCalledWith('Tab groups kept changing elsewhere; your edit was not saved.'); + expect(onFailure).toHaveBeenCalledTimes(1); + }); + it('refuses an external layout while writing, rebases pending edits onto one otherwise', async () => { const { put, calls } = controlledPut(); const { editor, applied } = makeEditor(put); @@ -523,6 +543,7 @@ beforeEach(() => { win.localStorage.clear(); win.sessionStorage.clear(); document.body.innerHTML = ''; + win.__codemanUser = { username: 'admin', role: 'admin', multiUser: false }; }); afterEach(() => { @@ -542,7 +563,7 @@ describe('row actions in the vertical rail', () => { expect(menuLabels()).toEqual([ 'Session options', 'Move down', - 'Move to Later', + 'Move to "Later"', 'Move to Ungrouped', 'Move to new group', 'Close session', @@ -555,11 +576,34 @@ describe('row actions in the vertical rail', () => { expect(menuLabels()).toEqual(['Session options', 'Close session']); }); + it('quotes group names, so a group called "New group" or "ungrouped" reads apart from the fixed entries', () => { + installFetch(); + const app = makeApp({ + ...serverLayout(), + groups: [ + { id: 'gn', name: 'New group', refs: [s('s2')] }, + { id: 'gu', name: 'ungrouped', refs: [] }, + ], + }); + app.openTabRailActionMenu({ preventDefault() {}, stopPropagation() {}, currentTarget: row('s2') }, 's2'); + expect(menuLabels()).toEqual([ + 'Session options', + 'Move to "ungrouped"', + 'Move to Ungrouped', + 'Move to new group', + 'Close session', + ]); + app.closeTabRailActionMenu(); + app.openTabRailActionMenu({ preventDefault() {}, stopPropagation() {}, currentTarget: row('s1') }, 's1'); + expect(menuLabels()).toContain('Move to "New group"'); + expect(menuLabels()).toContain('Move to new group'); + }); + it('moves a row into another group with one PUT carrying the held version', async () => { const puts = installFetch(); const app = makeApp(); app.openTabRailActionMenu({ preventDefault() {}, stopPropagation() {}, currentTarget: row('s1') }, 's1'); - clickMenu('Move to Later'); + clickMenu('Move to "Later"'); // Optimistic: the rail already shows it there. expect(row('s1').closest('.tab-layout-group')!.getAttribute('data-tab-group-id')).toBe('gy'); await flush(); @@ -621,7 +665,7 @@ describe('web tab rows', () => { expect(menuLabels()).toEqual([ 'Web tab settings', 'Move up', - 'Move to Later', + 'Move to "Later"', 'Move to Ungrouped', 'Move to new group', ]); @@ -671,6 +715,35 @@ describe('group menu', () => { expect(keys(last.ungrouped)).toEqual(['session:s1', 'session:s3', 'session:s2', 'webview:w1']); }); + it('cancelling "Delete group" returns focus to the header, with no write', async () => { + const puts = installFetch(); + const app = makeApp(); + win.confirm = vi.fn(() => false); + header('gy').focus(); + key(header('gy'), 'F10', { shiftKey: true }); + clickMenu('Delete group'); + expect(win.confirm).toHaveBeenCalledTimes(1); + await flush(); + expect(puts).toHaveLength(0); + expect(document.activeElement).toBe(header('gy')); + expect(app.tabLayout.groups).toHaveLength(2); + }); + + it("closing the row menu (as session:deleted does) leaves an open group menu and its listeners alone", () => { + installFetch(); + const app = makeApp(); + header('gx').focus(); + key(header('gx'), 'F10', { shiftKey: true }); + const menu = document.querySelector('.tab-layout-group-action-menu'); + expect(menu).not.toBeNull(); + app.closeTabRailActionMenu(); + expect(menu!.isConnected).toBe(true); + expect(app._tabGroupMenu).toBe(menu); + app.closeTabGroupMenu(); + expect(menu!.isConnected).toBe(false); + expect(app._tabGroupMenuKeydown).toBeNull(); + }); + it('renames from F2 and cancels on Escape without a write', async () => { const puts = installFetch(); const app = makeApp(); @@ -809,6 +882,107 @@ describe('server echoes and reloads', () => { expect(gets).toHaveLength(1); }); + it('a failed read during a write keeps the edit and its editor, and the 409 still rebases', async () => { + const pending: Array<{ body: any; resolve: (r: any) => void }> = []; + const gets: number[] = []; + win.fetch = vi.fn((_url: string, init: any) => { + if (init?.method === 'PUT') { + const body = JSON.parse(init.body); + return new Promise((resolve) => pending.push({ body, resolve })); + } + gets.push(1); + return Promise.resolve({ + ok: true, + status: 200, + json: async () => ({ success: true, data: { layout: app.tabLayout } }), + }); + }); + const app = makeApp(); + app.editTabLayout({ type: 'renameGroup', groupId: 'gy', name: 'Mine' }); + await flush(); + expect(pending).toHaveLength(1); + const editor = app._tabLayoutEditor; + // The layout read failed (the load coordinator's fallback) while the PUT is out. + app._applyTabLayout(null); + expect(app._tabLayoutEditor).toBe(editor); + expect(app.tabLayout.groups[1].name).toBe('Mine'); + expect(tabs().getAttribute('role')).toBe('tree'); + // Someone else wrote first: the 409 is rebased and retried, not lost. + const theirs = { ...serverLayout(9), groups: [...serverLayout().groups, { id: 'gz', name: 'Z', refs: [] }] }; + pending[0].resolve({ ok: false, status: 409, json: async () => ({ success: false, data: { layout: theirs } }) }); + await flush(); + expect(pending).toHaveLength(2); + expect(pending[1].body.baseVersion).toBe(9); + expect(pending[1].body.layout.groups.map((g: any) => g.name)).toEqual(['', 'Mine', 'Z']); + pending[1].resolve({ + ok: true, + status: 200, + json: async () => ({ success: true, data: { layout: { ...pending[1].body.layout, version: 10 } } }), + }); + await flush(); + expect(app.tabLayout.version).toBe(10); + expect(app.showToast).not.toHaveBeenCalled(); + // The read the failure deferred runs once the write settles. + expect(gets).toHaveLength(1); + }); + + it('replays only its own owner\'s recent copy of unsaved edits', async () => { + const copy = (extra: Record) => + win.sessionStorage.setItem( + 'codeman:tab-layout-pending', + JSON.stringify({ + owner: '@single', + baseVersion: 8, + savedAt: Date.now(), + operations: [{ type: 'renameGroup', groupId: 'gy', name: 'Replayed' }], + ...extra, + }) + ); + const replays = async (extra: Record) => { + copy(extra); + const puts = installFetch(); + makeApp(); + await flush(); + expect(win.sessionStorage.getItem('codeman:tab-layout-pending')).toBeNull(); + return puts.length; + }; + expect(await replays({})).toBe(1); + expect(await replays({ owner: 'alice' })).toBe(0); + expect(await replays({ savedAt: Date.now() - 5 * 60_000 })).toBe(0); + expect(await replays({ savedAt: undefined })).toBe(0); + expect(await replays({ baseVersion: 30 })).toBe(0); + + // Multi-user: the copy is keyed by the user name. + win.__codemanUser = { username: 'alice', role: 'user', multiUser: true }; + expect(await replays({ owner: 'alice' })).toBe(1); + expect(await replays({ owner: '@single' })).toBe(0); + + // Identity not known yet: hold the copy until /api/me answers. + win.__codemanUser = undefined; + copy({}); + const late = installFetch(); + makeApp(); + await flush(); + expect(late).toHaveLength(0); + expect(win.sessionStorage.getItem('codeman:tab-layout-pending')).not.toBeNull(); + win.__codemanUser = { username: 'admin', role: 'admin', multiUser: false }; + document.dispatchEvent(new win.CustomEvent('codeman:me')); + await flush(); + expect(late).toHaveLength(1); + expect(late[0].layout.groups[1].name).toBe('Replayed'); + }); + + it('says so when a read has to drop edits it could not rebase', () => { + installFetch(); + const app = makeApp(); + app.editTabLayout({ type: 'renameGroup', groupId: 'gy', name: 'Unsent' }); + // Not yet flushed (no write in flight), and the rebase refuses the read. + app._tabLayoutEditor.adoptExternal = () => false; + app._applyTabLayout(serverLayout(9)); + expect(app.showToast).toHaveBeenCalledWith('Your tab group edit was not saved.', 'error'); + expect(app._tabLayoutEditor).toBeNull(); + }); + it('keeps unsaved edits across a reload: keepalive PUT now, rebased replay after', async () => { const puts = installFetch(); const app = makeApp(); @@ -822,9 +996,11 @@ describe('server echoes and reloads', () => { ); expect(puts.at(-1).baseVersion).toBe(8); expect(puts.at(-1).layout.groups[1].name).toBe('Unsaved'); - expect(JSON.parse(win.sessionStorage.getItem('codeman:tab-layout-pending')).operations).toEqual([ - { type: 'renameGroup', groupId: 'gy', name: 'Unsaved' }, - ]); + const stored = JSON.parse(win.sessionStorage.getItem('codeman:tab-layout-pending')); + expect(stored.operations).toEqual([{ type: 'renameGroup', groupId: 'gy', name: 'Unsaved' }]); + expect(stored.owner).toBe('@single'); + expect(stored.baseVersion).toBe(8); + expect(Math.abs(Date.now() - stored.savedAt)).toBeLessThan(5000); // Next page: the keepalive lost a race; the layout moved on to version 12. const next = installFetch(); @@ -842,7 +1018,12 @@ describe('server echoes and reloads', () => { // And when the keepalive DID land, nothing is re-sent. win.sessionStorage.setItem( 'codeman:tab-layout-pending', - JSON.stringify({ operations: [{ type: 'renameGroup', groupId: 'gy', name: 'Later' }] }) + JSON.stringify({ + owner: '@single', + baseVersion: 8, + savedAt: Date.now(), + operations: [{ type: 'renameGroup', groupId: 'gy', name: 'Later' }], + }) ); const none = installFetch(); makeApp(); @@ -850,6 +1031,12 @@ describe('server echoes and reloads', () => { expect(none).toHaveLength(0); }); + it('keeps the group menu glyph visible where there is no hover (touch tablets)', () => { + const css = read('styles.css'); + const block = css.match(/@media \(hover: none\) \{\s*html\[data-tab-orientation='vertical'\] \.tab-rail \.tab-layout-group-menu \{([^}]*)\}/); + expect(block?.[1]).toMatch(/opacity:\s*1/); + }); + it('leaves the flat rail byte-identical when the layout has no groups, and never edits off the rail', () => { installFetch(); const noLayout = makeApp(null);