Maintain Table scroll position when changing expand (#10489, #10106)

Change-Id: I4937d8943eab65cd4da2ec2e01af5935f96e244d

4 files changed, 148 insertions, 0 deletions

diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java
index 021ab8fac1..9ac9532ae4 100644
--- a/client/src/com/vaadin/client/ui/VScrollTable.java
+++ b/client/src/com/vaadin/client/ui/VScrollTable.java
@@ -268,6 +268,12 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
     private double lastKnownRowHeight = Double.NaN;
 
     /**
+     * Remember scroll position when getting detached to properly scroll back to
+     * the location that there is data for if getting attached again.
+     */
+    private int detachedScrollPosition = 0;
+
+    /**
      * Represents a select range of rows
      */
     private class SelectionRange {
@@ -1732,6 +1738,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
 
     @Override
     protected void onDetach() {
+        detachedScrollPosition = scrollBodyPanel.getScrollPosition();
         rowRequestHandler.cancel();
         super.onDetach();
         // ensure that scrollPosElement will be detached
@@ -1743,6 +1750,12 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
         }
     }
 
+    @Override
+    public void onAttach() {
+        super.onAttach();
+        scrollBodyPanel.setScrollPosition(detachedScrollPosition);
+    }
+
     /**
      * Run only once when component is attached and received its initial
      * content. This function:
@@ -7149,4 +7162,5 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
     public Widget getWidgetForPaintable() {
         return this;
     }
+
 }
diff --git a/client/src/com/vaadin/client/ui/orderedlayout/VAbstractOrderedLayout.java b/client/src/com/vaadin/client/ui/orderedlayout/VAbstractOrderedLayout.java
index 84f946dec4..c9f5b92c90 100644
--- a/client/src/com/vaadin/client/ui/orderedlayout/VAbstractOrderedLayout.java
+++ b/client/src/com/vaadin/client/ui/orderedlayout/VAbstractOrderedLayout.java
@@ -346,11 +346,22 @@ public class VAbstractOrderedLayout extends FlowPanel {
         if (expandWrapper == null) {
             expandWrapper = DOM.createDiv();
             expandWrapper.setClassName("v-expand");
+
+            // Detach all widgets before modifying DOM
+            for (Widget widget : getChildren()) {
+                orphan(widget);
+            }
+
             while (getElement().getChildCount() > 0) {
                 Node el = getElement().getChild(0);
                 expandWrapper.appendChild(el);
             }
             getElement().appendChild(expandWrapper);
+
+            // Attach all widgets again
+            for (Widget widget : getChildren()) {
+                adopt(widget);
+            }
         }
 
         // Sum up expand ratios to get the denominator
@@ -395,6 +406,11 @@ public class VAbstractOrderedLayout extends FlowPanel {
      */
     public void clearExpand() {
         if (expandWrapper != null) {
+            // Detach all widgets before modifying DOM
+            for (Widget widget : getChildren()) {
+                orphan(widget);
+            }
+
             lastExpandSize = -1;
             while (expandWrapper.getChildCount() > 0) {
                 Element el = expandWrapper.getChild(0).cast();
@@ -409,6 +425,11 @@ public class VAbstractOrderedLayout extends FlowPanel {
             }
             expandWrapper.removeFromParent();
             expandWrapper = null;
+
+            // Attach children again
+            for (Widget widget : getChildren()) {
+                adopt(widget);
+            }
         }
     }
 
diff --git a/uitest/src/com/vaadin/tests/components/orderedlayout/ExpandChangeReattach.html b/uitest/src/com/vaadin/tests/components/orderedlayout/ExpandChangeReattach.html
new file mode 100644
index 0000000000..8fbd3d215b
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/orderedlayout/ExpandChangeReattach.html
@@ -0,0 +1,52 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<head profile="http://selenium-ide.openqa.org/profiles/test-case">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
+<link rel="selenium.base" href="" />
+<title>New Test</title>
+</head>
+<body>
+<table cellpadding="1" cellspacing="1" border="1">
+<thead>
+<tr><td rowspan="1" colspan="3">New Test</td></tr>
+</thead><tbody>
+<tr>
+	<td>open</td>
+	<td>/run/com.vaadin.tests.components.orderedlayout.ExpandChangeReattach?restartApplication</td>
+	<td></td>
+</tr>
+<tr>
+	<td>scroll</td>
+	<td>vaadin=runcomvaadintestscomponentsorderedlayoutExpandChangeReattach::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[0]/VVerticalLayout[0]/Slot[0]/VScrollTable[0]/domChild[1]</td>
+	<td>10000</td>
+</tr>
+<tr>
+	<td>pause</td>
+	<td>500</td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentsorderedlayoutExpandChangeReattach::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[0]/VVerticalLayout[0]/Slot[1]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>screenCapture</td>
+	<td></td>
+	<td>expandEnabled</td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentsorderedlayoutExpandChangeReattach::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[0]/VVerticalLayout[0]/Slot[1]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>screenCapture</td>
+	<td></td>
+	<td>expandDisabled</td>
+</tr>
+
+</tbody></table>
+</body>
+</html>
diff --git a/uitest/src/com/vaadin/tests/components/orderedlayout/ExpandChangeReattach.java b/uitest/src/com/vaadin/tests/components/orderedlayout/ExpandChangeReattach.java
new file mode 100644
index 0000000000..4360ff60d8
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/orderedlayout/ExpandChangeReattach.java
@@ -0,0 +1,61 @@
+/*
+ * Copyright 2012 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.components.orderedlayout;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.tests.util.TestUtils;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Table;
+import com.vaadin.ui.VerticalLayout;
+
+public class ExpandChangeReattach extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        final VerticalLayout verticalLayout = new VerticalLayout();
+        verticalLayout.setHeight(null);
+
+        Table table = new Table("Table", TestUtils.getISO3166Container());
+        verticalLayout.addComponent(table);
+        verticalLayout.addComponent(new Button("Toggle expand logic",
+                new Button.ClickListener() {
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        if (verticalLayout.getHeight() == -1) {
+                            verticalLayout.setHeight("900px");
+                        } else {
+                            verticalLayout.setHeight(null);
+                        }
+                    }
+                }));
+
+        addComponent(verticalLayout);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Table should not forget its scroll position when it is temporarily detached from the DOM because an ordered layout changes expand modes.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return Integer.valueOf(10489);
+    }
+
+}