fix: reset Dom node on changing StateNode parent (#12043) (#12102)

Allows to move children to the grandparent and remove parent at the same time

fixes #11983

Co-authored-by: Denis <[email protected]>

flow-client/src/main/java/com/vaadin/client/flow/StateNode.java

@@ -284,6 +284,10 @@ public StateNode getParent() {
      *            the parent state node
      */
     public void setParent(StateNode parent) {
+        if (this.parent != null && this.parent.isUnregistered()
+                && parent != null && this.parent.getId() != parent.getId()) {
+            setDomNode(null);
+        }
         this.parent = parent;
     }
 

flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java

@@ -202,6 +202,13 @@ assert hasSameTag(stateNode, htmlNode) : "Element tag name is '"
         if (boundNodes.has(stateNode)) {
             return;
         }
+
+        stateNode.addDomNodeSetListener(node -> {
+            if (node.getDomNode() == null) {
+                boundNodes.delete(node);
+            }
+            return true;
+        });
         boundNodes.set(stateNode, true);
 
         BindingContext context = new BindingContext(stateNode, htmlNode,

flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtBasicElementBinderTest.java

@@ -80,11 +80,11 @@ public void testBindExistingProperty() {
     }
 
     public void testBindExistingPropertyWithDifferentType() {
-        //set number as a property value for DOM elememnt
-        double value= setNumberValue(element, "bar");
-        
+        // set number as a property value for DOM elememnt
+        double value = setNumberValue(element, "bar");
+
         // set string as a StateTree property value
-        properties.getProperty("bar").setValue(""+value);
+        properties.getProperty("bar").setValue("" + value);
 
         Binder.bind(node, element);
 
@@ -1821,6 +1821,37 @@ public void testReadyCallback_deferredPolymerElement_readyIsCalledAndNotified()
         assertEquals("foobar", WidgetUtil.getJsProperty(element, "baz"));
     }
 
+    public void testBoundNode_canBeBoundAgainOnceDomNodeIsReset() {
+        // set some attribute
+        idAttribute.setValue("foo");
+
+        Binder.bind(node, element);
+
+        Reactive.flush();
+
+        assertEquals("foo", element.getId());
+
+        Element newElement = Browser.getDocument().createElement("div");
+        Binder.bind(node, newElement);
+
+        Reactive.flush();
+
+        // nothing happens since the node is already bound
+        assertEquals("", newElement.getId());
+
+        // reset node
+        node.setDomNode(null);
+
+        newElement = Browser.getDocument().createElement("div");
+
+        Binder.bind(node, newElement);
+
+        Reactive.flush();
+
+        // once the dom node has been reset the element can bound again
+        assertEquals("foo", newElement.getId());
+    }
+
     private void assertDeferredPolymerElement_originalReadyIsCalled(
             Element element) {
         initPolymer(element);
@@ -1979,7 +2010,7 @@ private native String getPropertyType(Object obj, String name)
     /*-{
        return typeof obj[name];
     }-*/;
-    
+
     private native double setNumberValue(Object obj, String name)
     /*-{
        obj[name] =2;

flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtStateNodeTest.java

@@ -18,6 +18,8 @@
 import com.vaadin.client.ClientEngineTestBase;
 import com.vaadin.client.Registry;
 
+import elemental.client.Browser;
+
 public class GwtStateNodeTest extends ClientEngineTestBase {
 
     private StateTree tree;
@@ -39,4 +41,20 @@ public void testNodeData_getNodeData_sameInstance() {
         assertSame(data, node.getNodeData(TestData.class));
     }
 
+    public void testSetNewParent_domNodeIsNull() {
+        StateNode parent = new StateNode(2, tree);
+        tree.registerNode(parent);
+
+        StateNode node = new StateNode(3, tree);
+
+        node.setDomNode(Browser.getDocument().createElement("div"));
+        node.setParent(parent);
+
+        assertNotNull(node.getDomNode());
+
+        tree.unregisterNode(parent);
+        node.setParent(new StateNode(4, tree));
+        assertNull(node.getDomNode());
+    }
+
 }

flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/RemoveElementAfterItsChildrenView.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright 2000-2021 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.flow.uitest.ui;
+
+import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.component.html.NativeButton;
+import com.vaadin.flow.router.Route;
+
+@Route("com.vaadin.flow.uitest.ui.RemoveElementAfterItsChildrenView")
+public class RemoveElementAfterItsChildrenView extends Div {
+
+    public RemoveElementAfterItsChildrenView() {
+        Div container = new Div();
+        container.setId("container");
+        Div wrapper = new Div();
+        wrapper.setId("wrapper");
+        Div first = new Div();
+        first.setText("First child");
+        first.setId("first");
+        Div second = new Div();
+        second.setText("Second child");
+        second.setId("second");
+        container.add(wrapper);
+        wrapper.add(first, second);
+
+        NativeButton button = new NativeButton();
+        button.setText("Move children and remove parent");
+        button.setId("move-children");
+        add(container, button);
+
+        // Then later on click the button, executing the following code
+        button.addClickListener(event -> {
+            container.add(first, second);
+            wrapper.getElement().removeFromParent();
+        });
+    }
+
+}

flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/RemoveElementAfterItsChildrenIT.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright 2000-2021 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.flow.uitest.ui;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.flow.testutil.ChromeBrowserTest;
+
+public class RemoveElementAfterItsChildrenIT extends ChromeBrowserTest {
+
+    @Test
+    public void moveChildrenToParentContainerAndRemoveParent_childrenAreMoved() {
+        open();
+
+        findElement(By.id("move-children")).click();
+
+        WebElement container = findElement(By.id("container"));
+
+        Assert.assertEquals(1, container.findElements(By.id("first")).size());
+        Assert.assertEquals(1, container.findElements(By.id("second")).size());
+
+        Assert.assertFalse(isElementPresent(By.id("wrapper")));
+    }
+}