Cleanup global event listeners (#124)

ealmloff · web-flow · commit 2901fd2d9e33 · 2025-10-09T12:38:31.000-05:00
* Cleanup global event listeners

* rename ReadOnlySignal to ReadSignal

* fix clippy

* remove patch

* use main branch for patch

* pull out keydown listener logic

* only call the top escape handler

* fix popover

* fix clippy

* bump dioxus version

* bump to rc.1

* update lockfile

* fix use_global_escape_listener
diff --git a/complaints.md b/complaints.md
@@ -12,11 +12,11 @@ It's verbose to set a `Signal` or `ReadSignal`'s default value through props.
 pub struct SomeProps {
 
     // This sets bool to be false
-    #[props(default)]
+    #[props(default)] 
     value: ReadSignal<bool>,
 
     // This is what I'd like, except it wants a ReadSignal
-    #[props(default = true)]
+    #[props(default = true)] 
     value: ReadSignal<bool>,
 
     // Instead you have to do this:
diff --git a/primitives/src/alert_dialog.rs b/primitives/src/alert_dialog.rs
@@ -1,5 +1,6 @@
 //! Defines the [`AlertDialogRoot`] component and its sub-components.
 
+use crate::use_global_escape_listener;
 use crate::{use_animated_open, use_id_or, use_unique_id, FOCUS_TRAP_JS};
 use dioxus::document;
 use dioxus::prelude::*;
@@ -93,25 +94,6 @@ pub fn AlertDialogRoot(props: AlertDialogRootProps) -> Element {
         describedby,
     });
 
-    // Add a escape key listener to the document when the dialog is open. We can't
-    // just add this to the dialog itself because it might not be focused if the user
-    // is highlighting text or interacting with another element.
-    use_effect(move || {
-        let mut escape = document::eval(
-            "document.addEventListener('keydown', (event) => {
-                if (event.key === 'Escape') {
-                    event.preventDefault();
-                    dioxus.send(true);
-                }
-            });",
-        );
-        spawn(async move {
-            while let Ok(true) = escape.recv().await {
-                set_open.call(false);
-            }
-        });
-    });
-
     let id = use_unique_id();
     let id = use_id_or(id, props.id);
     let render_element = use_animated_open(id, open);
@@ -195,6 +177,12 @@ pub fn AlertDialogContent(props: AlertDialogContentProps) -> Element {
     let ctx: AlertDialogCtx = use_context();
 
     let open = ctx.open;
+    let set_open = ctx.set_open;
+
+    // Add a escape key listener to the document when the dialog is open. We can't
+    // just add this to the dialog itself because it might not be focused if the user
+    // is highlighting text or interacting with another element.
+    use_global_escape_listener(move || set_open.call(false));
 
     let gen_id = use_unique_id();
     let id = use_id_or(gen_id, props.id);
diff --git a/primitives/src/dialog.rs b/primitives/src/dialog.rs
@@ -3,6 +3,7 @@
 use dioxus::document;
 use dioxus::prelude::*;
 
+use crate::use_global_escape_listener;
 use crate::{use_animated_open, use_controlled, use_id_or, use_unique_id, FOCUS_TRAP_JS};
 
 #[derive(Clone, Copy)]
@@ -109,25 +110,6 @@ pub fn DialogRoot(props: DialogRootProps) -> Element {
         dialog_describedby,
     });
 
-    // Add a escape key listener to the document when the dialog is open. We can't
-    // just add this to the dialog itself because it might not be focused if the user
-    // is highlighting text or interacting with another element.
-    use_effect(move || {
-        let mut escape = document::eval(
-            "document.addEventListener('keydown', (event) => {
-                if (event.key === 'Escape') {
-                    event.preventDefault();
-                    dioxus.send(true);
-                }
-            });",
-        );
-        spawn(async move {
-            while let Ok(true) = escape.recv().await {
-                set_open.call(false);
-            }
-        });
-    });
-
     let unique_id = use_unique_id();
     let id = use_id_or(unique_id, props.id);
 
