From e416162edf9f71316adf8c8b8026fa061ca285f0 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Thu, 24 Mar 2022 16:58:13 +0100 Subject: [PATCH] address review comments, slight refactorings --- examples/portals/Cargo.toml | 1 - examples/portals/src/main.rs | 11 +-- packages/yew/src/dom_bundle/mod.rs | 2 +- packages/yew/src/dom_bundle/subtree_root.rs | 83 ++++++--------------- 4 files changed, 28 insertions(+), 69 deletions(-) diff --git a/examples/portals/Cargo.toml b/examples/portals/Cargo.toml index 07300a4d603..308c2f8db5b 100644 --- a/examples/portals/Cargo.toml +++ b/examples/portals/Cargo.toml @@ -8,7 +8,6 @@ license = "MIT OR Apache-2.0" [dependencies] yew = { path = "../../packages/yew", features = ["csr"] } gloo-utils = "0.1" -gloo-console = "*" wasm-bindgen = "0.2" [dependencies.web-sys] diff --git a/examples/portals/src/main.rs b/examples/portals/src/main.rs index 56af33ab20d..da751f49c28 100644 --- a/examples/portals/src/main.rs +++ b/examples/portals/src/main.rs @@ -111,10 +111,7 @@ impl Component for App { } fn view(&self, ctx: &Context) -> Html { - let onclick = ctx.link().callback(|ev: web_sys::MouseEvent| { - gloo_console::log!(&ev, &ev.composed_path()); - AppMessage::IncreaseCounter - }); + let onclick = ctx.link().callback(|_| AppMessage::IncreaseCounter); let title = create_portal( html! { if self.counter > 0 { @@ -128,16 +125,16 @@ impl Component for App { html! { <> {self.style_html.clone()} + {title}

{"This paragraph is colored red, and its style is mounted into "}

{"document.head"}
{" with a portal"}

-
+

{"This paragraph is rendered in a shadow dom and thus not affected by the surrounding styling context"}

{"Buttons clicked inside the shadow dom work fine."} - +

{format!("The button has been clicked {} times. This is also reflected in the title of the tab!", self.counter)}

- {title} } } diff --git a/packages/yew/src/dom_bundle/mod.rs b/packages/yew/src/dom_bundle/mod.rs index 63395ef02f5..16df0b97db3 100644 --- a/packages/yew/src/dom_bundle/mod.rs +++ b/packages/yew/src/dom_bundle/mod.rs @@ -50,7 +50,7 @@ pub(crate) struct Bundle(BNode); impl Bundle { /// Creates a new bundle. - pub fn new() -> Self { + pub const fn new() -> Self { Self(BNode::List(BList::new())) } diff --git a/packages/yew/src/dom_bundle/subtree_root.rs b/packages/yew/src/dom_bundle/subtree_root.rs index c0329a72dbe..34b8e007bbd 100644 --- a/packages/yew/src/dom_bundle/subtree_root.rs +++ b/packages/yew/src/dom_bundle/subtree_root.rs @@ -154,7 +154,6 @@ impl HostHandlers { /// Per subtree data #[derive(Debug)] - struct SubtreeData { /// Data shared between all trees in an app app_data: Rc>, @@ -246,8 +245,9 @@ struct BrandingSearchResult { /// Subtree roots are always branded with their own subtree id. fn find_closest_branded_element(mut el: Element, do_bubble: bool) -> Option { if !do_bubble { + let branding = el.subtree_id()?; Some(BrandingSearchResult { - branding: el.subtree_id()?, + branding, closest_branded_ancestor: el, }) } else { @@ -266,53 +266,20 @@ fn find_closest_branded_element(mut el: Element, do_bubble: bool) -> Option { - event: &'tree Event, - subtree: &'tree Rc, - next_el: Option, +fn start_bubbling_from( + subtree: &SubtreeData, + root_or_listener: Element, should_bubble: bool, -} +) -> impl '_ + Iterator { + let start = subtree.bubble_to_inner_element(root_or_listener, should_bubble); -impl<'tree> Iterator for BubblingIterator<'tree> { - type Item = (&'tree Rc, Element); - - fn next(&mut self) -> Option { - let candidate = self.next_el.take()?; - let candidate_parent = self.subtree; - if self.event.cancel_bubble() { + std::iter::successors(start, move |(subtree, element)| { + if !should_bubble { return None; } - if self.should_bubble { - if let Some((next_subtree, parent)) = candidate - .parent_element() - .and_then(|parent| self.subtree.bubble_to_inner_element(parent, true)) - { - self.subtree = next_subtree; - self.next_el = Some(parent); - } - } - Some((candidate_parent, candidate)) - } -} - -impl<'tree> BubblingIterator<'tree> { - fn start_from( - subtree: &'tree Rc, - root_or_listener: Element, - event: &'tree Event, - should_bubble: bool, - ) -> Self { - let start = match subtree.bubble_to_inner_element(root_or_listener, should_bubble) { - Some((subtree, next_el)) => (subtree, Some(next_el)), - None => (subtree, None), - }; - Self { - event, - subtree: start.0, - next_el: start.1, - should_bubble, - } - } + let parent = element.parent_element()?; + subtree.bubble_to_inner_element(parent, true) + }) } impl SubtreeData { @@ -346,12 +313,11 @@ impl SubtreeData { } // Bubble a potential parent until it reaches an internal element - #[allow(clippy::needless_lifetimes)] // I don't see a way to omit the lifetimes here - fn bubble_to_inner_element<'s>( - self: &'s Rc, + fn bubble_to_inner_element( + &self, parent_el: Element, should_bubble: bool, - ) -> Option<(&'s Rc, Element)> { + ) -> Option<(&Self, Element)> { let mut next_subtree = self; let mut next_el = parent_el; if !should_bubble && next_subtree.host.eq(&next_el) { @@ -366,11 +332,10 @@ impl SubtreeData { Some((next_subtree, next_el)) } - #[allow(clippy::needless_lifetimes)] // I don't see a way to omit the lifetimes here fn start_bubbling_if_responsible<'s>( - self: &'s Rc, + &'s self, event: &'s Event, - ) -> Option> { + ) -> Option> { // Note: the event is not necessarily indentically the same object for all installed handlers // hence this cache can be unreliable. Hence the cached repsonsible_tree_id might be missing. // On the other hand, due to event retargeting at shadow roots, the cache might be wrong! @@ -421,12 +386,7 @@ impl SubtreeData { // One more special case: don't handle events that get fired directly on a subtree host return None; } - Some(BubblingIterator::start_from( - self, - bubbling_start, - event, - should_bubble, - )) + Some(start_bubbling_from(self, bubbling_start, should_bubble)) // # More details: When nesting occurs // // Event listeners are installed only on the subtree roots. Still, those roots can @@ -447,8 +407,8 @@ impl SubtreeData { // } /// Handle a global event firing - fn handle(self: &Rc, desc: EventDescriptor, event: Event) { - let run_handler = |root: &Rc, el: &Element| { + fn handle(&self, desc: EventDescriptor, event: Event) { + let run_handler = |root: &Self, el: &Element| { let handler = Registry::get_handler(root.event_registry(), el, &desc); if let Some(handler) = handler { handler(&event) @@ -457,6 +417,9 @@ impl SubtreeData { if let Some(bubbling_it) = self.start_bubbling_if_responsible(&event) { test_log!("Running handler on subtree {}", self.subtree_id); for (subtree, el) in bubbling_it { + if event.cancel_bubble() { + break; + } run_handler(subtree, &el); } }