Attempt to remove the dangeous element focus API

Kirill Bulatov created

Change summary

crates/diagnostics2/src/diagnostics.rs     | 24 ++++++++----
crates/gpui2/src/elements/div.rs           | 46 -----------------------
crates/gpui2/src/window.rs                 |  1 
crates/storybook2/src/stories/focus.rs     | 15 ++++---
crates/terminal_view2/src/terminal_view.rs | 33 ++++++----------
5 files changed, 39 insertions(+), 80 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/elements/div.rs 🔗

@@ -441,6 +441,7 @@ pub trait StatefulInteractiveElement: InteractiveElement {
     }
 }
 
+// TODO kb do we leave those on an element?
 pub trait FocusableElement: InteractiveElement {
     fn focus(mut self, f: impl FnOnce(StyleRefinement) -> StyleRefinement) -> Self
     where
@@ -485,51 +486,6 @@ pub trait FocusableElement: InteractiveElement {
             }));
         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 = SmallVec<[FocusListener; 2]>;

crates/gpui2/src/window.rs 🔗

@@ -168,6 +168,7 @@ impl FocusHandle {
         self.id.within_focused(cx)
     }
 
+    // TODO kb now never used?
     /// Obtains whether this handle contains the given handle in the most recently rendered frame.
     pub(crate) fn contains(&self, other: &Self, cx: &WindowContext) -> bool {
         self.id.contains(other.id, cx)

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

@@ -49,8 +49,9 @@ impl Render for FocusStory {
             }))
             .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")))
+            // TODO kb todo!() remove?
+            // .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()
@@ -70,8 +71,9 @@ impl Render for FocusStory {
                     .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")))
+                    // TODO kb todo!() remove?
+                    // .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)),
                     )
@@ -90,8 +92,9 @@ impl Render for FocusStory {
                     .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")))
+                    // TODO kb todo!() remove?
+                    // .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,