Use LayoutManager for details rows (#18821, #18619)

Change-Id: I430e55db8a3e2860f68f5351e06d8d069a657d6e

4 files changed, 106 insertions, 11 deletions

diff --git a/client/src/com/vaadin/client/LayoutManagerIE8.java b/client/src/com/vaadin/client/LayoutManagerIE8.java
index 4464c3bee8..479155d0e6 100644
--- a/client/src/com/vaadin/client/LayoutManagerIE8.java
+++ b/client/src/com/vaadin/client/LayoutManagerIE8.java
@@ -53,6 +53,10 @@ public class LayoutManagerIE8 extends LayoutManager {
         } else {
             measuredSizes.remove(element);
         }
+        // clear any values that are saved to the element
+        if (super.getMeasuredSize(element, null) != null) {
+            super.setMeasuredSize(element, null);
+        }
     }
 
     @Override
@@ -62,6 +66,13 @@ public class LayoutManagerIE8 extends LayoutManager {
         if (measured != null) {
             return measured;
         } else {
+            // check if saved to the element instead
+            MeasuredSize measuredSize = super.getMeasuredSize(element, null);
+            if (measuredSize != null) {
+                // move the value back to the map
+                setMeasuredSize(element, measuredSize);
+                return measuredSize;
+            }
             return defaultSize;
         }
     }
@@ -72,13 +83,17 @@ public class LayoutManagerIE8 extends LayoutManager {
 
         // #12688: IE8 was leaking memory when adding&removing components.
         // Uses IE specific logic to figure if an element has been removed from
-        // DOM or not. For removed elements the measured size is discarded.
+        // DOM or not. For removed elements the measured size is stored within
+        // the element in case the element gets re-attached.
         Node rootNode = Document.get().getBody();
 
         Iterator<Element> i = measuredSizes.keySet().iterator();
         while (i.hasNext()) {
             Element e = i.next();
             if (!rootNode.isOrHasChild(e)) {
+                // Store in element in case is still needed.
+                // Not attached, so reflow isn't a problem.
+                super.setMeasuredSize(e, measuredSizes.get(e));
                 i.remove();
             }
         }
diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java
index 053487620a..c6075d3bd4 100644
--- a/client/src/com/vaadin/client/connectors/GridConnector.java
+++ b/client/src/com/vaadin/client/connectors/GridConnector.java
@@ -54,11 +54,11 @@ import com.vaadin.client.ui.ConnectorFocusAndBlurHandler;
 import com.vaadin.client.ui.SimpleManagedLayout;
 import com.vaadin.client.widget.grid.CellReference;
 import com.vaadin.client.widget.grid.CellStyleGenerator;
-import com.vaadin.client.widget.grid.DetailsGenerator;
 import com.vaadin.client.widget.grid.EditorHandler;
 import com.vaadin.client.widget.grid.EventCellReference;
 import com.vaadin.client.widget.grid.RowReference;
 import com.vaadin.client.widget.grid.RowStyleGenerator;
+import com.vaadin.client.widget.grid.HeightAwareDetailsGenerator;
 import com.vaadin.client.widget.grid.events.BodyClickHandler;
 import com.vaadin.client.widget.grid.events.BodyDoubleClickHandler;
 import com.vaadin.client.widget.grid.events.ColumnReorderEvent;
@@ -489,13 +489,45 @@ public class GridConnector extends AbstractHasComponentsConnector implements
         }
     };
 
