Fix grid details height calculation issue (#10453)

6 files changed, 188 insertions, 3 deletions

diff --git a/client/src/main/java/com/vaadin/client/WidgetUtil.java b/client/src/main/java/com/vaadin/client/WidgetUtil.java
index 706cc6fb95..53d9ee8f17 100644
--- a/client/src/main/java/com/vaadin/client/WidgetUtil.java
+++ b/client/src/main/java/com/vaadin/client/WidgetUtil.java
@@ -1882,6 +1882,26 @@ public class WidgetUtil {
     }
 
     /**
+     * Returns whether the given element is displayed.
+     * <p>
+     * This method returns false if either the given element or any of its
+     * ancestors has the style {@code display: none} applied.
+     *
+     * @param element
+     *         the element to test for visibility
+     * @return {@code true} if the element is displayed, {@code false} otherwise
+     * @since 7.7.13
+     */
+    public static native boolean isDisplayed(Element element)
+    /*-{
+        // This measurement is borrowed from JQuery and measures the visible
+        // size of the element. The measurement should return false when either
+        // the element or any of its ancestors has "display: none" style.
+        return !!(element.offsetWidth || element.offsetHeight
+            || element.getClientRects().length);
+    }-*/;
+
+    /**
      * Utility methods for displaying error message on components.
      *
      * @since 7.7.11
diff --git a/client/src/main/java/com/vaadin/client/connectors/GridConnector.java b/client/src/main/java/com/vaadin/client/connectors/GridConnector.java
index 9cbafdbe21..2805f8eb7b 100644
--- a/client/src/main/java/com/vaadin/client/connectors/GridConnector.java
+++ b/client/src/main/java/com/vaadin/client/connectors/GridConnector.java
@@ -56,6 +56,8 @@ import com.vaadin.client.ui.layout.ElementResizeEvent;
 import com.vaadin.client.ui.layout.ElementResizeListener;
 import com.vaadin.client.widget.escalator.events.RowHeightChangedEvent;
 import com.vaadin.client.widget.escalator.events.RowHeightChangedHandler;
+import com.vaadin.client.widget.escalator.events.SpacerVisibilityChangedEvent;
+import com.vaadin.client.widget.escalator.events.SpacerVisibilityChangedHandler;
 import com.vaadin.client.widget.grid.CellReference;
 import com.vaadin.client.widget.grid.CellStyleGenerator;
 import com.vaadin.client.widget.grid.EditorHandler;
@@ -496,6 +498,8 @@ public class GridConnector extends AbstractHasComponentsConnector
     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>();
         private final Map<Element, ScheduledCommand> elementToResizeCommand = new HashMap<Element, Scheduler.ScheduledCommand>();
@@ -544,9 +548,13 @@ public class GridConnector extends AbstractHasComponentsConnector
                     if (spacerCellBorderHeights != null
                             && !getLayoutManager().isLayoutRunning()
                             && hasDetailsOpen(rowIndex)) {
-                        double height = getLayoutManager().getOuterHeightDouble(
-                                element) + spacerCellBorderHeights;
-                        getWidget().setDetailsHeight(rowIndex, height);
+                        // Measure and set details height if element is visible
+                        if (WidgetUtil.isDisplayed(element)) {
+                            double height =
+                                    getLayoutManager().getOuterHeightDouble(
+                                            element) + spacerCellBorderHeights;
+                            getWidget().setDetailsHeight(rowIndex, height);
+                        }
                     }
                 }
             };
@@ -874,6 +882,24 @@ public class GridConnector extends AbstractHasComponentsConnector
             }
         });
 
+        // When details element is shown, remeasure it in the layout phase.
+        // This is necessary because details might've been laid out incorrectly
+        // while they were out of view.
+        getWidget().addSpacerVisibilityChangedHandler(
+                new SpacerVisibilityChangedHandler() {
+                    @Override
+                    public void onSpacerVisibilityChanged(
+                            SpacerVisibilityChangedEvent event) {
+                        if (event.isVisible()) {
+                            ComponentConnector connector = customDetailsGenerator.idToDetailsMap
+                                    .get(customDetailsGenerator
+                                            .getId(event.getRowIndex()));
+                            getLayoutManager()
+                                    .setNeedsMeasureRecursively(connector);
+                        }
+                    }
+                });
+
         layout();
     }
 
diff --git a/client/src/main/java/com/vaadin/client/widget/escalator/events/SpacerVisibilityChangedEvent.java b/client/src/main/java/com/vaadin/client/widget/escalator/events/SpacerVisibilityChangedEvent.java
new file mode 100644
index 0000000000..d635ff7893
--- /dev/null
+++ b/client/src/main/java/com/vaadin/client/widget/escalator/events/SpacerVisibilityChangedEvent.java
@@ -0,0 +1,84 @@
+/*
+ * 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.escalator.events;
+
+import com.google.gwt.event.shared.GwtEvent;
+
+/**
+ * Event fired when a spacer element is hidden or shown in Escalator.
+ *
+ * @author Vaadin Ltd
+ * @since 7.7.13
+ */
+public class SpacerVisibilityChangedEvent
+        extends GwtEvent<SpacerVisibilityChangedHandler> {
+
+    /**
+     * Handler type.
+     */
+    public static final Type<SpacerVisibilityChangedHandler> TYPE = new Type<SpacerVisibilityChangedHandler>();
+
+    public static final Type<SpacerVisibilityChangedHandler> getType() {
+        return TYPE;
+    }
+
+    private final int rowIndex;
+    private final boolean visible;
+
+    /**
+     * Creates a spacer visibility changed event.
+     *
+     * @param rowIndex
+     *         index of row to which the spacer belongs
+     * @param visible
+     *         {@code true} if the spacer element is shown, {@code false} if the
+     *         spacer element is hidden
+     */
+    public SpacerVisibilityChangedEvent(int rowIndex, boolean visible) {
+        this.rowIndex = rowIndex;
+        this.visible = visible;
+    }
+
+    /**
+     * Gets the row index to which the spacer element belongs.
+     *
+     * @return the row index to which the spacer element belongs
+     */
+    public int getRowIndex() {
+        return rowIndex;
+    }
+
+    /**
+     * Gets whether the spacer element is displayed.
+     *
+     * @return {@code true} if the spacer element is shown, {@code false} if the
+     * spacer element is hidden
+     */
+    public boolean isVisible() {
+        return visible;
+    }
+
+    @Override
+    public Type<SpacerVisibilityChangedHandler> getAssociatedType() {
+        return TYPE;
+    }
+
+    @Override
+    protected void dispatch(SpacerVisibilityChangedHandler handler) {
+        handler.onSpacerVisibilityChanged(this);
+    }
+
+}
\ No newline at end of file
diff --git a/client/src/main/java/com/vaadin/client/widget/escalator/events/SpacerVisibilityChangedHandler.java b/client/src/main/java/com/vaadin/client/widget/escalator/events/SpacerVisibilityChangedHandler.java
new file mode 100644
index 0000000000..5d058b6442
--- /dev/null
+++ b/client/src/main/java/com/vaadin/client/widget/escalator/events/SpacerVisibilityChangedHandler.java
@@ -0,0 +1,36 @@
+/*
+ * 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.escalator.events;
+
+import com.google.gwt.event.shared.EventHandler;
+
+/**
+ * Event handler for a spacer visibility changed event.
+ *
+ * @author Vaadin Ltd
+ * @since 7.7.13
+ */
+public interface SpacerVisibilityChangedHandler extends EventHandler {
+
+    /**
+     * Called when a spacer visibility changed event is fired, when a spacer's
+     * visibility changes.
+     *
+     * @param event
+     *            the spacer visibility changed event
+     */
+    public void onSpacerVisibilityChanged(SpacerVisibilityChangedEvent event);
+}
\ No newline at end of file
diff --git a/client/src/main/java/com/vaadin/client/widgets/Escalator.java b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
index 7b4add7661..d764837c2d 100644
--- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
@@ -85,6 +85,7 @@ import com.vaadin.client.widget.escalator.ScrollbarBundle.VerticalScrollbarBundl
 import com.vaadin.client.widget.escalator.Spacer;
 import com.vaadin.client.widget.escalator.SpacerUpdater;
 import com.vaadin.client.widget.escalator.events.RowHeightChangedEvent;
+import com.vaadin.client.widget.escalator.events.SpacerVisibilityChangedEvent;
 import com.vaadin.client.widget.grid.events.ScrollEvent;
 import com.vaadin.client.widget.grid.events.ScrollHandler;
 import com.vaadin.client.widgets.Escalator.JsniUtil.TouchHandlerBundle;
@@ -4820,11 +4821,15 @@ public class Escalator extends Widget
             public void show() {
                 getRootElement().getStyle().clearDisplay();
                 getDecoElement().getStyle().clearDisplay();
+                Escalator.this.fireEvent(
+                        new SpacerVisibilityChangedEvent(getRow(), true));
             }
 
             public void hide() {
                 getRootElement().getStyle().setDisplay(Display.NONE);
                 getDecoElement().getStyle().setDisplay(Display.NONE);
+                Escalator.this.fireEvent(
+                        new SpacerVisibilityChangedEvent(getRow(), false));
             }
 
             /**
diff --git a/client/src/main/java/com/vaadin/client/widgets/Grid.java b/client/src/main/java/com/vaadin/client/widgets/Grid.java
index be4d3da587..a0c7f353e9 100755
--- a/client/src/main/java/com/vaadin/client/widgets/Grid.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java
@@ -109,6 +109,8 @@ import com.vaadin.client.widget.escalator.Spacer;
 import com.vaadin.client.widget.escalator.SpacerUpdater;
 import com.vaadin.client.widget.escalator.events.RowHeightChangedEvent;
 import com.vaadin.client.widget.escalator.events.RowHeightChangedHandler;
+import com.vaadin.client.widget.escalator.events.SpacerVisibilityChangedEvent;
+import com.vaadin.client.widget.escalator.events.SpacerVisibilityChangedHandler;
 import com.vaadin.client.widget.grid.AutoScroller;
 import com.vaadin.client.widget.grid.AutoScroller.AutoScrollerCallback;
 import com.vaadin.client.widget.grid.AutoScroller.ScrollAxis;
@@ -8464,6 +8466,18 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>,
     }
 
     /**
+     * Adds a spacer visibility changed handler to the underlying escalator.
+     *
+     * @param handler
+     *         the handler to be called when a spacer's visibility changes
+     * @return the registration object with which the handler can be removed
+     */
+    public HandlerRegistration addSpacerVisibilityChangedHandler(
+            SpacerVisibilityChangedHandler handler) {
+        return escalator.addHandler(handler, SpacerVisibilityChangedEvent.TYPE);
+    }
+
+    /**
      * Adds a low-level DOM event handler to this Grid. The handler is inserted
      * into the given position in the list of handlers. The handlers are invoked
      * in order. If the