Maintain focus correctly when activating panes in zed2 (#3582)

Previously, before emitting a `Focus` event from the pane inside of the
`focus_in` listener, we would erroneously check whether the pane's focus
handle was _not_ focused. However, by the time the pane was notified of
being "focused in", the focus handle would already be focused, which was
preventing the pane from ever emitting a `Focus` event. In turn, this
would cause the workspace to not maintain the active pane correctly.

This pull request maintains an explicit `was_focused` boolean as part of
the `Pane` state, which ensures we only emit the `Focus` event the first
time the pane receives focus.

As part of this, I also reworked how the outline view gets deployed to
allow clicking breadcrumbs even when the corresponding pane doesn't have
focus.

Release Notes:

- N/A

crates/breadcrumbs2/src/breadcrumbs.rs

@@ -1,13 +1,14 @@
+use editor::Editor;
 use gpui::{
     Div, Element, EventEmitter, IntoElement, ParentElement, Render, StyledText, Subscription,
-    ViewContext, WeakView,
+    ViewContext,
 };
 use itertools::Itertools;
 use theme::ActiveTheme;
 use ui::{prelude::*, ButtonLike, ButtonStyle, Label, Tooltip};
 use workspace::{
     item::{ItemEvent, ItemHandle},
-    ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView, Workspace,
+    ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView,
 };
 
 pub enum Event {
@@ -18,16 +19,14 @@ pub struct Breadcrumbs {
     pane_focused: bool,
     active_item: Option<Box<dyn ItemHandle>>,
     subscription: Option<Subscription>,
-    workspace: WeakView<Workspace>,
 }
 
 impl Breadcrumbs {
-    pub fn new(workspace: &Workspace) -> Self {
+    pub fn new() -> Self {
         Self {
             pane_focused: false,
             active_item: Default::default(),
             subscription: Default::default(),
-            workspace: workspace.weak_handle(),
         }
     }
 }
@@ -62,31 +61,19 @@ impl Render for Breadcrumbs {
             Label::new("›").into_any_element()
         });
 
+        let editor = active_item
+            .downcast::<Editor>()
+            .map(|editor| editor.downgrade());
+
         element.child(
             ButtonLike::new("toggle outline view")
                 .style(ButtonStyle::Subtle)
                 .child(h_stack().gap_1().children(breadcrumbs))
-                // We disable the button when the containing pane is not focused:
+                .on_click(move |_, cx| {
-                //    Because right now all the breadcrumb does is open the outline view, which is an
+                    if let Some(editor) = editor.as_ref().and_then(|editor| editor.upgrade()) {
-                //    action which operates on the active editor, clicking the breadcrumbs of another
+                        outline::toggle(editor, &outline::Toggle, cx)
-                //    editor could cause weirdness. I remember that at one point it actually caused a
-                //    panic weirdly.
-                //
-                //    It might be possible that with changes around how focus is managed that we
-                //    might be able to update the active editor to the one with the breadcrumbs
-                //    clicked on? That or we could just add a code path for being able to open the
-                //    outline for a specific editor. Long term we'd like for it to be an actual
-                //    breadcrumb bar so that problem goes away
-                //
-                //   — Julia (https://github.com/zed-industries/zed/pull/3505#pullrequestreview-1766198050)
-                .disabled(!self.pane_focused)
-                .on_click(cx.listener(|breadcrumbs, _, cx| {
-                    if let Some(workspace) = breadcrumbs.workspace.upgrade() {
-                        workspace.update(cx, |workspace, cx| {
-                            outline::toggle(workspace, &outline::Toggle, cx)
-                        })
                     }
-                }))
+                })
                 .tooltip(|cx| Tooltip::for_action("Show symbol outline", &outline::Toggle, cx)),
         )
     }

crates/outline2/src/outline.rs

@@ -1,6 +1,6 @@
 use editor::{
     display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, Anchor, AnchorRangeExt,
-    DisplayPoint, Editor, ToPoint,
+    DisplayPoint, Editor, EditorMode, ToPoint,
 };
 use fuzzy::StringMatch;
 use gpui::{
@@ -20,7 +20,7 @@ use std::{
 use theme::{color_alpha, ActiveTheme, ThemeSettings};
 use ui::{prelude::*, ListItem};
 use util::ResultExt;
-use workspace::{ModalView, Workspace};
+use workspace::ModalView;
 
 actions!(outline, [Toggle]);
 
@@ -28,21 +28,18 @@ pub fn init(cx: &mut AppContext) {
     cx.observe_new_views(OutlineView::register).detach();
 }
 
-pub fn toggle(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContext<Workspace>) {
+pub fn toggle(editor: View<Editor>, _: &Toggle, cx: &mut WindowContext) {
-    if let Some(editor) = workspace
+    let outline = editor
-        .active_item(cx)
+        .read(cx)
-        .and_then(|item| item.downcast::<Editor>())
+        .buffer()
-    {
+        .read(cx)
-        let outline = editor
+        .snapshot(cx)
-            .read(cx)
+        .outline(Some(&cx.theme().syntax()));
-            .buffer()
-            .read(cx)
-            .snapshot(cx)
-            .outline(Some(&cx.theme().syntax()));
 
-        if let Some(outline) = outline {
+    if let Some((workspace, outline)) = editor.read(cx).workspace().zip(outline) {
+        workspace.update(cx, |workspace, cx| {
             workspace.toggle_modal(cx, |cx| OutlineView::new(outline, editor, cx));
-        }
+        })
     }
 }
 
@@ -68,8 +65,15 @@ impl Render for OutlineView {
 }
 
 impl OutlineView {
-    fn register(workspace: &mut Workspace, _: &mut ViewContext<Workspace>) {
+    fn register(editor: &mut Editor, cx: &mut ViewContext<Editor>) {
-        workspace.register_action(toggle);
+        if editor.mode() == EditorMode::Full {
+            let handle = cx.view().downgrade();
+            editor.register_action(move |action, cx| {
+                if let Some(editor) = handle.upgrade() {
+                    toggle(editor, action, cx);
+                }
+            });
+        }
     }
 
     fn new(
@@ -239,6 +243,7 @@ impl PickerDelegate for OutlineViewDelegate {
                     s.select_ranges([position..position])
                 });
                 active_editor.highlight_rows(None);
+                active_editor.focus(cx);
             }
         });
 

crates/workspace2/src/pane.rs

@@ -159,6 +159,7 @@ pub struct Pane {
     items: Vec<Box<dyn ItemHandle>>,
     activation_history: Vec<EntityId>,
     zoomed: bool,
+    was_focused: bool,
     active_item_index: usize,
     last_focused_view_by_item: HashMap<EntityId, FocusHandle>,
     autoscroll: bool,
@@ -317,6 +318,7 @@ impl Pane {
             focus_handle: cx.focus_handle(),
             items: Vec::new(),
             activation_history: Vec::new(),
+            was_focused: false,
             zoomed: false,
             active_item_index: 0,
             last_focused_view_by_item: Default::default(),
@@ -413,7 +415,8 @@ impl Pane {
     }
 
     fn focus_in(&mut self, cx: &mut ViewContext<Self>) {
-        if !self.has_focus(cx) {
+        if !self.was_focused {
+            self.was_focused = true;
             cx.emit(Event::Focus);
             cx.notify();
         }
@@ -444,6 +447,7 @@ impl Pane {
     }
 
     fn focus_out(&mut self, cx: &mut ViewContext<Self>) {
+        self.was_focused = false;
         self.toolbar.update(cx, |toolbar, cx| {
             toolbar.focus_changed(false, cx);
         });

crates/zed2/src/zed2.rs

@@ -419,7 +419,7 @@ pub fn initialize_workspace(app_state: Arc<AppState>, cx: &mut AppContext) {
 fn initialize_pane(workspace: &mut Workspace, pane: &View<Pane>, cx: &mut ViewContext<Workspace>) {
     pane.update(cx, |pane, cx| {
         pane.toolbar().update(cx, |toolbar, cx| {
-            let breadcrumbs = cx.build_view(|_| Breadcrumbs::new(workspace));
+            let breadcrumbs = cx.build_view(|_| Breadcrumbs::new());
             toolbar.add_item(breadcrumbs, cx);
             let buffer_search_bar = cx.build_view(search::BufferSearchBar::new);
             toolbar.add_item(buffer_search_bar.clone(), cx);