From 3c8b573a785c20e1167bb0c21a3fa161e3746e25 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:17:39 +0800 Subject: [PATCH] fix(ui): un-zooming a pane keeps focus in that pane (#869) Un-zoom routed focus through the tab's `last_focused`, which only focus-in writes, so any gap between that record and the pane actually zoomed put the cursor in a different pane when the split came back. Since #843 the record is kept current on focus-in, which is why the reported sequence no longer reproduces on main, but the zoomed pane is the answer outright: hand it to the tab before focusing, so un-zoom no longer depends on the record having caught up. Zooming with focus off the panes (a palette just closed, the tab strip) also zoomed the tab's first leaf rather than the pane the tab remembers; it now falls back to `focus_target`, the same pane a switch back to the tab would focus. --- src/ui/app.rs | 127 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 124 insertions(+), 3 deletions(-) diff --git a/src/ui/app.rs b/src/ui/app.rs index fcb5134a..da1f5bb2 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -4687,9 +4687,19 @@ impl Tty7App { }) } + /// Zoom the pane being worked in over its siblings, or put the layout back. + /// + /// Both directions name the pane outright rather than leaving it to + /// whatever holds focus at the instant the toggle lands (#869). Un-zooming + /// hands the tab the pane that was zoomed as its answer before focusing it: + /// that pane was the only one on screen to work in, and the tab's memory is + /// written by focus-in alone, which nothing promises has caught up. Zooming + /// with focus off the panes — a palette just closed, the tab strip — takes + /// the pane the tab remembers, the one switching back to it would focus, + /// and not whichever leaf the layout happens to list first. fn toggle_maximize(&mut self, window: &mut Window, cx: &mut Context) { - if self.maximized.is_some() { - self.maximized = None; + if let Some(leaf) = self.maximized.take() { + remember_leaf_in(&mut self.tabs, leaf.entity_id()); self.focus_active(window, cx); cx.notify(); return; @@ -4700,7 +4710,11 @@ impl Tty7App { if tab.pane.leaves().len() < 2 { return; } - let leaf = tab.pane.focused_or_first(window, cx); + let leaf = tab + .pane + .focused_leaf(window, cx) + .or_else(|| tab.focus_target()) + .and_then(|slot| slot.terminal().cloned()); if let Some(leaf) = leaf { let handle = leaf.read(cx).focus_handle.clone(); self.maximized = Some(leaf); @@ -12133,4 +12147,111 @@ mod tab_focus_memory_tests { ); }); } + + /// A split tab on screen with both panes watched the way every spawn path + /// watches them, plus a handle off the panes for focus to wander to. + #[allow(clippy::type_complexity)] + fn watched_split( + cx: &mut TestAppContext, + ) -> ( + Entity, + gpui::VisualTestContext, + Entity, + Entity, + gpui::FocusHandle, + impl Sized, + ) { + use super::{test_window::harness, watch_pane_focus}; + use crate::terminal::view::quiet_test_pane; + + let (app, mut vcx) = harness(cx); + let (left, right, elsewhere, held) = app.update_in(&mut vcx, |app, window, cx| { + let (left, left_stream) = quiet_test_pane(1, window, cx); + let (right, right_stream) = quiet_test_pane(2, window, cx); + app.tabs.push(Tab::new(Pane::split_node( + Axis::Horizontal, + 0.5, + Pane::leaf(PaneSlot::Ready(left.clone())), + Pane::leaf(PaneSlot::Ready(right.clone())), + ))); + app.active = 0; + for view in [&left, &right] { + let handle = view.read(cx).focus_handle.clone(); + watch_pane_focus(&handle, view.entity_id(), window, cx); + } + cx.notify(); + (left, right, cx.focus_handle(), (left_stream, right_stream)) + }); + vcx.background_executor.run_until_parked(); + (app, vcx, left, right, elsewhere, held) + } + + /// Zoom in on a pane, zoom back out, and the cursor is still in that pane + /// (#869). The pane being un-zoomed is the answer outright — it is the one + /// that was on screen to work in — so it is not a question for the tab's + /// memory, which only focus-in writes and which nothing guarantees is + /// current at the instant the toggle lands. + #[gpui::test] + fn unzooming_leaves_focus_in_the_pane_that_was_zoomed(cx: &mut TestAppContext) { + let (app, mut vcx, left, right, _elsewhere, _held) = watched_split(cx); + + // Into the right-hand pane by keyboard, the road the report took. + app.update_in(&mut vcx, |app, window, cx| { + left.read(cx).focus_handle.clone().focus(window, cx); + app.cycle_pane(true, window, cx); + }); + vcx.background_executor.run_until_parked(); + + app.update_in(&mut vcx, |app, window, cx| { + app.toggle_maximize(window, cx); + assert_eq!( + app.maximized.as_ref().map(|l| l.entity_id()), + Some(right.entity_id()), + "the pane focus is in is the one zoomed" + ); + // Whatever the tab's memory says while zoomed — here the stale + // answer the report found — must not decide where un-zooming goes. + app.tabs[0].last_focused = Some(left.entity_id()); + app.toggle_maximize(window, cx); + }); + vcx.background_executor.run_until_parked(); + + app.update_in(&mut vcx, |app, window, cx| { + assert!(app.maximized.is_none()); + assert!( + right.read(cx).focus_handle.is_focused(window), + "#869: un-zooming keeps focus in the pane that was zoomed" + ); + assert_eq!( + app.tabs[0].last_focused, + Some(right.entity_id()), + "and the tab remembers it, so a switch away and back agrees" + ); + }); + } + + /// Zooming with focus off the panes — a palette just closed, the tab strip + /// clicked — zooms the pane the tab remembers, not whichever leaf happens + /// to be first: the same pane switching back to the tab would focus. + #[gpui::test] + fn zooming_from_off_the_panes_zooms_the_remembered_pane(cx: &mut TestAppContext) { + let (app, mut vcx, _left, right, elsewhere, _held) = watched_split(cx); + + app.update_in(&mut vcx, |_, window, cx| { + right.read(cx).focus_handle.clone().focus(window, cx); + }); + vcx.background_executor.run_until_parked(); + app.update_in(&mut vcx, |_, window, cx| elsewhere.focus(window, cx)); + vcx.background_executor.run_until_parked(); + + app.update_in(&mut vcx, |app, window, cx| { + app.toggle_maximize(window, cx); + assert_eq!( + app.maximized.as_ref().map(|l| l.entity_id()), + Some(right.entity_id()), + "the zoom lands on the pane the reader was last in" + ); + assert!(right.read(cx).focus_handle.is_focused(window)); + }); + } }