From 1582e725b88433b14fd739d63d790de3390ab118 Mon Sep 17 00:00:00 2001 From: Umputun Date: Sun, 9 Aug 2026 12:32:30 -0500 Subject: [PATCH 1/5] fix: animate the workspace row-click expand/collapse A workspace row carries two hit targets for one toggle. The disclosure triangle goes through AppKit's native path and animates; the row-click path called expandItem/collapseItem directly, which do not animate, so the same action on the same row rendered two different ways. Route that toggle through outline.animator(). Checked against AppKit that the proxy animates only the row insert/remove while flipping expansion and firing didExpand/didCollapse synchronously, so the suppressExpansionPersist protocol and the persist write-back are unaffected; testWorkspaceCollapsePersistsAcrossRelaunch and testActiveWorkspaceCollapsePersistsDespiteReveal both drive the collapse from a row click and pin that. Not gated on Reduce Motion, unlike the app's own pulses: both hit targets reach the same AppKit animation, so a gate here would re-split them. Scope is the row-click path. The other call sites stay unanimated, being either bulk (expand/collapse all, rebuild re-apply) or programmatic (selection reveal, drag spring-load). The delay half of the report is a separate behavior decision and is untouched. Related to #407 --- .claude/rules/sidebar.md | 5 ++++- agterm/Views/WorkspaceSidebar+ContextMenu.swift | 8 +++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/.claude/rules/sidebar.md b/.claude/rules/sidebar.md index 40154b2d..ca7dad1e 100644 --- a/.claude/rules/sidebar.md +++ b/.claude/rules/sidebar.md @@ -43,7 +43,10 @@ paths: toggle. This click routing is keep-in-sync exempt. `GhosttyApp.workspaceRowClickExpands` (default on) gates the whole-row target only; the disclosure triangle toggles natively and stays unconditional. The deferred item re-reads that mirror when it fires, so turning the setting off inside the deferral - window cancels the pending toggle. Control expansion instead persists through + window cancels the pending toggle. Route the row-click toggle through `outline.animator()` so it animates + like the triangle; a bare `expandItem`/`collapseItem` silently drops the animation. Do not gate that on + Reduce Motion — both hit targets reach the same AppKit animation, so a gate would re-split them. Every + other expand/collapse site stays unanimated: bulk or programmatic. Control expansion instead persists through `AppActions.setWorkspaceExpanded`, then posts `.agtermSetWorkspaceExpanded` to synchronize a live outline. - A session-row click selects, then asynchronously calls `revealActiveBlockedPane` with the captured pre-reset indicator. Blocked/completed pane tags reveal split, scratch, or primary; idle and active use diff --git a/agterm/Views/WorkspaceSidebar+ContextMenu.swift b/agterm/Views/WorkspaceSidebar+ContextMenu.swift index e21be9b4..84d2d010 100644 --- a/agterm/Views/WorkspaceSidebar+ContextMenu.swift +++ b/agterm/Views/WorkspaceSidebar+ContextMenu.swift @@ -47,8 +47,14 @@ extension WorkspaceSidebar.Coordinator { renameController.beginEditing(node: node) } + /// Animated to match the disclosure triangle, which toggles the same row through AppKit's own animation — + /// two hit targets for one action must not render differently. Deliberately NOT gated on Reduce Motion + /// like the app's own pulses: both paths reach the same AppKit animation, so gating here would restore + /// the divergence. The proxy still flips expansion and fires didExpand/didCollapse synchronously, so the + /// persist write-back is unaffected. Every other call site stays unanimated — bulk or programmatic. private func toggleExpansion(of node: SidebarNode, in outline: NSOutlineView) { - if outline.isItemExpanded(node) { outline.collapseItem(node) } else { outline.expandItem(node) } + let proxy = outline.animator() + if outline.isItemExpanded(node) { proxy.collapseItem(node) } else { proxy.expandItem(node) } } /// Builds the per-row context menu, resolving the clicked row lazily so one menu serves every row. From b9b532f98aa6a823631e087ea663f8c680400862 Mon Sep 17 00:00:00 2001 From: Umputun Date: Sun, 9 Aug 2026 12:32:40 -0500 Subject: [PATCH 2/5] docs(backlog): record the row-toggle deferral dropping and misfiring clicks Surfaced while fixing the animation half of #407. The single pendingRowToggle slot and the guard ordering in handleSingleClick mean a second workspace-row click cancels the first with no feedback, and a session-row click returns before the cancel so a pending workspace toggle still fires under it. Deferred because settling them is the same behavior decision as the report's delay half, not a rendering fix. --- ...kspace-row-toggle-deferral-drops-clicks.md | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 docs/backlog/workspace-row-toggle-deferral-drops-clicks.md diff --git a/docs/backlog/workspace-row-toggle-deferral-drops-clicks.md b/docs/backlog/workspace-row-toggle-deferral-drops-clicks.md new file mode 100644 index 00000000..1590081a --- /dev/null +++ b/docs/backlog/workspace-row-toggle-deferral-drops-clicks.md @@ -0,0 +1,33 @@ +--- +worth: later +where: agterm/Views/WorkspaceSidebar+ContextMenu.swift:15 +added: 2026-08-09 +--- +# the deferred workspace-row toggle drops one click and fires another under the wrong row + +`handleSingleClick` parks the toggle in a single `pendingRowToggle` slot for `NSEvent.doubleClickInterval` +so a rename double-click can cancel it. Two consequences fall out of the guard ordering, both reachable +with ordinary clicking: + +- **A click on a second workspace row eats the first.** The `pendingRowToggle?.cancel()` at line 28 is + unconditional and the slot holds one item, so clicking workspace A's row and then workspace B's inside + the interval cancels A and toggles only B. A's click is gone with nothing on screen to say so. +- **A session-row click does not disarm a pending workspace toggle.** The guard at lines 17-18 returns for + `node.kind == .session` before reaching that cancel, so clicking workspace A and then a session in + another workspace selects the session immediately and then, a beat later, expands A underneath it and + shifts every row below. + +Both are worse than they look because a workspace row is not selectable +(`WorkspaceSidebar+RowRendering.swift:14-17` returns `shouldSelectItem` only for sessions), so the whole +deferral window carries no feedback at all - no pill, no press state, no disclosure movement. The click +reads as dropped either way. + +Not fixed alongside #407's animation because the fix is a behavior decision, not a rendering one. #407 +itself names the two candidates for the delay - toggle optimistically and reverse on a double-click, or +move workspace rename off double-click - and either would settle these two as a side effect, while +patching them in isolation (per-node pending toggles, or hoisting the cancel above the guards) preserves +a half-second of silent latency that is itself the complaint. + +Worth weighing that workspace rename already has five other entry points - the row context menu, the +Workspace menu, the `rename_workspace` built-in, the command palette, and `workspace.rename` - so the +deferral protects a sixth path to something already well covered. From 1fad6ab782f4146397d7006f4904e9997ae1149f Mon Sep 17 00:00:00 2001 From: Umputun Date: Sun, 9 Aug 2026 12:43:50 -0500 Subject: [PATCH 3/5] docs: let sidebar.md own the row-toggle animation contract The doc comment on toggleExpansion restated three facts the same change wrote into .claude/rules/sidebar.md, which already owns the click-routing contract. Keep only what is code-local - that the animator proxy fires didExpand/didCollapse synchronously, so the persist write-back and the suppressExpansionPersist bracketing around the other call sites still hold - and cross-reference the rest. Also correct the backlog item: workspace rename sits in the File menu's Workspace section, not a Workspace menu (there is none; the only CommandMenu is Navigate), and shouldSelectItem is at WorkspaceSidebar+RowRendering.swift:15-18. --- agterm/Views/WorkspaceSidebar+ContextMenu.swift | 9 ++++----- .../workspace-row-toggle-deferral-drops-clicks.md | 8 ++++---- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/agterm/Views/WorkspaceSidebar+ContextMenu.swift b/agterm/Views/WorkspaceSidebar+ContextMenu.swift index 84d2d010..8bce4219 100644 --- a/agterm/Views/WorkspaceSidebar+ContextMenu.swift +++ b/agterm/Views/WorkspaceSidebar+ContextMenu.swift @@ -47,11 +47,10 @@ extension WorkspaceSidebar.Coordinator { renameController.beginEditing(node: node) } - /// Animated to match the disclosure triangle, which toggles the same row through AppKit's own animation — - /// two hit targets for one action must not render differently. Deliberately NOT gated on Reduce Motion - /// like the app's own pulses: both paths reach the same AppKit animation, so gating here would restore - /// the divergence. The proxy still flips expansion and fires didExpand/didCollapse synchronously, so the - /// persist write-back is unaffected. Every other call site stays unanimated — bulk or programmatic. + /// Animated to match the disclosure triangle; `.claude/rules/sidebar.md` owns why, why it is not gated on + /// Reduce Motion, and which sites stay unanimated. The proxy flips expansion and fires + /// didExpand/didCollapse synchronously, so the persist write-back is unaffected and the + /// `suppressExpansionPersist` bracketing the other sites wrap their calls in still holds. private func toggleExpansion(of node: SidebarNode, in outline: NSOutlineView) { let proxy = outline.animator() if outline.isItemExpanded(node) { proxy.collapseItem(node) } else { proxy.expandItem(node) } diff --git a/docs/backlog/workspace-row-toggle-deferral-drops-clicks.md b/docs/backlog/workspace-row-toggle-deferral-drops-clicks.md index 1590081a..eb373ac3 100644 --- a/docs/backlog/workspace-row-toggle-deferral-drops-clicks.md +++ b/docs/backlog/workspace-row-toggle-deferral-drops-clicks.md @@ -18,7 +18,7 @@ with ordinary clicking: shifts every row below. Both are worse than they look because a workspace row is not selectable -(`WorkspaceSidebar+RowRendering.swift:14-17` returns `shouldSelectItem` only for sessions), so the whole +(`WorkspaceSidebar+RowRendering.swift:15-18` returns `shouldSelectItem` only for sessions), so the whole deferral window carries no feedback at all - no pill, no press state, no disclosure movement. The click reads as dropped either way. @@ -28,6 +28,6 @@ move workspace rename off double-click - and either would settle these two as a patching them in isolation (per-node pending toggles, or hoisting the cancel above the guards) preserves a half-second of silent latency that is itself the complaint. -Worth weighing that workspace rename already has five other entry points - the row context menu, the -Workspace menu, the `rename_workspace` built-in, the command palette, and `workspace.rename` - so the -deferral protects a sixth path to something already well covered. +Worth weighing that workspace rename already has five other entry points - the row context menu, the File +menu's Workspace section, the `rename_workspace` built-in, the command palette, and `workspace.rename` - +so the deferral protects a sixth path to something already well covered. From e8fa3a151b6bb108933c89c5c45f40f649a0c901 Mon Sep 17 00:00:00 2001 From: Umputun Date: Sun, 9 Aug 2026 13:01:51 -0500 Subject: [PATCH 4/5] docs(backlog): record the three rejected approaches to the row-toggle delay Keeps the next reader from re-proposing any of them: dropping double-click rename, the optimistic toggle with an inverted undo, and press feedback during the deferral were each weighed and judged either a worse trade or disproportionate to the defect. --- docs/backlog/workspace-row-toggle-deferral-drops-clicks.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/backlog/workspace-row-toggle-deferral-drops-clicks.md b/docs/backlog/workspace-row-toggle-deferral-drops-clicks.md index eb373ac3..2e3ca6e7 100644 --- a/docs/backlog/workspace-row-toggle-deferral-drops-clicks.md +++ b/docs/backlog/workspace-row-toggle-deferral-drops-clicks.md @@ -22,6 +22,13 @@ Both are worse than they look because a workspace row is not selectable deferral window carries no feedback at all - no pill, no press state, no disclosure movement. The click reads as dropped either way. +Three approaches were weighed and none taken: drop double-click rename on workspace rows (rejected - +double-click rename is the discoverable path and the macOS convention, whatever the other entry points); +toggle optimistically and invert on double-click (rejected - trades a dead pause for a visible flicker on +rename); and keep the deferral but add press feedback (rejected - a custom-drawn disclosure triangle or a +transient row pill is too much machinery for the size of the defect). Revisit only if the delay starts +costing more than it looks like it does. + Not fixed alongside #407's animation because the fix is a behavior decision, not a rendering one. #407 itself names the two candidates for the delay - toggle optimistically and reverse on a double-click, or move workspace rename off double-click - and either would settle these two as a side effect, while From 58e0aed5c4820d02a9ad9cae90f89adb6b0ab341 Mon Sep 17 00:00:00 2001 From: Umputun Date: Sun, 9 Aug 2026 13:13:20 -0500 Subject: [PATCH 5/5] docs: untangle the toggleExpansion comment's second sentence "the suppressExpansionPersist bracketing the other sites wrap their calls in" stranded a preposition off a gerund and read as noise. Same three facts, one fewer line. --- agterm/Views/WorkspaceSidebar+ContextMenu.swift | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/agterm/Views/WorkspaceSidebar+ContextMenu.swift b/agterm/Views/WorkspaceSidebar+ContextMenu.swift index 8bce4219..527284f9 100644 --- a/agterm/Views/WorkspaceSidebar+ContextMenu.swift +++ b/agterm/Views/WorkspaceSidebar+ContextMenu.swift @@ -48,9 +48,8 @@ extension WorkspaceSidebar.Coordinator { } /// Animated to match the disclosure triangle; `.claude/rules/sidebar.md` owns why, why it is not gated on - /// Reduce Motion, and which sites stay unanimated. The proxy flips expansion and fires - /// didExpand/didCollapse synchronously, so the persist write-back is unaffected and the - /// `suppressExpansionPersist` bracketing the other sites wrap their calls in still holds. + /// Reduce Motion, and which sites stay unanimated. The proxy fires didExpand/didCollapse synchronously, so + /// the persist write-back still runs and the other sites' `suppressExpansionPersist` brackets still hold. private func toggleExpansion(of node: SidebarNode, in outline: NSOutlineView) { let proxy = outline.animator() if outline.isItemExpanded(node) { proxy.collapseItem(node) } else { proxy.expandItem(node) }