Fix Grid details height calculation issue (#10343)

7 files changed, 204 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 cf4addc37e..e91ab9172c 100644
--- a/client/src/main/java/com/vaadin/client/WidgetUtil.java
+++ b/client/src/main/java/com/vaadin/client/WidgetUtil.java
@@ -1867,6 +1867,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
+     */
+    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 8.2
diff --git a/client/src/main/java/com/vaadin/client/connectors/grid/DetailsManagerConnector.java b/client/src/main/java/com/vaadin/client/connectors/grid/DetailsManagerConnector.java
index 177056c0e3..b9792ad79d 100644
--- a/client/src/main/java/com/vaadin/client/connectors/grid/DetailsManagerConnector.java
+++ b/client/src/main/java/com/vaadin/client/connectors/grid/DetailsManagerConnector.java
@@ -21,6 +21,7 @@ import java.util.Map;
 import com.google.gwt.core.client.Scheduler;
 import com.google.gwt.core.client.Scheduler.ScheduledCommand;
 import com.google.gwt.dom.client.Element;
+import com.google.gwt.event.shared.HandlerRegistration;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.ComponentConnector;
 import com.vaadin.client.ConnectorMap;
@@ -56,6 +57,11 @@ public class DetailsManagerConnector extends AbstractExtensionConnector {
     /* Registration for data change handler. */
     private Registration dataChangeRegistration;
 
+    /**
+     * Handle for the spacer visibility change handler.
+     */
+    private HandlerRegistration spacerVisibilityChangeRegistration;
+
     private final Map<Element, ScheduledCommand> elementToResizeCommand = new HashMap<Element, Scheduler.ScheduledCommand>();
     private final ElementResizeListener detailsRowResizeListener = event -> {
         if (elementToResizeCommand.containsKey(event.getElement())) {
@@ -140,9 +146,12 @@ public class DetailsManagerConnector extends AbstractExtensionConnector {
                 if (spacerCellBorderHeights != null
                         && !getLayoutManager().isLayoutRunning()
                         && getDetailsComponentConnectorId(rowIndex) != null) {
-                    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);
+                    }
                 }
             };
         }
@@ -180,6 +189,19 @@ public class DetailsManagerConnector extends AbstractExtensionConnector {
         getWidget().setDetailsGenerator(new CustomDetailsGenerator());
         dataChangeRegistration = getWidget().getDataSource()
                 .addDataChangeHandler(new DetailsChangeHandler());
+
+        // When details element is shown, remeasure it in the layout phase
+        spacerVisibilityChangeRegistration = getParent().getWidget()
+                .addSpacerVisibilityChangedHandler(event -> {
+                    if (event.isSpacerVisible()) {
+                        String id = indexToDetailConnectorId
+                                .get(event.getRowIndex());
+                        ComponentConnector connector = (ComponentConnector) ConnectorMap
+                                .get(getConnection()).getConnector(id);
+                        getLayoutManager()
+                                .setNeedsMeasureRecursively(connector);
+                    }
+                });
     }
 
     private void detachIfNeeded(int rowIndex, String id) {
@@ -215,6 +237,8 @@ public class DetailsManagerConnector extends AbstractExtensionConnector {
         dataChangeRegistration.remove();
         dataChangeRegistration = null;
 
+        spacerVisibilityChangeRegistration.removeHandler();
+
         indexToDetailConnectorId.clear();
     }
 
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..116d9a31ac
--- /dev/null
+++ b/client/src/main/java/com/vaadin/client/widget/escalator/events/SpacerVisibilityChangedEvent.java
@@ -0,0 +1,83 @@
+/*
+ * 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.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
+ */
+public class SpacerVisibilityChangedEvent extends GwtEvent<SpacerVisibilityChangedHandler> {
+
+    /**
+     * Handler type.
+     */
+    public static final Type<SpacerVisibilityChangedHandler> TYPE = new Type<>();
+
+    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 isSpacerVisible() {
+        return visible;
+    }
+
+    @Override
+    public Type<SpacerVisibilityChangedHandler> getAssociatedType() {
+        return TYPE;
+    }
+
+    @Override
+    protected void dispatch(SpacerVisibilityChangedHandler handler) {
+        handler.onSpacerVisibilityChanged(this);
+    }
+
+}
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..fe7b02af55
--- /dev/null
+++ b/client/src/main/java/com/vaadin/client/widget/escalator/events/SpacerVisibilityChangedHandler.java
@@ -0,0 +1,36 @@
+/*
+ * 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.client.widget.escalator.events;
+
+import com.google.gwt.event.shared.EventHandler;
+
+/**
+ * Event handler for a spacer visibility changed event.
+ *
+ * @author Vaadin Ltd
+ * @since
+ */
+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);
+}
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 b3008d28dc..da488e1153 100644
--- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java
@@ -64,6 +64,7 @@ import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.BrowserInfo;
 import com.vaadin.client.ComputedStyle;
 import com.vaadin.client.DeferredWorker;
+import com.vaadin.client.LayoutManager;
 import com.vaadin.client.Profiler;
 import com.vaadin.client.WidgetUtil;
 import com.vaadin.client.ui.SubPartAware;
@@ -87,6 +88,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;
@@ -4962,11 +4964,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 a790ccb314..982e6d2ecb 100755
--- a/client/src/main/java/com/vaadin/client/widgets/Grid.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java
@@ -103,6 +103,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;
@@ -8559,6 +8561,19 @@ 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
+     * @since
+     */
+    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
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/basics/GridBasicDetailsTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/basics/GridBasicDetailsTest.java
index 3cc1906dcc..68542268d5 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/basics/GridBasicDetailsTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/basics/GridBasicDetailsTest.java
@@ -319,4 +319,21 @@ public class GridBasicDetailsTest extends GridBasicsTest {
                 getGridElement().getDetails(0).getText().contains("One"));
     }
 
+    @Test
+    public void detailsSizeCorrectAfterScrolling() {
+        selectMenuPath(DETAILS_GENERATOR_PERSISTING);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+
+        // Scroll to request next range
+        getGridElement().scrollToRow(21);
+        getGridElement().scrollToRow(0);
+        assertGreater("Details row should have correct height",
+                getGridElement().getDetails(0).getSize().getHeight(), 30);
+
+        // Scroll outside of cached rows
+        getGridElement().scrollToRow(101);
+        getGridElement().scrollToRow(0);
+        assertGreater("Details row should have correct height",
+                getGridElement().getDetails(0).getSize().getHeight(), 30);
+    }
 }