Details row decorator and border positioning and sizes (#17423)

IE8 still isn't pixel perfect, but you can't have 'em all.

Change-Id: I1780441f130032503d783657103066f502dce570

3 files changed, 159 insertions, 31 deletions

diff --git a/client/src/com/vaadin/client/WidgetUtil.java b/client/src/com/vaadin/client/WidgetUtil.java
index 5f88f6da46..a9cd23c841 100644
--- a/client/src/com/vaadin/client/WidgetUtil.java
+++ b/client/src/com/vaadin/client/WidgetUtil.java
@@ -1459,4 +1459,124 @@ public class WidgetUtil {
         return Logger.getLogger(WidgetUtil.class.getName());
     }
 
+    /**
+     * Returns the thickness of the given element's top border.
+     * <p>
+     * The value is determined using computed style when available and
+     * calculated otherwise.
+     * 
+     * @since
+     * @param element
+     *            the element to measure
+     * @return the top border thickness
+     */
+    public static double getBorderTopThickness(Element element) {
+        return getBorderThickness(element, new String[] { "borderTopWidth" });
+    }
+
+    /**
+     * Returns the thickness of the given element's bottom border.
+     * <p>
+     * The value is determined using computed style when available and
+     * calculated otherwise.
+     * 
+     * @since
+     * @param element
+     *            the element to measure
+     * @return the bottom border thickness
+     */
+    public static double getBorderBottomThickness(Element element) {
+        return getBorderThickness(element, new String[] { "borderBottomWidth" });
+    }
+
+    /**
+     * Returns the combined thickness of the given element's top and bottom
+     * borders.
+     * <p>
+     * The value is determined using computed style when available and
+     * calculated otherwise.
+     * 
+     * @since
+     * @param element
+     *            the element to measure
+     * @return the top and bottom border thickness
+     */
+    public static double getBorderTopAndBottomThickness(Element element) {
+        return getBorderThickness(element, new String[] { "borderTopWidth",
+                "borderBottomWidth" });
+    }
+
+    /**
+     * Returns the thickness of the given element's left border.
+     * <p>
+     * The value is determined using computed style when available and
+     * calculated otherwise.
+     * 
+     * @since
+     * @param element
+     *            the element to measure
+     * @return the left border thickness
+     */
+    public static double getBorderLeftThickness(Element element) {
+        return getBorderThickness(element, new String[] { "borderLeftWidth" });
+    }
+
+    /**
+     * Returns the thickness of the given element's right border.
+     * <p>
+     * The value is determined using computed style when available and
+     * calculated otherwise.
+     * 
+     * @since
+     * @param element
+     *            the element to measure
+     * @return the right border thickness
+     */
+    public static double getBorderRightThickness(Element element) {
+        return getBorderThickness(element, new String[] { "borderRightWidth" });
+    }
+
+    /**
+     * Returns the thickness of the given element's left and right borders.
+     * <p>
+     * The value is determined using computed style when available and
+     * calculated otherwise.
+     * 
+     * @since
+     * @param element
+     *            the element to measure
+     * @return the top border thickness
+     */
+    public static double getBorderLeftAndRightThickness(Element element) {
+        return getBorderThickness(element, new String[] { "borderLeftWidth",
+                "borderRightWidth" });
+    }
+
+    private static native double getBorderThickness(
+            com.google.gwt.dom.client.Element element, String[] borderNames)
+    /*-{
+        if (typeof $wnd.getComputedStyle === 'function') {
+            var computedStyle = $wnd.getComputedStyle(element);
+            var width = 0;
+            for (i=0; i< borderNames.length; i++) {
+                var borderWidth = computedStyle[borderNames[i]];
+                width += parseFloat(borderWidth);
+            }
+            return width;
+        } else {
+            var parentElement = element.offsetParent;
+            var cloneElement = element.cloneNode(false);
+            cloneElement.style.boxSizing ="content-box";
+            parentElement.appendChild(cloneElement);
+            cloneElement.style.height = "10px"; // IE8 wants the height to be set to something...
+            var heightWithBorder = cloneElement.offsetHeight;
+            for (i=0; i< borderNames.length; i++) {
+                cloneElement.style[borderNames[i]] = "0";
+            }
+            var heightWithoutBorder = cloneElement.offsetHeight;
+            parentElement.removeChild(cloneElement);
+            
+            return heightWithBorder - heightWithoutBorder;
+        }
+    }-*/;
 }
diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java
index 17236c5e30..0cd59ce7ed 100644
--- a/client/src/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/com/vaadin/client/widgets/Escalator.java
@@ -4580,6 +4580,7 @@ public class Escalator extends Widget implements RequiresResize,
             private double height = -1;
             private boolean domHasBeenSetup = false;
             private double decoHeight;
