Ensure that notify observations are sent during `Window::draw()` (#4236)

Mikayla Maki created 2 years ago

This PR fixes a bug in GPUI where the observation API would not be
triggered if a focus event caused a `notify()`

Release Notes:

- N/A

Change summary

crates/gpui/src/element.rs | 19 +++++++++++
crates/gpui/src/window.rs  | 64 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 80 insertions(+), 3 deletions(-)

Detailed changes

crates/gpui/src/element.rs

@@ -136,6 +136,25 @@ impl Render for () {
     fn render(&mut self, _cx: &mut ViewContext<Self>) -> impl IntoElement {}
 }
 
+/// A quick way to create a [`Render`]able view without having to define a new type.
+#[cfg(any(test, feature = "test-support"))]
+pub struct TestView(Box<dyn FnMut(&mut ViewContext<TestView>) -> AnyElement>);
+
+#[cfg(any(test, feature = "test-support"))]
+impl TestView {
+    /// Construct a TestView from a render closure.
+    pub fn new<F: FnMut(&mut ViewContext<TestView>) -> AnyElement + 'static>(f: F) -> Self {
+        Self(Box::new(f))
+    }
+}
+
+#[cfg(any(test, feature = "test-support"))]
+impl Render for TestView {
+    fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement {
+        (self.0)(cx)
+    }
+}
+
 /// You can derive [`IntoElement`] on any type that implements this trait.
 /// It is used to construct reusable `components` out of plain data. Think of
 /// components as a recipe for a certain pattern of elements. RenderOnce allows

crates/gpui/src/window.rs

@@ -94,6 +94,7 @@ type AnyObserver = Box<dyn FnMut(&mut WindowContext) -> bool + 'static>;
 
 type AnyWindowFocusListener = Box<dyn FnMut(&FocusEvent, &mut WindowContext) -> bool + 'static>;
 
+#[derive(Debug)]
 struct FocusEvent {
     previous_focus_path: SmallVec<[FocusId; 8]>,
     current_focus_path: SmallVec<[FocusId; 8]>,
@@ -2015,11 +2016,12 @@ impl<'a, V: 'static> ViewContext<'a, V> {
             }
         }
 
+        // Always emit a notify effect, so that handlers fire correctly
+        self.window_cx.app.push_effect(Effect::Notify {
+            emitter: self.view.model.entity_id,
+        });
         if !self.window.drawing {
             self.window_cx.window.dirty = true;
-            self.window_cx.app.push_effect(Effect::Notify {
-                emitter: self.view.model.entity_id,
-            });
         }
     }
 
@@ -2751,3 +2753,59 @@ pub fn outline(bounds: impl Into<Bounds<Pixels>>, border_color: impl Into<Hsla>)
         border_color: border_color.into(),
     }
 }
+
+#[cfg(test)]
+mod test {
+
+    use std::{cell::RefCell, rc::Rc};
+
+    use crate::{
+        self as gpui, div, FocusHandle, InteractiveElement, IntoElement, Render, TestAppContext,
+        ViewContext, VisualContext,
+    };
+
+    #[gpui::test]
+    fn test_notify_on_focus(cx: &mut TestAppContext) {
+        struct TestFocusView {
+            handle: FocusHandle,
+        }
+
+        impl Render for TestFocusView {
+            fn render(&mut self, _cx: &mut ViewContext<Self>) -> impl IntoElement {
+                div().id("test").track_focus(&self.handle)
+            }
+        }
+
+        let notify_counter = Rc::new(RefCell::new(0));
+
+        let (notify_producer, cx) = cx.add_window_view(|cx| {
+            cx.activate_window();
+            let handle = cx.focus_handle();
+
+            cx.on_focus(&handle, |_, cx| {
+                cx.notify();
+            })
+            .detach();
+
+            TestFocusView { handle }
+        });
+
+        let focus_handle = cx.update(|cx| notify_producer.read(cx).handle.clone());
+
+        let _notify_consumer = cx.new_view({
+            |cx| {
+                let notify_counter = notify_counter.clone();
+                cx.observe(&notify_producer, move |_, _, _| {
+                    *notify_counter.borrow_mut() += 1;
+                })
+                .detach();
+            }
+        });
+
+        cx.update(|cx| {
+            cx.focus(&focus_handle);
+        });
+
+        assert_eq!(*notify_counter.borrow(), 1);
+    }
+}