Finish removing all dangerous focus APIs

Kirill Bulatov created

Change summary

crates/gpui2/src/elements/div.rs          | 29 ------------
crates/gpui2/src/window.rs                |  3 
crates/storybook2/src/stories/focus.rs    | 57 +++++++++++++++---------
crates/ui2/src/components/context_menu.rs | 12 +++-
4 files changed, 45 insertions(+), 56 deletions(-)

Detailed changes

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

@@ -441,7 +441,6 @@ 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
@@ -458,34 +457,6 @@ pub trait FocusableElement: InteractiveElement {
         self.interactivity().in_focus_style = 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
-    }
 }
 
 pub type FocusListeners = SmallVec<[FocusListener; 2]>;

crates/gpui2/src/window.rs 🔗

@@ -168,9 +168,8 @@ 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 {
+    pub fn contains(&self, other: &Self, cx: &WindowContext) -> bool {
         self.id.contains(other.id, cx)
     }
 }

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,11 +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")))
-            // 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()
@@ -69,11 +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")))
-                    // 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,11 +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")))
-                    // 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/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(