Fix portal shifting on reconciliation too often (#2891)

* fix portal shifting on reconciliation too often

the public vdom api changes to only allow directly
setting a Node as sibling (still optional) instead of a NodeRef.
This was the intention all along, since the NodeRef was
not dynamically tracked, and creating a portal into a subtree
already controlled by yew is not supported anway.

* fix feature soundness

* fix doc tests

packages/yew/src/dom_bundle/bportal.rs

@@ -48,6 +48,11 @@ impl Reconcilable for VPortal {
             inner_sibling,
             node,
         } = self;
+        let inner_sibling = {
+            let sib_ref = NodeRef::default();
+            sib_ref.set(inner_sibling);
+            sib_ref
+        };
         let inner_root = root.create_subroot(parent.clone(), &host);
         let (_, inner) = node.attach(&inner_root, parent_scope, &host, inner_sibling.clone());
         (
@@ -92,9 +97,11 @@ impl Reconcilable for VPortal {
         } = self;
 
         let old_host = std::mem::replace(&mut portal.host, host);
-        let old_inner_sibling = std::mem::replace(&mut portal.inner_sibling, inner_sibling);
 
-        if old_host != portal.host || old_inner_sibling != portal.inner_sibling {
+        let should_shift = old_host != portal.host || portal.inner_sibling.get() != inner_sibling;
+        portal.inner_sibling.set(inner_sibling);
+
+        if should_shift {
             // Remount the inner node somewhere else instead of diffing
             // Move the node, but keep the state
             let inner_sibling = portal.inner_sibling.clone();
@@ -123,12 +130,15 @@ impl BPortal {
 mod layout_tests {
     extern crate self as yew;
 
+    use gloo::utils::document;
     use wasm_bindgen_test::{wasm_bindgen_test as test, wasm_bindgen_test_configure};
+    use web_sys::HtmlInputElement;
     use yew::virtual_dom::VPortal;
 
-    use crate::html;
+    use super::*;
     use crate::tests::layout_tests::{diff_layouts, TestLayout};
     use crate::virtual_dom::VNode;
+    use crate::{create_portal, html};
 
     wasm_bindgen_test_configure!(run_in_browser);
 
@@ -200,4 +210,43 @@ mod layout_tests {
 
         diff_layouts(layouts)
     }
+
+    fn setup_parent_with_portal() -> (BSubtree, AnyScope, Element, Element) {
+        let scope = AnyScope::test();
+        let parent = document().create_element("div").unwrap();
+        let portal_host = document().create_element("div").unwrap();
+        let root = BSubtree::create_root(&parent);
+
+        let body = document().body().unwrap();
+        body.append_child(&parent).unwrap();
+        body.append_child(&portal_host).unwrap();
+
+        (root, scope, parent, portal_host)
+    }
+
+    #[test]
+    fn test_no_shift() {
+        // Portals shouldn't shift (which e.g. causes internal inputs to unfocus) when sibling
+        // doesn't change.
+        let (root, scope, parent, portal_host) = setup_parent_with_portal();
+        let input_ref = NodeRef::default();
+
+        let portal = create_portal(
+            html! { <input type="text" ref={&input_ref} /> },
+            portal_host,
+        );
+        let (_, mut bundle) = portal
+            .clone()
+            .attach(&root, &scope, &parent, NodeRef::default());
+
+        // Focus the input, then reconcile again
+        let input_el = input_ref.cast::<HtmlInputElement>().unwrap();
+        input_el.focus().unwrap();
+
+        let _ = portal.reconcile_node(&root, &scope, &parent, NodeRef::default(), &mut bundle);
+
+        let new_input_el = input_ref.cast::<HtmlInputElement>().unwrap();
+        assert_eq!(input_el, new_input_el);
+        assert_eq!(document().active_element(), Some(new_input_el.into()));
+    }
 }

packages/yew/src/html/mod.rs

@@ -124,13 +124,6 @@ impl NodeRef {
         let node = self.get();
         node.map(Into::into).map(INTO::from)
     }
-
-    /// Place a Node in a reference for later use
-    pub(crate) fn set(&self, node: Option<Node>) {
-        let mut this = self.0.borrow_mut();
-        this.node = node;
-        this.link = None;
-    }
 }
 
 #[cfg(feature = "csr")]
@@ -156,6 +149,13 @@ mod feat_csr {
             node_ref.set(Some(node));
             node_ref
         }
+
+        /// Place a Node in a reference for later use
+        pub(crate) fn set(&self, node: Option<Node>) {
+            let mut this = self.0.borrow_mut();
+            this.node = node;
+            this.link = None;
+        }
     }
 }
 

packages/yew/src/virtual_dom/vportal.rs

@@ -3,14 +3,13 @@
 use web_sys::{Element, Node};
 
 use super::VNode;
-use crate::html::NodeRef;
 
 #[derive(Debug, Clone)]
 pub struct VPortal {
     /// The element under which the content is inserted.
     pub host: Element,
     /// The next sibling after the inserted content. Most be a child of `host`.
-    pub inner_sibling: NodeRef,
+    pub inner_sibling: Option<Node>,
     /// The inserted node
     pub node: Box<VNode>,
 }
@@ -20,7 +19,7 @@ impl VPortal {
     pub fn new(content: VNode, host: Element) -> Self {
         Self {
             host,
-            inner_sibling: NodeRef::default(),
+            inner_sibling: None,
             node: Box::new(content),
         }
     }
@@ -31,11 +30,7 @@ impl VPortal {
     pub fn new_before(content: VNode, host: Element, inner_sibling: Option<Node>) -> Self {
         Self {
             host,
-            inner_sibling: {
+            inner_sibling,
-                let sib_ref = NodeRef::default();
-                sib_ref.set(inner_sibling);
-                sib_ref
-            },
             node: Box::new(content),
         }
     }

website/docs/concepts/html/introduction.mdx

@@ -156,14 +156,18 @@ free to [chime in on this issue](https://github.com/yewstack/yew/issues/1334).
 Attributes are set on elements in the same way as in normal HTML:
 
 ```rust
+use yew::prelude::*;
+
 let value = "something";
-html! { <div attribute={value} />
+html! { <div attribute={value} /> };
 ```
 
 Properties are specified with `~` before the element name:
 
 ```rust
-html! { <my-element ~property="abc" /> }
+use yew::prelude::*;
+
+html! { <my-element ~property="abc" /> };
 ```
 
 :::tip