From a979916568b0225123711b1aa401d67102e3cf95 Mon Sep 17 00:00:00 2001 From: Season Saw Date: Mon, 27 Jul 2026 21:17:14 +0800 Subject: fix: keep focus on the current workspace when another self-closes (#1912) handle_pane_died only clamped the stale active/selected indices after removing a workspace that closed itself, so it never shifted them down when the removed workspace sat before the focused one. With workspaces [x, y, z] and focus on y, removing x left active pointing at z, silently moving the user into a workspace they did not select. Resolve both indices by workspace id across the removal, as close_selected_workspace already does, keeping the clamp as the fallback for when the removed workspace was the focused one. Also scroll the focused workspace back into view, which the explicit-close path already did and this one did not. refs #1621 Co-authored-by: Can Celik --- src/app/actions.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) (limited to 'src') diff --git a/src/app/actions.rs b/src/app/actions.rs index c23c219c..91572765 100644 --- a/src/app/actions.rs +++ b/src/app/actions.rs @@ -3326,6 +3326,11 @@ impl AppState { self.mark_session_dirty(); if should_close_workspace { + let active_workspace_id = self + .active + .and_then(|idx| self.workspaces.get(idx)) + .map(|ws| ws.id.clone()); + let selected_workspace_id = self.workspaces.get(self.selected).map(|ws| ws.id.clone()); self.workspaces.remove(ws_idx); self.remove_unattached_terminal_ids(workspace_terminal_ids); if self.workspaces.is_empty() { @@ -3335,14 +3340,29 @@ impl AppState { self.mode = Mode::Navigate; } } else { + // Keep focus on the previously focused workspace + if let Some(id) = active_workspace_id { + if let Some(idx) = self.workspaces.iter().position(|ws| ws.id == id) { + self.active = Some(idx); + } + } if let Some(active) = self.active { if active >= self.workspaces.len() { self.active = Some(self.workspaces.len() - 1); } } + if let Some(id) = selected_workspace_id { + if let Some(idx) = self.workspaces.iter().position(|ws| ws.id == id) { + self.selected = idx; + } + } if self.selected >= self.workspaces.len() { self.selected = self.workspaces.len() - 1; } + self.workspace_scroll = self + .workspace_scroll + .min(self.workspaces.len().saturating_sub(1)); + self.ensure_workspace_visible(self.selected); } } else { self.remove_unattached_terminal_ids(pane_terminal_id); @@ -4645,6 +4665,34 @@ mod tests { state.assert_invariants_for_test(); } + #[test] + fn pane_died_self_closing_earlier_workspace_keeps_focus() { + let names = (0..20).map(|i| format!("ws{i:02}")).collect::>(); + let name_refs = names.iter().map(String::as_str).collect::>(); + let mut state = app_with_workspaces(&name_refs); + state.selected = 1; + state.active = Some(1); + // Constrain the sidebar viewport so the focused workspace starts + // off-screen: 20 workspaces cannot all fit in 12 rows, so index 1 is + // hidden at maximum scroll regardless of individual card height. + state.view.sidebar_rect = ratatui::layout::Rect::new(0, 0, 30, 12); + state.workspace_scroll = + crate::ui::normalized_workspace_scroll(&state, state.view.sidebar_rect, usize::MAX / 2); + let cards = crate::ui::compute_workspace_card_areas(&state, state.view.sidebar_rect); + assert!(cards.iter().all(|card| card.ws_idx != 1)); + + let pane_id = *state.workspaces[0].panes.keys().next().unwrap(); + state.handle_pane_died(pane_id); + + assert_eq!(state.workspaces.len(), 19); + assert_eq!(state.workspaces[0].display_name(), "ws01"); + assert_eq!(state.selected, 0); + assert_eq!(state.active, Some(0)); + let cards = crate::ui::compute_workspace_card_areas(&state, state.view.sidebar_rect); + assert!(cards.iter().any(|card| card.ws_idx == 0)); + state.assert_invariants_for_test(); + } + #[test] fn pane_died_last_pane_removes_workspace() { let mut state = app_with_workspaces(&["a", "b"]); -- cgit v1.2.3