@@ -224,6 +206,12 @@ pub fn DialogContent(props: DialogContentProps) -> Element {
     let ctx: DialogCtx = use_context();
     let open = ctx.open;
     let is_modal = ctx.is_modal;
+    let set_open = ctx.set_open;
+
+    // Add a escape key listener to the document when the dialog is open. We can't
+    // just add this to the dialog itself because it might not be focused if the user
+    // is highlighting text or interacting with another element.
+    use_global_escape_listener(move || set_open.call(false));
 
     let gen_id = use_unique_id();
     let id = use_id_or(gen_id, props.id);
diff --git a/primitives/src/lib.rs b/primitives/src/lib.rs
@@ -1,8 +1,11 @@
 #![doc = include_str!("../README.md")]
 #![warn(missing_docs)]
 
+use std::cell::RefCell;
+use std::rc::Rc;
 use std::sync::atomic::{AtomicUsize, Ordering};
 
+use dioxus::core::{current_scope_id, use_drop};
 use dioxus::prelude::*;
 use dioxus::prelude::{asset, manganis, Asset};
 
@@ -107,6 +110,76 @@ fn use_effect_cleanup<F: FnOnce() + 'static>(#[allow(unused)] cleanup: F) {
     client!(crate::dioxus_core::use_drop(cleanup))
 }
 
