Skip to content

feat(desktop): add tab close controls with focus recovery - #83051

Closed
nicolasdmolina wants to merge 11 commits into
NousResearch:mainfrom
nicolasdmolina:feat/desktop-tab-close-button
Closed

feat(desktop): add tab close controls with focus recovery#83051
nicolasdmolina wants to merge 11 commits into
NousResearch:mainfrom
nicolasdmolina:feat/desktop-tab-close-button

Conversation

@nicolasdmolina

@nicolasdmolina nicolasdmolina commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add accessible direct close controls to closeable desktop pane and session tabs while preserving middle-click, modifier-click, and context-menu behavior.
  • Route pointer, keyboard, menu, global, terminal, confirmation, visibility, and bulk-close paths through a shared focus-recovery lifecycle.
  • Keep close targeting aligned with the tab the renderer exposes as selected, including hidden raw successors and layout-edit mode.
  • Preserve focused xterm identity independently from the selected terminal rail tab, and keep focus in the originating split group or terminal rail across pane removal and delayed initialization.
  • Add vertical-tab ARIA relationships, roving terminal-rail focus, and broad unit plus real Electron interaction coverage.

Motivation

Tab closing has several asynchronous entry points. Without a shared completion lifecycle, focus can remain on disappearing controls, jump to the wrong split group, target an invisible raw active pane, or be stolen by a delayed terminal mount. This change makes direct close controls durable and keyboard-accessible across those paths.

Changes

  • Add horizontal pane/session close controls with separate tab and close-button semantics.
  • Serialize aggregate session closes around busy-session confirmation and settle focus only after the close result is known.
  • Preserve source-group identity and choose an actually visible, focusable survivor before application-level fallback.
  • Share the renderer's shown/selected-pane projection with global close routing so normal, narrow, and layout-edit modes cannot disagree.
  • Wrap the complete visual session tab in its context-menu trigger so right-clicking the sibling close glyph opens the session menu rather than a dead zone or the strip menu.
  • Stamp persistent terminal panels with terminal identity, synchronize selection on focus entry, fail closed for stale panel owners, preserve terminal-owned no-op results, and route global close through the focused xterm panel before falling back to the selected rail tab.
  • Suppress automatic xterm focus while the rail or pending tree recovery owns focus, without changing intentional drag/drop focus.
  • Implement terminal rail tablist/tab/tabpanel linkage, selected state, and roving keyboard navigation.

Test Plan

  • Typecheck and lint on final source — 0 errors, 89 existing warnings
  • Bounded full UI suite on the saturated local host — 3,758/3,758 across 418 files; the literal unconstrained aggregate reproduced unrelated timeout/overlapping-act() failures, and every implicated file passed when isolated with adequate timeout
  • Electron-platform suite — 1,035 passed, 2 skipped
  • Production build and macOS DMG packaging passed on the clean committed snapshot, stamped exactly to eb7b0865cb93 with no dirty-tree warning
  • npx playwright test e2e/pane-tab-close.spec.ts e2e/right-pane.spec.ts --reporter=list --timeout=180000 --workers=1 — 6/6 real macOS Electron tests, including close-glyph context menu, focused real-xterm close, survivor rail focus, and a second global close that removes the surviving nested terminal rather than the outer pane
  • git diff --check origin/main...HEAD clean; added-line credential/unsafe-execution/debug scan found no matches
  • Synced with current upstream main; conflict resolution preserves ContribRender, split-share memory, deferred close results, and tab-panel semantics
  • Two fresh independent strict read-only auditors returned SHIP on exact commit eb7b0865cb930bfcc62e25544e420ff7c24ca36f with zero blocking, nonblocking, or security findings
  • Temporary real-Electron race probe sent two global-close accelerators back-to-back from a focused xterm with no intermediate wait; both terminal tabs and persisted terminal state were removed, confirming no escape to the containing pane. The probe and generated evidence were deleted, then the exact committed tree was reattested clean.

Review History

Two strict reviewers of the previous snapshot independently identified three reachable interaction defects:

  1. global close could disagree with the selected tab in layout-edit mode;
  2. right-clicking a session tab's close glyph did not open its session context menu;
  3. a focused xterm could lose close affinity if terminal selection drifted; follow-up review additionally found stale panel owners could fall through to the surviving terminal and the dispatcher discarded an owned false result.

Each issue now has a red-before/green-after regression, including two-gesture and back-to-back real Electron assertions plus separate stale-owner helper/dispatcher contracts. Two fresh hash-bound semantic and interaction/accessibility/security reviews returned SHIP on the exact final snapshot with no findings.

Notes for Reviewers

  • Supersedes feat(desktop): hover X close button on zone tabs #69392, whose branch conflicts with current main.
  • Retains Austin Pickett attribution for the original hover-close feature while adding current-main accessibility, lifecycle, focus-recovery, terminal, and regression work.
  • The close control renders only for closeable horizontal tabs; minimized vertical rails use explicit focus restoration instead.

nicolasdmolina and others added 6 commits August 10, 2026 04:14
Add a hover-to-close X button to PaneTab (the fancy-zones tab shell).
The button slot is always reserved inline (shrink-0) so the tab width
stays stable whether the X is visible or not — no layout shift on hover.
Visible on group-hover/tab, hidden by default via opacity transition.

The dirty dot yields to the X when both are present (closeable + dirty):
the X wins on hover, the dot shows otherwise. Vertical tabs skip the X
(writing-mode:vertical-rl makes an inline button awkward) and keep the
absolute-positioned dirty dot.

Pointerdown on the X is stopped so the tab's drag/activate handlers
never fire — the X is a leaf close action, not a drag start.
@alt-glitch alt-glitch added type/feature New feature or request comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have labels Aug 10, 2026
Route pane, session, and terminal close operations through a shared focus-recovery lifecycle.

Keep terminal rail focus stable across global close and delayed terminal initialization, and add interaction regressions for split, minimized, hidden-successor, and confirmation paths.
@nicolasdmolina nicolasdmolina changed the title feat(desktop): add close controls to session tabs feat(desktop): add tab close controls with focus recovery Aug 11, 2026
@nicolasdmolina

Copy link
Copy Markdown
Contributor Author

Correction: PR mergeability is CONFLICTING and mergeStateStatus is DIRTY. Latest branch workflows are action_required with zero jobs, so automation hasn’t produced substantive check output yet. Please resolve merge conflicts locally before CI can run to completion.

@nicolasdmolina

Copy link
Copy Markdown
Contributor Author

Closing this broad replacement PR. The primary visible hover-close affordance has since landed in #89428 and was refined in #91777. This branch is now merge-conflicted, and its 37-file expansion combines several independent focus, pane, and terminal lifecycle changes that should not be reviewed as one stale PR. Any remaining behavior that still reproduces on current main should be filed and fixed as a small issue-linked change. The branch and history remain available, including Austin Pickett’s original commits and attribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants