Fix focus issues with gpui2 docks (#3606)

Kirill Bulatov created

* Fixed dock toggling not focusing the terminal element
* Fixed loosing focus on dock close (e.g. cmd-d on the last terminal in
the dock)
* Removed element stateless focus API since it would not work when the
element is not rendered, update all API usages to the stateful one via
`gpui::Subscription`

Release Notes:

- N/A

Change summary

crates/diagnostics2/src/diagnostics.rs     | 24 +++++--
crates/gpui2/src/app.rs                    | 44 +++++++-------
crates/gpui2/src/elements/div.rs           | 73 ------------------------
crates/gpui2/src/window.rs                 | 47 +++++++++++++++
crates/storybook2/src/stories/focus.rs     | 54 +++++++++++-----
crates/terminal_view2/src/terminal_view.rs | 33 +++------
crates/ui2/src/components/context_menu.rs  | 12 ++-
crates/workspace2/src/dock.rs              |  1 
crates/workspace2/src/pane.rs              | 25 +++----
crates/workspace2/src/workspace2.rs        |  6 +
10 files changed, 158 insertions(+), 161 deletions(-)

Detailed changes

crates/diagnostics2/src/diagnostics.rs 🔗

@@ -13,10 +13,10 @@ use editor::{
 };
 use futures::future::try_join_all;
 use gpui::{
-    actions, div, AnyElement, AnyView, AppContext, Context, Div, EventEmitter, FocusEvent,
-    FocusHandle, Focusable, FocusableElement, FocusableView, InteractiveElement, IntoElement,
-    Model, ParentElement, Render, SharedString, Styled, Subscription, Task, View, ViewContext,
-    VisualContext, WeakView, WindowContext,
+    actions, div, AnyElement, AnyView, AppContext, Context, Div, EventEmitter, FocusHandle,
+    Focusable, FocusableView, InteractiveElement, IntoElement, Model, ParentElement, Render,
+    SharedString, Styled, Subscription, Task, View, ViewContext, VisualContext, WeakView,
+    WindowContext,
 };
 use language::{
     Anchor, Bias, Buffer, Diagnostic, DiagnosticEntry, DiagnosticSeverity, Point, Selection,
@@ -109,7 +109,6 @@ impl Render for ProjectDiagnosticsEditor {
         div()
             .track_focus(&self.focus_handle)
             .size_full()
-            .on_focus_in(cx.listener(Self::focus_in))
             .on_action(cx.listener(Self::toggle_warnings))
             .child(child)
     }
@@ -149,6 +148,11 @@ impl ProjectDiagnosticsEditor {
                 _ => {}
             });
 
+        let focus_handle = cx.focus_handle();
+
+        let focus_in_subscription =
+            cx.on_focus_in(&focus_handle, |diagnostics, cx| diagnostics.focus_in(cx));
+
         let excerpts = cx.build_model(|cx| MultiBuffer::new(project_handle.read(cx).replica_id()));
         let editor = cx.build_view(|cx| {
             let mut editor =
@@ -171,13 +175,17 @@ impl ProjectDiagnosticsEditor {
             summary,
             workspace,
             excerpts,
-            focus_handle: cx.focus_handle(),
+            focus_handle,
             editor,
             path_states: Default::default(),
             paths_to_update: HashMap::default(),
             include_warnings: ProjectDiagnosticsSettings::get_global(cx).include_warnings,
             current_diagnostics: HashMap::default(),
-            _subscriptions: vec![project_event_subscription, editor_event_subscription],
+            _subscriptions: vec![
+                project_event_subscription,
+                editor_event_subscription,
+                focus_in_subscription,
+            ],
         };
         this.update_excerpts(None, cx);
         this
@@ -202,7 +210,7 @@ impl ProjectDiagnosticsEditor {
         cx.notify();
     }
 
-    fn focus_in(&mut self, _: &FocusEvent, cx: &mut ViewContext<Self>) {
+    fn focus_in(&mut self, cx: &mut ViewContext<Self>) {
         if self.focus_handle.is_focused(cx) && !self.path_states.is_empty() {
             self.editor.focus_handle(cx).focus(cx)
         }

crates/gpui2/src/app.rs 🔗

@@ -599,30 +599,32 @@ impl AppContext {
                     }
                 }
             } else {
-                break;
-            }
-        }
+                for window in self.windows.values() {
+                    if let Some(window) = window.as_ref() {
+                        if window.dirty {
+                            window.platform_window.invalidate();
+                        }
+                    }
+                }
+
+                #[cfg(any(test, feature = "test-support"))]
+                for window in self
+                    .windows
+                    .values()
+                    .filter_map(|window| {
+                        let window = window.as_ref()?;
+                        window.dirty.then_some(window.handle)
+                    })
+                    .collect::<Vec<_>>()
+                {
+                    self.update_window(window, |_, cx| cx.draw()).unwrap();
+                }
 
-        for window in self.windows.values() {
-            if let Some(window) = window.as_ref() {
-                if window.dirty {
-                    window.platform_window.invalidate();
+                if self.pending_effects.is_empty() {
+                    break;
                 }
             }
         }
-
-        #[cfg(any(test, feature = "test-support"))]
-        for window in self
-            .windows
-            .values()
-            .filter_map(|window| {
-                let window = window.as_ref()?;
-                window.dirty.then_some(window.handle)
-            })
-            .collect::<Vec<_>>()
-        {
-            self.update_window(window, |_, cx| cx.draw()).unwrap();
-        }
     }
 
     /// Repeatedly called during `flush_effects` to release any entities whose
@@ -646,8 +648,6 @@ impl AppContext {
     }
 
     /// Repeatedly called during `flush_effects` to handle a focused handle being dropped.
-    /// For now, we simply blur the window if this happens, but we may want to support invoking
-    /// a window blur handler to restore focus to some logical element.
     fn release_dropped_focus_handles(&mut self) {
         for window_handle in self.windows() {
             window_handle

crates/gpui2/src/elements/div.rs 🔗

@@ -457,79 +457,6 @@ pub trait FocusableElement: InteractiveElement {
         self.interactivity().in_focus_style = Some(Box::new(f(StyleRefinement::default())));
         self
     }
-
-    fn on_focus(mut self, listener: impl Fn(&FocusEvent, &mut WindowContext) + 'static) -> Self
-    where
-        Self: Sized,
-    {
-        self.interactivity()
-            .focus_listeners
-            .push(Box::new(move |focus_handle, event, cx| {
-                if event.focused.as_ref() == Some(focus_handle) {
-                    listener(event, cx)
-                }
-            }));
-        self
-    }
-
-    fn on_blur(mut self, listener: impl Fn(&FocusEvent, &mut WindowContext) + 'static) -> Self
-    where
-        Self: Sized,
-    {
-        self.interactivity()
-            .focus_listeners
-            .push(Box::new(move |focus_handle, event, cx| {
-                if event.blurred.as_ref() == Some(focus_handle) {
-                    listener(event, cx)
-                }
-            }));
-        self
-    }
-
-    fn on_focus_in(mut self, listener: impl Fn(&FocusEvent, &mut WindowContext) + 'static) -> Self
-    where
-        Self: Sized,
-    {
-        self.interactivity()
-            .focus_listeners
-            .push(Box::new(move |focus_handle, event, cx| {
-                let descendant_blurred = event
-                    .blurred
-                    .as_ref()
-                    .map_or(false, |blurred| focus_handle.contains(blurred, cx));
-                let descendant_focused = event
-                    .focused
-                    .as_ref()
-                    .map_or(false, |focused| focus_handle.contains(focused, cx));
-
-                if !descendant_blurred && descendant_focused {
-                    listener(event, cx)
-                }
-            }));
-        self
-    }
-
-    fn on_focus_out(mut self, listener: impl Fn(&FocusEvent, &mut WindowContext) + 'static) -> Self
-    where
-        Self: Sized,
-    {
-        self.interactivity()
-            .focus_listeners
-            .push(Box::new(move |focus_handle, event, cx| {
-                let descendant_blurred = event
-                    .blurred
-                    .as_ref()
-                    .map_or(false, |blurred| focus_handle.contains(blurred, cx));
-                let descendant_focused = event
-                    .focused
-                    .as_ref()
-                    .map_or(false, |focused| focus_handle.contains(focused, cx));
-                if descendant_blurred && !descendant_focused {
-                    listener(event, cx)
-                }
-            }));
-        self
-    }
 }
 
 pub type FocusListeners = Vec<FocusListener>;

crates/gpui2/src/window.rs 🔗

@@ -169,7 +169,7 @@ impl FocusHandle {
     }
 
     /// Obtains whether this handle contains the given handle in the most recently rendered frame.
-    pub(crate) fn contains(&self, other: &Self, cx: &WindowContext) -> bool {
+    pub fn contains(&self, other: &Self, cx: &WindowContext) -> bool {
         self.id.contains(other.id, cx)
     }
 }
@@ -228,6 +228,7 @@ pub struct Window {
     pub(crate) next_frame: Frame,
     pub(crate) focus_handles: Arc<RwLock<SlotMap<FocusId, AtomicUsize>>>,
     pub(crate) focus_listeners: SubscriberSet<(), AnyWindowFocusListener>,
+    pub(crate) blur_listeners: SubscriberSet<(), AnyObserver>,
     default_prevented: bool,
     mouse_position: Point<Pixels>,
     requested_cursor_style: Option<CursorStyle>,
@@ -361,6 +362,7 @@ impl Window {
             next_frame: Frame::new(DispatchTree::new(cx.keymap.clone(), cx.actions.clone())),
             focus_handles: Arc::new(RwLock::new(SlotMap::with_key())),
             focus_listeners: SubscriberSet::new(),
+            blur_listeners: SubscriberSet::new(),
             default_prevented: true,
             mouse_position,
             requested_cursor_style: None,
@@ -1234,6 +1236,16 @@ impl<'a> WindowContext<'a> {
 
     /// Draw pixels to the display for this window based on the contents of its scene.
     pub(crate) fn draw(&mut self) -> Scene {
+        let window_was_focused = self
+            .window
+            .focus
+            .and_then(|focus_id| {
+                self.window
+                    .rendered_frame
+                    .dispatch_tree
+                    .focusable_node_id(focus_id)
+            })
+            .is_some();
         self.text_system().start_frame();
         self.window.platform_window.clear_input_handler();
         self.window.layout_engine.as_mut().unwrap().clear();
@@ -1272,6 +1284,23 @@ impl<'a> WindowContext<'a> {
             });
         }
 
+        let window_is_focused = self
+            .window
+            .focus
+            .and_then(|focus_id| {
+                self.window
+                    .next_frame
+                    .dispatch_tree
+                    .focusable_node_id(focus_id)
+            })
+            .is_some();
+        if window_was_focused && !window_is_focused {
+            self.window
+                .blur_listeners
+                .clone()
+                .retain(&(), |listener| listener(self));
+        }
+
         self.window
             .next_frame
             .dispatch_tree
@@ -2419,6 +2448,22 @@ impl<'a, V: 'static> ViewContext<'a, V> {
         subscription
     }
 
+    /// Register a listener to be called when the window loses focus.
+    /// Unlike [on_focus_changed], returns a subscription and persists until the subscription
+    /// is dropped.
+    pub fn on_blur_window(
+        &mut self,
+        mut listener: impl FnMut(&mut V, &mut ViewContext<V>) + 'static,
+    ) -> Subscription {
+        let view = self.view.downgrade();
+        let (subscription, activate) = self.window.blur_listeners.insert(
+            (),
+            Box::new(move |cx| view.update(cx, |view, cx| listener(view, cx)).is_ok()),
+        );
+        activate();
+        subscription
+    }
+
     /// Register a listener to be called when the given focus handle or one of its descendants loses focus.
     /// Unlike [on_focus_changed], returns a subscription and persists until the subscription
     /// is dropped.

crates/storybook2/src/stories/focus.rs 🔗

@@ -1,14 +1,16 @@
 use gpui::{
-    actions, div, prelude::*, Div, FocusHandle, Focusable, KeyBinding, Render, Stateful, View,
-    WindowContext,
+    actions, div, prelude::*, Div, FocusHandle, Focusable, KeyBinding, Render, Stateful,
+    Subscription, View, WindowContext,
 };
 use ui::prelude::*;
 
 actions!(focus, [ActionA, ActionB, ActionC]);
 
 pub struct FocusStory {
+    parent_focus: FocusHandle,
     child_1_focus: FocusHandle,
     child_2_focus: FocusHandle,
+    _focus_subscriptions: Vec<Subscription>,
 }
 
 impl FocusStory {
@@ -19,9 +21,37 @@ impl FocusStory {
             KeyBinding::new("cmd-c", ActionC, None),
         ]);
 
-        cx.build_view(move |cx| Self {
-            child_1_focus: cx.focus_handle(),
-            child_2_focus: cx.focus_handle(),
+        cx.build_view(move |cx| {
+            let parent_focus = cx.focus_handle();
+            let child_1_focus = cx.focus_handle();
+            let child_2_focus = cx.focus_handle();
+            let _focus_subscriptions = vec![
+                cx.on_focus(&parent_focus, |_, _| {
+                    println!("Parent focused");
+                }),
+                cx.on_blur(&parent_focus, |_, _| {
+                    println!("Parent blurred");
+                }),
+                cx.on_focus(&child_1_focus, |_, _| {
+                    println!("Child 1 focused");
+                }),
+                cx.on_blur(&child_1_focus, |_, _| {
+                    println!("Child 1 blurred");
+                }),
+                cx.on_focus(&child_2_focus, |_, _| {
+                    println!("Child 2 focused");
+                }),
+                cx.on_blur(&child_2_focus, |_, _| {
+                    println!("Child 2 blurred");
+                }),
+            ];
+
+            Self {
+                parent_focus,
+                child_1_focus,
+                child_2_focus,
+                _focus_subscriptions,
+            }
         })
     }
 }
@@ -39,7 +69,7 @@ impl Render for FocusStory {
 
         div()
             .id("parent")
-            .focusable()
+            .track_focus(&self.parent_focus)
             .key_context("parent")
             .on_action(cx.listener(|_, _action: &ActionA, _cx| {
                 println!("Action A dispatched on parent");
@@ -47,10 +77,6 @@ impl Render for FocusStory {
             .on_action(cx.listener(|_, _action: &ActionB, _cx| {
                 println!("Action B dispatched on parent");
             }))
-            .on_focus(cx.listener(|_, _, _| println!("Parent focused")))
-            .on_blur(cx.listener(|_, _, _| println!("Parent blurred")))
-            .on_focus_in(cx.listener(|_, _, _| println!("Parent focus_in")))
-            .on_focus_out(cx.listener(|_, _, _| println!("Parent focus_out")))
             .on_key_down(cx.listener(|_, event, _| println!("Key down on parent {:?}", event)))
             .on_key_up(cx.listener(|_, event, _| println!("Key up on parent {:?}", event)))
             .size_full()
@@ -68,10 +94,6 @@ impl Render for FocusStory {
                     .bg(color_4)
                     .focus(|style| style.bg(color_5))
                     .in_focus(|style| style.bg(color_6))
-                    .on_focus(cx.listener(|_, _, _| println!("Child 1 focused")))
-                    .on_blur(cx.listener(|_, _, _| println!("Child 1 blurred")))
-                    .on_focus_in(cx.listener(|_, _, _| println!("Child 1 focus_in")))
-                    .on_focus_out(cx.listener(|_, _, _| println!("Child 1 focus_out")))
                     .on_key_down(
                         cx.listener(|_, event, _| println!("Key down on child 1 {:?}", event)),
                     )
@@ -88,10 +110,6 @@ impl Render for FocusStory {
                     .w_full()
                     .h_6()
                     .bg(color_4)
-                    .on_focus(cx.listener(|_, _, _| println!("Child 2 focused")))
-                    .on_blur(cx.listener(|_, _, _| println!("Child 2 blurred")))
-                    .on_focus_in(cx.listener(|_, _, _| println!("Child 2 focus_in")))
-                    .on_focus_out(cx.listener(|_, _, _| println!("Child 2 focus_out")))
                     .on_key_down(
                         cx.listener(|_, event, _| println!("Key down on child 2 {:?}", event)),
                     )

crates/terminal_view2/src/terminal_view.rs 🔗

@@ -10,9 +10,8 @@ pub mod terminal_panel;
 use editor::{scroll::autoscroll::Autoscroll, Editor};
 use gpui::{
     div, impl_actions, overlay, AnyElement, AppContext, DismissEvent, Div, EventEmitter,
-    FocusEvent, FocusHandle, Focusable, FocusableElement, FocusableView, KeyContext, KeyDownEvent,
-    Keystroke, Model, MouseButton, MouseDownEvent, Pixels, Render, Styled, Subscription, Task,
-    View, VisualContext, WeakView,
+    FocusHandle, Focusable, FocusableView, KeyContext, KeyDownEvent, Keystroke, Model, MouseButton,
+    MouseDownEvent, Pixels, Render, Styled, Subscription, Task, View, VisualContext, WeakView,
 };
 use language::Bias;
 use persistence::TERMINAL_DB;
@@ -263,19 +262,13 @@ impl TerminalView {
         })
         .detach();
 
-        let focus = cx.focus_handle();
-        // let focus_in = cx.on_focus_in(&focus, |this, cx| {
-        //     this.has_new_content = false;
-        //     this.terminal.read(cx).focus_in();
-        //     this.blink_cursors(this.blink_epoch, cx);
-        //     cx.notify();
-        // });
-        // let focus_out = cx.on_focus_out(&focus, |this, cx| {
-        //     this.terminal.update(cx, |terminal, _| {
-        //         terminal.focus_out();
-        //     });
-        //     cx.notify();
-        // });
+        let focus_handle = cx.focus_handle();
+        let focus_in = cx.on_focus_in(&focus_handle, |terminal_view, cx| {
+            terminal_view.focus_in(cx);
+        });
+        let focus_out = cx.on_focus_out(&focus_handle, |terminal_view, cx| {
+            terminal_view.focus_out(cx);
+        });
 
         Self {
             terminal,
@@ -289,7 +282,7 @@ impl TerminalView {
             blink_epoch: 0,
             can_navigate_to_selected_word: false,
             workspace_id,
-            _subscriptions: vec![/*focus_in, focus_out*/],
+            _subscriptions: vec![focus_in, focus_out],
         }
     }
 
@@ -614,14 +607,14 @@ impl TerminalView {
         });
     }
 
-    fn focus_in(&mut self, event: &FocusEvent, cx: &mut ViewContext<Self>) {
+    fn focus_in(&mut self, cx: &mut ViewContext<Self>) {
         self.has_new_content = false;
         self.terminal.read(cx).focus_in();
         self.blink_cursors(self.blink_epoch, cx);
         cx.notify();
     }
 
-    fn focus_out(&mut self, event: &FocusEvent, cx: &mut ViewContext<Self>) {
+    fn focus_out(&mut self, cx: &mut ViewContext<Self>) {
         self.terminal.update(cx, |terminal, _| {
             terminal.focus_out();
         });
@@ -650,8 +643,6 @@ impl Render for TerminalView {
             .on_action(cx.listener(TerminalView::clear))
             .on_action(cx.listener(TerminalView::show_character_palette))
             .on_action(cx.listener(TerminalView::select_all))
-            .on_focus_in(cx.listener(Self::focus_in))
-            .on_focus_out(cx.listener(Self::focus_out))
             .on_key_down(cx.listener(Self::key_down))
             .on_mouse_down(
                 MouseButton::Right,

crates/ui2/src/components/context_menu.rs 🔗

@@ -4,7 +4,7 @@ use crate::{
 };
 use gpui::{
     px, Action, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, FocusableView,
-    IntoElement, Render, View, VisualContext,
+    IntoElement, Render, Subscription, View, VisualContext,
 };
 use menu::{SelectFirst, SelectLast, SelectNext, SelectPrev};
 use std::{rc::Rc, time::Duration};
@@ -25,6 +25,7 @@ pub struct ContextMenu {
     focus_handle: FocusHandle,
     selected_index: Option<usize>,
     delayed: bool,
+    _on_blur_subscription: Subscription,
 }
 
 impl FocusableView for ContextMenu {
@@ -40,14 +41,18 @@ impl ContextMenu {
         cx: &mut WindowContext,
         f: impl FnOnce(Self, &mut WindowContext) -> Self,
     ) -> View<Self> {
-        // let handle = cx.view().downgrade();
         cx.build_view(|cx| {
+            let focus_handle = cx.focus_handle();
+            let _on_blur_subscription = cx.on_blur(&focus_handle, |this: &mut ContextMenu, cx| {
+                this.cancel(&menu::Cancel, cx)
+            });
             f(
                 Self {
                     items: Default::default(),
-                    focus_handle: cx.focus_handle(),
+                    focus_handle,
                     selected_index: None,
                     delayed: false,
+                    _on_blur_subscription,
                 },
                 cx,
             )
@@ -223,7 +228,6 @@ impl Render for ContextMenu {
                     }
                     el
                 })
-                .on_blur(cx.listener(|this, _, cx| this.cancel(&Default::default(), cx)))
                 .flex_none()
                 .child(
                     List::new().children(self.items.iter().enumerate().map(

crates/workspace2/src/dock.rs 🔗

@@ -346,6 +346,7 @@ impl Dock {
                         })
                         .ok();
                 }
+                // todo!() we do not use this event in the production code (even in zed1), remove it
                 PanelEvent::Activate => {
                     if let Some(ix) = this
                         .panel_entries

crates/workspace2/src/pane.rs 🔗

@@ -10,7 +10,7 @@ use gpui::{
     actions, impl_actions, overlay, prelude::*, rems, Action, AnchorCorner, AnyWeakView,
     AppContext, AsyncWindowContext, DismissEvent, Div, EntityId, EventEmitter, FocusHandle,
     Focusable, FocusableView, Model, MouseButton, NavigationDirection, Pixels, Point, PromptLevel,
-    Render, Task, View, ViewContext, VisualContext, WeakView, WindowContext,
+    Render, Subscription, Task, View, ViewContext, VisualContext, WeakView, WindowContext,
 };
 use parking_lot::Mutex;
 use project::{Project, ProjectEntryId, ProjectPath};
@@ -188,6 +188,7 @@ pub struct Pane {
     //     can_drop: Rc<dyn Fn(&DragAndDrop<Workspace>, &WindowContext) -> bool>,
     can_split: bool,
     //     render_tab_bar_buttons: Rc<dyn Fn(&mut Pane, &mut ViewContext<Pane>) -> AnyElement<Pane>>,
+    subscriptions: Vec<Subscription>,
 }
 
 pub struct ItemNavHistory {
@@ -326,10 +327,17 @@ impl Pane {
         // context_menu.update(cx, |menu, _| {
         //     menu.set_position_mode(OverlayPositionMode::Local)
         // });
+        //
+        let focus_handle = cx.focus_handle();
+
+        let subscriptions = vec![
+            cx.on_focus_in(&focus_handle, move |this, cx| this.focus_in(cx)),
+            cx.on_focus_out(&focus_handle, move |this, cx| this.focus_out(cx)),
+        ];
 
         let handle = cx.view().downgrade();
         Self {
-            focus_handle: cx.focus_handle(),
+            focus_handle,
             items: Vec::new(),
             activation_history: Vec::new(),
             was_focused: false,
@@ -416,6 +424,7 @@ impl Pane {
             //         })
             //         .into_any()
             // }),
+            subscriptions,
         }
     }
 
@@ -2098,18 +2107,6 @@ impl Render for Pane {
             .track_focus(&self.focus_handle)
             .size_full()
             .overflow_hidden()
-            .on_focus_in({
-                let this = this.clone();
-                move |event, cx| {
-                    this.update(cx, |this, cx| this.focus_in(cx)).ok();
-                }
-            })
-            .on_focus_out({
-                let this = this.clone();
-                move |event, cx| {
-                    this.update(cx, |this, cx| this.focus_out(cx)).ok();
-                }
-            })
             .on_action(cx.listener(|pane, _: &SplitLeft, cx| pane.split(SplitDirection::Left, cx)))
             .on_action(cx.listener(|pane, _: &SplitUp, cx| pane.split(SplitDirection::Up, cx)))
             .on_action(

crates/workspace2/src/workspace2.rs 🔗

@@ -532,6 +532,11 @@ impl Workspace {
             cx.notify()
         })
         .detach();
+        cx.on_blur_window(|this, cx| {
+            let focus_handle = this.focus_handle(cx);
+            cx.focus(&focus_handle);
+        })
+        .detach();
 
         let weak_handle = cx.view().downgrade();
         let pane_history_timestamp = Arc::new(AtomicUsize::new(0));
@@ -1633,6 +1638,7 @@ impl Workspace {
                             panel.focus_handle(cx).focus(cx);
                             reveal_dock = true;
                         } else {
+                            // todo!()
                             // if panel.is_zoomed(cx) {
                             //     dock.set_open(false, cx);
                             // }