+/// Run some cleanup code when the component is unmounted if the effect was run.
+fn use_effect_with_cleanup<F: FnMut() -> C + 'static, C: FnOnce() + 'static>(mut effect: F) {
+    let mut cleanup = use_hook(|| CopyValue::new(None as Option<C>));
+    use_effect(move || {
+        if let Some(cleanup) = cleanup.take() {
+            cleanup();
+        }
+        cleanup.set(Some(effect()));
+    });
+    client!(crate::dioxus_core::use_drop(move || {
+        if let Some(cleanup) = cleanup.take() {
+            cleanup();
+        }
+    }))
+}
+
+/// A stack of escape listeners to allow only the top-most listener to be called.
+#[derive(Clone)]
+struct EscapeListenerStack(Rc<RefCell<Vec<ScopeId>>>);
+
+fn use_global_escape_listener(mut on_escape: impl FnMut() + Clone + 'static) {
+    let scope_id = current_scope_id();
+    let stack = use_hook(move || {
+        // Get or create the escape listener stack
+        let stack: EscapeListenerStack = try_consume_context()
+            .unwrap_or_else(|| provide_context(EscapeListenerStack(Default::default())));
+        // Push the current scope onto the stack
+        stack.0.borrow_mut().push(scope_id);
+        stack
+    });
+    // Remove the current scope id from the stack when we unmount
+    use_drop({
+        let stack = stack.clone();
+        move || {
+            let mut stack = stack.0.borrow_mut();
+            stack.retain(|id| *id != scope_id);
+        }
+    });
+    use_global_keydown_listener("Escape", move || {
+        // Only call the listener if this component is on top of the stack
+        let stack = stack.0.borrow();
+        if stack.last() == Some(&scope_id) {
+            on_escape();
+        }
+    });
+}
+
+fn use_global_keydown_listener(key: &'static str, on_escape: impl FnMut() + Clone + 'static) {
+    use_effect_with_cleanup(move || {
+        let mut escape = document::eval(&format!(
+            "function listener(event) {{
+                if (event.key === '{key}') {{
+                    event.preventDefault();
+                    dioxus.send(true);
+                }}
+            }}
+            document.addEventListener('keydown', listener);
+            await dioxus.recv();
+            document.removeEventListener('keydown', listener);"
+        ));
+        let mut on_escape = on_escape.clone();
+        spawn(async move {
+            while let Ok(true) = escape.recv().await {
+                on_escape();
+            }
+        });
+        move || _ = escape.send(true)
+    });
+}
+
 fn use_animated_open(
     id: impl Readable<Target = String> + Copy + 'static,
     open: impl Readable<Target = bool> + Copy + 'static,
diff --git a/primitives/src/popover.rs b/primitives/src/popover.rs
@@ -3,6 +3,7 @@
 use dioxus::document;
 use dioxus::prelude::*;
 
+use crate::use_global_escape_listener;
 use crate::{
     use_animated_open, use_controlled, use_id_or, use_unique_id, ContentAlign, ContentSide,
     FOCUS_TRAP_JS,
@@ -109,25 +110,6 @@ pub fn PopoverRoot(props: PopoverRootProps) -> Element {
         labelledby,
     });
 
-    // Add a escape key listener to the document when the dialog is open. We can't
-    // just add this to the dialog itself because it might not be focused if the user
-    // is highlighting text or interacting with another element.
-    use_effect(move || {
-        let mut escape = document::eval(
-            "document.addEventListener('keydown', (event) => {
-                if (event.key === 'Escape') {
-                    event.preventDefault();
-                    dioxus.send(true);
-                }
-            });",
-        );
-        spawn(async move {
-            while let Ok(true) = escape.recv().await {
-                set_open.call(false);
-            }
-        });
-    });
-
     rsx! {
         div {
             "data-state": if open() { "open" } else { "closed" },
@@ -219,7 +201,6 @@ pub struct PopoverContentProps {
 pub fn PopoverContent(props: PopoverContentProps) -> Element {
     let ctx: PopoverCtx = use_context();
     let open = ctx.open;
-    let is_open = open();
     let is_modal = ctx.is_modal;
 
     let gen_id = use_unique_id();
@@ -257,23 +238,56 @@ pub fn PopoverContent(props: PopoverContentProps) -> Element {
             defer: true
         }
         if render() {
-            div {
+            PopoverContentRendered {
                 id,
-                role: "dialog",
-                aria_modal: "true",
-                aria_labelledby: ctx.labelledby,
-                aria_hidden: (!is_open).then_some("true"),
-                class: props.class.clone().unwrap_or_else(|| "popover-content".to_string()),
-                "data-state": if is_open { "open" } else { "closed" },
-                "data-side": props.side.as_str(),
-                "data-align": props.align.as_str(),
-                ..props.attributes,
-                {props.children}
+                class: props.class,
+                side: props.side,
+                align: props.align,
+                attributes: props.attributes,
+                children: props.children
             }
         }
     }
 }
 
+/// The rendered content of the popover. This is separated out so the global event listener
+/// is only added when the popover is actually rendered.
+#[component]
+pub fn PopoverContentRendered(
+    id: String,
+    class: Option<String>,
+    side: ContentSide,
+    align: ContentAlign,
+    attributes: Vec<Attribute>,
+    children: Element,
+) -> Element {
+    let ctx: PopoverCtx = use_context();
+    let open = ctx.open;
+    let is_open = open();
+    let set_open = ctx.set_open;
+
+    // Add a escape key listener to the document when the dialog is open. We can't
+    // just add this to the dialog itself because it might not be focused if the user
+    // is highlighting text or interacting with another element.
+    use_global_escape_listener(move || set_open.call(false));
+
+    rsx! {
+        div {
+            id,
+            role: "dialog",
+            aria_modal: "true",
+            aria_labelledby: ctx.labelledby,
+            aria_hidden: (!is_open).then_some("true"),
+            class: class.unwrap_or_else(|| "popover-content".to_string()),
+            "data-state": if is_open { "open" } else { "closed" },
+            "data-side": side.as_str(),
+            "data-align": align.as_str(),
+            ..attributes,
+            {children}
+        }
+    }
+}
+
 /// The props for the [`PopoverTrigger`] component.
 #[derive(Props, Clone, PartialEq)]
 pub struct PopoverTriggerProps {
diff --git a/primitives/src/toast.rs b/primitives/src/toast.rs
@@ -2,7 +2,7 @@
 
 use crate::{
     portal::{use_portal, PortalIn, PortalOut},
-    use_unique_id,
+    use_global_keydown_listener, use_unique_id,
 };
 use dioxus::dioxus_core::DynamicNode;
 use dioxus::prelude::*;
@@ -209,18 +209,8 @@ pub fn ToastProvider(props: ToastProviderProps) -> Element {
         });
     });
 
-    // Mount the first toast when the user presses f6
-    use_effect(move || {
-        let mut eval = dioxus::document::eval(
-            "document.addEventListener('keydown', (event) => { if (event.key === 'F6') { dioxus.send(true) } });",
-        );
-        spawn(async move {
-            while let Ok(true) = eval.recv().await {
-                // Focus the first toast when F6 is pressed
-                focus_region(())
-            }
-        });
-    });
+    // Focus the first toast when the user presses f6
+    use_global_keydown_listener("F6", move || focus_region(()));
 
     // Provide the context
     let ctx = use_context_provider(|| ToastCtx {