+            private double defaultCellBorderBottomSize = -1;
 
             public SpacerImpl(int rowIndex) {
                 this.rowIndex = rowIndex;
@@ -4620,7 +4621,12 @@ public class Escalator extends Widget implements RequiresResize,
 
             public void setPosition(double x, double y) {
                 positions.set(getRootElement(), x, y);
-                positions.set(getDecoElement(), 0, y);
+                positions
+                        .set(getDecoElement(), 0, y - getSpacerDecoTopOffset());
+            }
+
+            private double getSpacerDecoTopOffset() {
+                return getBody().getDefaultRowHeight();
             }
 
             public void setStylePrimaryName(String style) {
@@ -4637,7 +4643,18 @@ public class Escalator extends Widget implements RequiresResize,
                 final double oldHeight = this.height;
 
                 this.height = height;
-                root.getStyle().setHeight(height, Unit.PX);
+
+                // since the spacer might be rendered on top of the previous
+                // rows border (done with css), need to increase height the
+                // amount of the border thickness
+                if (defaultCellBorderBottomSize < 0) {
+                    defaultCellBorderBottomSize = WidgetUtil
+                            .getBorderBottomThickness(body.getRowElement(
+                                    getVisibleRowRange().getStart())
+                                    .getFirstChildElement());
+                }
+                root.getStyle().setHeight(height + defaultCellBorderBottomSize,
+                        Unit.PX);
 
                 // move the visible spacers getRow row onwards.
                 shiftSpacerPositionsAfterRow(getRow(), heightDiff);
@@ -4722,34 +4739,9 @@ public class Escalator extends Widget implements RequiresResize,
             private void updateDecoratorGeometry(double detailsHeight) {
                 Style style = deco.getStyle();
                 decoHeight = detailsHeight + getBody().getDefaultRowHeight();
-
-                style.setTop(
-                        -(getBody().getDefaultRowHeight() - getBorderTopHeight(getElement())),
-                        Unit.PX);
                 style.setHeight(decoHeight, Unit.PX);
             }
 
-            private native double getBorderTopHeight(Element spacerCell)
-            /*-{
-                if (typeof $wnd.getComputedStyle === 'function') {
-                    var computedStyle = $wnd.getComputedStyle(spacerCell);
-                    var borderTopWidth = computedStyle['borderTopWidth'];
-                    var width = parseFloat(borderTopWidth);
-                    return width;
-                } else {
-                    var spacerRow = spacerCell.offsetParent;
-                    var cloneCell = spacerCell.cloneNode(false);
-                    spacerRow.appendChild(cloneCell);
-                    cloneCell.style.height = "10px"; // IE8 wants the height to be set to something...
-                    var heightWithBorder = cloneCell.offsetHeight;
-                    cloneCell.style.borderTopWidth = "0";
-                    var heightWithoutBorder = cloneCell.offsetHeight;
-                    spacerRow.removeChild(cloneCell);
-                    
-                    return heightWithBorder - heightWithoutBorder;
-                }
-            }-*/;
-
             @Override
             public Element getElement() {
                 return spacerElement;
@@ -4807,10 +4799,12 @@ public class Escalator extends Widget implements RequiresResize,
 
             public void show() {
                 getRootElement().getStyle().clearDisplay();
+                getDecoElement().getStyle().clearDisplay();
             }
 
             public void hide() {
                 getRootElement().getStyle().setDisplay(Display.NONE);
+                getDecoElement().getStyle().setDisplay(Display.NONE);
             }
 
             /**
@@ -4832,9 +4826,10 @@ public class Escalator extends Widget implements RequiresResize,
                     final double topClip = Math.max(0.0D, bodyTop - top);
                     final double bottomClip = decoHeight
                             - Math.max(0.0D, bottom - bodyBottom);
+                    // TODO [optimize] not sure how GWT compiles this
                     final String clip = new StringBuilder("rect(")
                             .append(topClip).append("px,").append(decoWidth)
-                            .append("px,").append(bottomClip).append("px,0")
+                            .append("px,").append(bottomClip).append("px,0)")
                             .toString();
                     deco.getStyle().setProperty("clip", clip);
                 } else {
@@ -5258,16 +5253,21 @@ public class Escalator extends Widget implements RequiresResize,
                 spacerScrollerRegistration = addScrollHandler(spacerScroller);
             }
 
-            SpacerImpl spacer = new SpacerImpl(rowIndex);
+            final SpacerImpl spacer = new SpacerImpl(rowIndex);
 
             rowIndexToSpacer.put(rowIndex, spacer);
-            spacer.setPosition(getScrollLeft(), calculateSpacerTop(rowIndex));
+            // set the position before adding it to DOM
+            positions.set(spacer.getRootElement(), getScrollLeft(),
+                    calculateSpacerTop(rowIndex));
 
             TableRowElement spacerRoot = spacer.getRootElement();
             spacerRoot.getStyle().setWidth(
                     columnConfiguration.calculateRowWidth(), Unit.PX);
             body.getElement().appendChild(spacerRoot);
             spacer.setupDom(height);
+            // set the deco position, requires that spacer is in the DOM
+            positions.set(spacer.getDecoElement(), 0,
+                    spacer.getTop() - spacer.getSpacerDecoTopOffset());
 
             spacerDecoContainer.appendChild(spacer.getDecoElement());
             if (spacerDecoContainer.getParentElement() == null) {
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index 08a86fe6a7..4951997995 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -2899,9 +2899,17 @@ public class Grid<T> extends ResizeComposite implements
                 /*
                  * Once we have the content properly inside the DOM, we should
                  * re-measure it to make sure that it's the correct height.
+                 * 
+                 * This is rather tricky, since the row (tr) will get the
+                 * height, but the spacer cell (td) has the borders, which
+                 * should go on top of the previous row and next row.
                  */
-                double measuredHeight = WidgetUtil
+                double requiredHeightBoundingClientRectDouble = WidgetUtil
                         .getRequiredHeightBoundingClientRectDouble(element);
+                double borderTopAndBottomHeight = WidgetUtil
+                        .getBorderTopAndBottomThickness(spacerElement);
+                double measuredHeight = requiredHeightBoundingClientRectDouble
+                        + borderTopAndBottomHeight;
                 assert getElement().isOrHasChild(spacerElement) : "The spacer element wasn't in the DOM during measurement, but was assumed to be.";
                 spacerHeight = measuredHeight;
             }