Fix agent panel closing unexpectedly when zoomed (#49037)

Richard Feldman created 2 months ago

Closes https://github.com/zed-industries/zed/issues/33430

Two changes:

1. Fix `observe_global_in` to not permanently remove observers when the
window is transiently unavailable (e.g. temporarily taken during a
nested update). Previously this returned false and silently removed the
observer from the subscriber set. Now it checks whether the entity is
actually dropped before removing — if the entity is alive but the window
is just unavailable, it keeps the observer alive.

2. Extend `dock_to_preserve` in `handle_pane_focused` to also preserve
docks whose active panel has focus, not just docks whose panel's inner
pane matches the focused pane. Panels like `AgentPanel` don't implement
`pane()` (only panels like `TerminalPanel` that contain panes do), so
the existing preservation logic never identified the agent panel's dock
as needing protection. This meant that when the agent panel was zoomed
and a center pane received focus (e.g. during macOS window activation
events), `dismiss_zoomed_items_to_reveal` would close the dock, making
the panel disappear unexpectedly.

Closes AI-16

Release Notes:

- Fixed agent panel unexpectedly closing when zoomed and the window
regains focus.

Change summary

crates/gpui/src/app/context.rs    | 18 +++++--
crates/workspace/src/workspace.rs | 79 ++++++++++++++++++++++++++++++--
2 files changed, 85 insertions(+), 12 deletions(-)

Detailed changes

crates/gpui/src/app/context.rs

@@ -697,11 +697,19 @@ impl<'a, T: 'static> Context<'a, T> {
         let (subscription, activate) = self.global_observers.insert(
             TypeId::of::<G>(),
             Box::new(move |cx| {
-                window_handle
-                    .update(cx, |_, window, cx| {
-                        view.update(cx, |view, cx| f(view, window, cx)).is_ok()
-                    })
-                    .unwrap_or(false)
+                // If the entity has been dropped, remove this observer.
+                if view.upgrade().is_none() {
+                    return false;
+                }
+                // If the window is unavailable (e.g. temporarily taken during a
+                // nested update, or already closed), skip this notification but
+                // keep the observer alive so it can fire on future changes.
+                let Ok(entity_alive) = window_handle.update(cx, |_, window, cx| {
+                    view.update(cx, |view, cx| f(view, window, cx)).is_ok()
+                }) else {
+                    return true;
+                };
+                entity_alive
             }),
         );
         self.defer(move |_| activate());

crates/workspace/src/workspace.rs

@@ -4540,16 +4540,18 @@ impl Workspace {
 
         // If this pane is in a dock, preserve that dock when dismissing zoomed items.
         // This prevents the dock from closing when focus events fire during window activation.
+        // We also preserve any dock whose active panel itself has focus — this covers
+        // panels like AgentPanel that don't implement `pane()` but can still be zoomed.
         let dock_to_preserve = self.all_docks().iter().find_map(|dock| {
             let dock_read = dock.read(cx);
-            if let Some(panel) = dock_read.active_panel()
-                && let Some(dock_pane) = panel.pane(cx)
-                && dock_pane == pane
-            {
-                Some(dock_read.position())
-            } else {
-                None
+            if let Some(panel) = dock_read.active_panel() {
+                if panel.pane(cx).is_some_and(|dock_pane| dock_pane == pane)
+                    || panel.panel_focus_handle(cx).contains_focused(window, cx)
+                {
+                    return Some(dock_read.position());
+                }
             }
+            None
         });
 
         self.dismiss_zoomed_items_to_reveal(dock_to_preserve, window, cx);
@@ -12845,4 +12847,67 @@ mod tests {
         });
         item
     }
+
+    #[gpui::test]
+    async fn test_zoomed_panel_without_pane_preserved_on_center_focus(
+        cx: &mut gpui::TestAppContext,
+    ) {
+        init_test(cx);
+        let fs = FakeFs::new(cx.executor());
+
+        let project = Project::test(fs, [], cx).await;
+        let (workspace, cx) =
+            cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx));
+
+        let panel = workspace.update_in(cx, |workspace, window, cx| {
+            let panel = cx.new(|cx| TestPanel::new(DockPosition::Right, 100, cx));
+            workspace.add_panel(panel.clone(), window, cx);
+            workspace
+                .right_dock()
+                .update(cx, |dock, cx| dock.set_open(true, window, cx));
+            panel
+        });
+
+        let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone());
+        pane.update_in(cx, |pane, window, cx| {
+            let item = cx.new(TestItem::new);
+            pane.add_item(Box::new(item), true, true, None, window, cx);
+        });
+
+        // Transfer focus to the panel, then zoom it. Using toggle_panel_focus
+        // mirrors the real-world flow and avoids side effects from directly
+        // focusing the panel while the center pane is active.
+        workspace.update_in(cx, |workspace, window, cx| {
+            workspace.toggle_panel_focus::<TestPanel>(window, cx);
+        });
+
+        panel.update_in(cx, |panel, window, cx| {
+            panel.set_zoomed(true, window, cx);
+        });
+
+        workspace.update_in(cx, |workspace, window, cx| {
+            assert!(workspace.right_dock().read(cx).is_open());
+            assert!(panel.is_zoomed(window, cx));
+            assert!(panel.read(cx).focus_handle(cx).contains_focused(window, cx));
+        });
+
+        // Simulate a spurious pane::Event::Focus on the center pane while the
+        // panel still has focus. This mirrors what happens during macOS window
+        // activation: the center pane fires a focus event even though actual
+        // focus remains on the dock panel.
+        pane.update_in(cx, |_, _, cx| {
+            cx.emit(pane::Event::Focus);
+        });
+
+        // The dock must remain open because the panel had focus at the time the
+        // event was processed. Before the fix, dock_to_preserve was None for
+        // panels that don't implement pane(), causing the dock to close.
+        workspace.update_in(cx, |workspace, window, cx| {
+            assert!(
+                workspace.right_dock().read(cx).is_open(),
+                "Dock should stay open when its zoomed panel (without pane()) still has focus"
+            );
+            assert!(panel.is_zoomed(window, cx));
+        });
+    }
 }