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..527284f9 100644 --- a/agterm/Views/WorkspaceSidebar+ContextMenu.swift +++ b/agterm/Views/WorkspaceSidebar+ContextMenu.swift @@ -47,8 +47,12 @@ extension WorkspaceSidebar.Coordinator { renameController.beginEditing(node: node) } + /// 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 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) { - 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. 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..2e3ca6e7 --- /dev/null +++ b/docs/backlog/workspace-row-toggle-deferral-drops-clicks.md @@ -0,0 +1,40 @@ +--- +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: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. + +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 +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 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.