-    private class CustomDetailsGenerator implements DetailsGenerator {
+    private class CustomDetailsGenerator implements HeightAwareDetailsGenerator {
 
         private final Map<String, ComponentConnector> idToDetailsMap = new HashMap<String, ComponentConnector>();
         private final Map<String, Integer> idToRowIndex = new HashMap<String, Integer>();
 
         @Override
         public Widget getDetails(int rowIndex) {
+            String id = getId(rowIndex);
+            if (id == null) {
+                return null;
+            }
+            ComponentConnector componentConnector = idToDetailsMap.get(id);
+            idToRowIndex.put(id, rowIndex);
+
+            return componentConnector.getWidget();
+        }
+
+        @Override
+        public double getDetailsHeight(int rowIndex) {
+            String id = getId(rowIndex);
+            ComponentConnector componentConnector = idToDetailsMap.get(id);
+
+            getLayoutManager().setNeedsMeasureRecursively(componentConnector);
+            getLayoutManager().layoutNow();
+
+            return getLayoutManager().getOuterHeightDouble(
+                    componentConnector.getWidget().getElement());
+        }
+
+        /**
+         * Fetches id from the row object that corresponds with the given
+         * rowIndex.
+         * 
+         * @since
+         * @param rowIndex
+         *            the index of the row for which to fetch the id
+         * @return id of the row if such id exists, {@code null} otherwise
+         */
+        private String getId(int rowIndex) {
             JsonObject row = getWidget().getDataSource().getRow(rowIndex);
 
             if (!row.hasKey(GridState.JSONKEY_DETAILS_VISIBLE)
@@ -504,11 +536,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements
                 return null;
             }
 
-            String id = row.getString(GridState.JSONKEY_DETAILS_VISIBLE);
-            ComponentConnector componentConnector = idToDetailsMap.get(id);
-            idToRowIndex.put(id, rowIndex);
-
-            return componentConnector.getWidget();
+            return row.getString(GridState.JSONKEY_DETAILS_VISIBLE);
         }
 
         public void updateConnectorHierarchy(List<ServerConnector> children) {
diff --git a/client/src/com/vaadin/client/widget/grid/HeightAwareDetailsGenerator.java b/client/src/com/vaadin/client/widget/grid/HeightAwareDetailsGenerator.java
new file mode 100644
index 0000000000..fb7dd2c8ea
--- /dev/null
+++ b/client/src/com/vaadin/client/widget/grid/HeightAwareDetailsGenerator.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2000-2014 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.client.widget.grid;
+
+/**
+ * {@link DetailsGenerator} that is aware of content heights.
+ * <p>
+ * <b>FOR INTERNAL USE ONLY!</b> This class exists only for the sake of a
+ * temporary workaround and might be removed or renamed at any time.
+ * </p>
+ * 
+ * @since
+ * @author Vaadin Ltd
+ */
+@Deprecated
+public interface HeightAwareDetailsGenerator extends DetailsGenerator {
+
+    /**
+     * This method is called for whenever a details row's height needs to be
+     * calculated.
+     * <p>
+     * <b>FOR INTERNAL USE ONLY!</b> This method exists only for the sake of a
+     * temporary workaround and might be removed or renamed at any time.
+     * </p>
+     * 
+     * @since
+     * @param rowIndex
+     *            the index of the row for which to calculate details row height
+     * @return height of the details row
+     */
+    public double getDetailsHeight(int rowIndex);
+}
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index 05b90e204d..e3c51830fe 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -119,6 +119,7 @@ import com.vaadin.client.widget.grid.EventCellReference;
 import com.vaadin.client.widget.grid.RendererCellReference;
 import com.vaadin.client.widget.grid.RowReference;
 import com.vaadin.client.widget.grid.RowStyleGenerator;
+import com.vaadin.client.widget.grid.HeightAwareDetailsGenerator;
 import com.vaadin.client.widget.grid.events.AbstractGridKeyEventHandler;
 import com.vaadin.client.widget.grid.events.AbstractGridMouseEventHandler;
 import com.vaadin.client.widget.grid.events.BodyClickHandler;
@@ -3530,11 +3531,17 @@ public class Grid<T> extends ResizeComposite implements
                  * height, but the spacer cell (td) has the borders, which
                  * should go on top of the previous row and next row.
                  */
-                double requiredHeightBoundingClientRectDouble = WidgetUtil
-                        .getRequiredHeightBoundingClientRectDouble(element);
+                double contentHeight;
+                if (detailsGenerator instanceof HeightAwareDetailsGenerator) {
+                    HeightAwareDetailsGenerator sadg = (HeightAwareDetailsGenerator) detailsGenerator;
+                    contentHeight = sadg.getDetailsHeight(rowIndex);
+                } else {
+                    contentHeight = WidgetUtil
+                            .getRequiredHeightBoundingClientRectDouble(element);
+                }
                 double borderTopAndBottomHeight = WidgetUtil
                         .getBorderTopAndBottomThickness(spacerElement);
-                double measuredHeight = requiredHeightBoundingClientRectDouble
+                double measuredHeight = contentHeight
                         + borderTopAndBottomHeight;
                 assert getElement().isOrHasChild(spacerElement) : "The spacer element wasn't in the DOM during measurement, but was assumed to be.";
                 spacerHeight = measuredHeight;