From: Henri Sara <henri.sara@gmail.com>
Date: Wed, 23 Aug 2017 10:11:24 +0000 (+0300)
Subject: Avoid detaching CssLayout children unnecessarily (#9861)
X-Git-Tag: 8.1.3~13
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=7a82eb3bfbe970f114ffc6c3d3fa2e25ef3bf99f;p=vaadin-framework.git

Avoid detaching CssLayout children unnecessarily (#9861)

When child components are removed from a CssLayout, do not remove other
children from the DOM if no other hierarchy changes are made.

Fixes #7712
---

diff --git a/client/src/main/java/com/vaadin/client/ui/csslayout/CssLayoutConnector.java b/client/src/main/java/com/vaadin/client/ui/csslayout/CssLayoutConnector.java
index 40e473c8d8..8b90fffeb6 100644
--- a/client/src/main/java/com/vaadin/client/ui/csslayout/CssLayoutConnector.java
+++ b/client/src/main/java/com/vaadin/client/ui/csslayout/CssLayoutConnector.java
@@ -114,19 +114,6 @@ public class CssLayoutConnector extends AbstractLayoutConnector {
     public void onConnectorHierarchyChange(
             ConnectorHierarchyChangeEvent event) {
         Profiler.enter("CssLayoutConnector.onConnectorHierarchyChange");
-        Profiler.enter(
-                "CssLayoutConnector.onConnectorHierarchyChange add children");
-        int index = 0;
-        for (ComponentConnector child : getChildComponents()) {
-            VCaption childCaption = childIdToCaption
-                    .get(child.getConnectorId());
-            if (childCaption != null) {
-                getWidget().addOrMove(childCaption, index++);
-            }
-            getWidget().addOrMove(child.getWidget(), index++);
-        }
-        Profiler.leave(
-                "CssLayoutConnector.onConnectorHierarchyChange add children");
 
         // Detach old child widgets and possibly their caption
         Profiler.enter(
@@ -145,6 +132,21 @@ public class CssLayoutConnector extends AbstractLayoutConnector {
         }
         Profiler.leave(
                 "CssLayoutConnector.onConnectorHierarchyChange remove old children");
+
+        Profiler.enter(
+                "CssLayoutConnector.onConnectorHierarchyChange add children");
+        int index = 0;
+        for (ComponentConnector child : getChildComponents()) {
+            VCaption childCaption = childIdToCaption
+                    .get(child.getConnectorId());
+            if (childCaption != null) {
+                getWidget().addOrMove(childCaption, index++);
+            }
+            getWidget().addOrMove(child.getWidget(), index++);
+        }
+        Profiler.leave(
+                "CssLayoutConnector.onConnectorHierarchyChange add children");
+
         Profiler.leave("CssLayoutConnector.onConnectorHierarchyChange");
     }
 
diff --git a/uitest/src/main/java/com/vaadin/tests/layouts/CssLayoutRemoveComponent.java b/uitest/src/main/java/com/vaadin/tests/layouts/CssLayoutRemoveComponent.java
index d8a9d3d27e..1aa87091ba 100644
--- a/uitest/src/main/java/com/vaadin/tests/layouts/CssLayoutRemoveComponent.java
+++ b/uitest/src/main/java/com/vaadin/tests/layouts/CssLayoutRemoveComponent.java
@@ -2,10 +2,8 @@ package com.vaadin.tests.layouts;
 
 import com.vaadin.tests.components.TestBase;
 import com.vaadin.ui.Button;
-import com.vaadin.ui.Button.ClickEvent;
-import com.vaadin.ui.Button.ClickListener;
 import com.vaadin.ui.CssLayout;
-import com.vaadin.v7.ui.TextField;
+import com.vaadin.ui.TextField;
 
 @SuppressWarnings("serial")
 public class CssLayoutRemoveComponent extends TestBase {
@@ -14,18 +12,15 @@ public class CssLayoutRemoveComponent extends TestBase {
     protected void setup() {
         final CssLayout layout = new CssLayout();
         final TextField tf = new TextField("Caption1");
-        Button b = new Button("Remove field ", new ClickListener() {
-
-            @Override
-            public void buttonClick(ClickEvent event) {
-                layout.removeComponent(tf);
-            }
-
-        });
+        Button b = new Button("Remove field ",
+                event -> layout.removeComponent(tf));
         layout.addComponent(tf);
         layout.addComponent(b);
         layout.addComponent(new TextField("Caption2"));
-        layout.addComponent(new TextField("Caption3"));
+        TextField tf3 = new TextField("Caption3");
+        layout.addComponent(tf3);
+
+        tf3.focus();
 
         addComponent(layout);
     }
diff --git a/uitest/src/test/java/com/vaadin/tests/layouts/CssLayoutRemoveComponentTest.java b/uitest/src/test/java/com/vaadin/tests/layouts/CssLayoutRemoveComponentTest.java
new file mode 100644
index 0000000000..789f15e5e4
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/layouts/CssLayoutRemoveComponentTest.java
@@ -0,0 +1,47 @@
+/*
+ * Copyright 2000-2016 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.tests.layouts;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.TextFieldElement;
+import com.vaadin.tests.tb3.SingleBrowserTestPhantomJS2;
+
+public class CssLayoutRemoveComponentTest extends SingleBrowserTestPhantomJS2 {
+    @Test
+    public void testRemoveOnlyNecessaryComponentsFromDom() {
+        openTestURL();
+
+        String script = "document.mutationEventCount = 0;"
+                + "var observer = new MutationObserver(function(mutations){"
+                + "mutations.forEach(function(mutation) { document.mutationEventCount += mutation.removedNodes.length; });"
+                + "});"
+                + "observer.observe(arguments[0].parentNode, { childList: true });";
+
+        executeScript(script,
+                $(TextFieldElement.class).caption("Caption1").first());
+
+        $(ButtonElement.class).first().click();
+
+        Long mutationEvents = (Long) executeScript(
+                "return document.mutationEventCount;");
+        Assert.assertEquals(
+                "Parent should only have two mutation events (remove field and its caption)",
+                2, mutationEvents.intValue());
+    }
+}