Calculate Grid sidebar button height when closed #17412

+ contains Valo theming fixes for all browsers.
+ includes screenshot test for sidebar in Valo

Change-Id: Ic6401057efff7e4d4ab65c46885dda2d995bce5d

WebContent/VAADIN/themes/base/grid/grid.scss

@@ -85,9 +85,9 @@ $v-grid-details-border-bottom-stripe: 1px solid darken($v-grid-row-background-co
       }
     }
   }
-  
+
   // Sidebar
-  
+
   .#{$primaryStyleName}-sidebar.v-contextmenu {
     @include box-shadow(none);
     position: absolute;
@@ -103,32 +103,44 @@ $v-grid-details-border-bottom-stripe: 1px solid darken($v-grid-row-background-co
       background: transparent;
       border: none;
       cursor: pointer;
-      height: $v-grid-header-row-height;
       outline: none;
       padding: 0 4px;
       text-align: right;
 
+      &::-moz-focus-inner {
+        border: 0;
+      }
+
       &:after {
         content: "\f0c9";
+        display: block;
         font-family: FontAwesome, sans-serif;
         font-size: $v-grid-header-font-size;
-        line-height: $v-grid-header-row-height;
       }
     }
 
     &.closed {
       border-radius: 0;
     }
-    
+
     &.opened {
       .#{$primaryStyleName}-sidebar-button {
         width: 100%;
-      
+ 
         &:after {
           content: "\00d7";
           font-size: 16px;
+          line-height: 1;
         }
       }
+    }      
+
+    .v-ie &.opened .#{$primaryStyleName}-sidebar-button {
+      vertical-align: middle;
+    }
+
+    .v-ie8 &.opened .#{$primaryStyleName}-sidebar-button:after {
+      display: inline;
     }
 
     .#{$primaryStyleName}-sidebar-content {

WebContent/VAADIN/themes/reindeer/grid/grid.scss

@@ -34,15 +34,9 @@
       border-color: #b1cde4;
     }
   }
-  
+
   // Sidebar
   .#{$primaryStyleName}-sidebar.v-contextmenu {
-    &.closed {
-      .#{$primaryStyleName}-sidebar-button:after {
-        line-height: 20px;
-      }
-    }
-    
     .#{$primaryStyleName}-sidebar-content {
       background-color: #f8f8f9;
     }

WebContent/VAADIN/themes/valo/components/_grid.scss

@@ -193,24 +193,19 @@ $v-grid-details-border-bottom-stripe: $v-grid-cell-horizontal-border !default;
     &.opened {
       .#{$primary-stylename}-sidebar-button:after {
         font-size: 20px;
-        line-height: 20px;
       }
-      
+
       .#{$primary-stylename}-sidebar-content {
         margin: 0 0 2px;
         padding: 4px 4px 2px;
       }
     }
-    
+
     &.closed {
       @include valo-gradient($v-grid-header-background-color);
-      
-      .#{$primary-stylename}-sidebar-button {
-        line-height: 36px;
-      }
     }
   }
-  
+
   // Customize scrollbars
   .#{$primary-stylename}-scroller {
     &::-webkit-scrollbar {

client/src/com/vaadin/client/widgets/Grid.java

@@ -3080,6 +3080,7 @@ public class Grid<T> extends ResizeComposite implements
                 removeStyleName("closed");
                 rootContainer.add(content);
             }
+            openCloseButton.setHeight("");
         }
 
         /**
@@ -3090,6 +3091,8 @@ public class Grid<T> extends ResizeComposite implements
                 removeStyleName("opened");
                 addStyleName("closed");
                 content.removeFromParent();
+                // adjust open button to header height when closed
+                setHeightToHeaderCellHeight();
             }
         }
 
@@ -3156,6 +3159,23 @@ public class Grid<T> extends ResizeComposite implements
             }
         }
 
+        private void setHeightToHeaderCellHeight() {
+            try {
+                double height = WidgetUtil
+                        .getRequiredHeightBoundingClientRectDouble(grid.escalator
+                                .getHeader().getRowElement(0)
+                                .getFirstChildElement())
+                        - (WidgetUtil.measureVerticalBorder(getElement()) / 2);
+                openCloseButton.setHeight(height + "px");
+            } catch (NullPointerException npe) {
+                getLogger()
+                        .warning(
+                                "Got null header first row or first row cell when calculating sidebar button height");
+                openCloseButton.setHeight(grid.escalator.getHeader()
+                        .getDefaultRowHeight() + "px");
+            }
+        }
+
         private void updateVisibility() {
             final boolean hasWidgets = content.getWidgetCount() > 0;
             final boolean isVisible = isInDOM();
@@ -3166,6 +3186,8 @@ public class Grid<T> extends ResizeComposite implements
                 close();
                 grid.getElement().appendChild(getElement());
                 Grid.setParent(this, grid);
+                // border calculation won't work until attached
+                setHeightToHeaderCellHeight();
             }
         }
 

uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeaturesTest.java

@@ -233,4 +233,31 @@ public abstract class GridBasicFeaturesTest extends MultiBrowserTest {
         assertTrue(getGridElement().getCell(row, column).getAttribute("class")
                 .contains("focused"));
     }
+
+    protected WebElement getSidebar() {
+        List<WebElement> elements = findElements(By.className("v-grid-sidebar"));
+        return elements.isEmpty() ? null : elements.get(0);
+    }
+
+    protected WebElement getSidebarOpenButton() {
+        List<WebElement> elements = findElements(By
+                .className("v-grid-sidebar-button"));
+        return elements.isEmpty() ? null : elements.get(0);
+    }
+
+    /**
+     * Returns the toggle inside the sidebar for hiding the column at the given
+     * index, or null if not found.
+     */
+    protected WebElement getColumnHidingToggle(int columnIndex) {
+        WebElement sidebar = getSidebar();
+        List<WebElement> elements = sidebar.findElements(By
+                .className("column-hiding-toggle"));
+        for (WebElement e : elements) {
+            if ((e.getText().toLowerCase()).startsWith("column " + columnIndex)) {
+                return e;
+            }
+        }
+        return null;
+    }
 }

uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridColumnHidingTest.java

@@ -921,12 +921,14 @@ public class GridColumnHidingTest extends GridBasicClientFeaturesTest {
         assertNotNull(sidebar);
     }
 
-    private WebElement getSidebar() {
+    @Override
+    protected WebElement getSidebar() {
         List<WebElement> elements = findElements(By.className("v-grid-sidebar"));
         return elements.isEmpty() ? null : elements.get(0);
     }
 
-    private WebElement getSidebarOpenButton() {
+    @Override
+    protected WebElement getSidebarOpenButton() {
         List<WebElement> elements = findElements(By
                 .className("v-grid-sidebar-button"));
         return elements.isEmpty() ? null : elements.get(0);
@@ -936,7 +938,8 @@ public class GridColumnHidingTest extends GridBasicClientFeaturesTest {
      * Returns the toggle inside the sidebar for hiding the column at the given
      * index, or null if not found.
      */
-    private WebElement getColumnHidingToggle(int columnIndex) {
+    @Override
+    protected WebElement getColumnHidingToggle(int columnIndex) {
         WebElement sidebar = getSidebar();
         List<WebElement> elements = sidebar.findElements(By
                 .className("column-hiding-toggle"));

uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java

@@ -21,12 +21,8 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
-import java.util.List;
-
 import org.junit.Before;
 import org.junit.Test;
-import org.openqa.selenium.By;
-import org.openqa.selenium.WebElement;
 
 import com.vaadin.testbench.parallel.TestCategory;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest;
@@ -281,31 +277,4 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest {
         selectMenuPath("Component", "Columns", "Column " + index,
                 "Add / Remove");
     }
-
-    private WebElement getSidebar() {
-        List<WebElement> elements = findElements(By.className("v-grid-sidebar"));
-        return elements.isEmpty() ? null : elements.get(0);
-    }
-
-    private WebElement getSidebarOpenButton() {
-        List<WebElement> elements = findElements(By
-                .className("v-grid-sidebar-button"));
-        return elements.isEmpty() ? null : elements.get(0);
-    }
-
-    /**
-     * Returns the toggle inside the sidebar for hiding the column at the given
-     * index, or null if not found.
-     */
-    private WebElement getColumnHidingToggle(int columnIndex) {
-        WebElement sidebar = getSidebar();
-        List<WebElement> elements = sidebar.findElements(By
-                .className("column-hiding-toggle"));
-        for (WebElement e : elements) {
-            if ((e.getText().toLowerCase()).startsWith("column " + columnIndex)) {
-                return e;
-            }
-        }
-        return null;
-    }
 }

uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridSidebarThemeTest.java

@@ -0,0 +1,86 @@
+/*
+ * 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.tests.components.grid.basicfeatures.server;
+
+import java.io.IOException;
+import java.util.List;
+
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.interactions.Actions;
+import org.openqa.selenium.remote.DesiredCapabilities;
+import org.openqa.selenium.support.ui.ExpectedConditions;
+
+import com.vaadin.testbench.parallel.Browser;
+import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest;
+
+public class GridSidebarThemeTest extends GridBasicFeaturesTest {
+
+    @Test
+    public void testValo() throws Exception {
+        runTestSequence("valo");
+    }
+
+    private void runTestSequence(String theme) throws IOException {
+        openTestURL("theme=" + theme);
+        if (getDesiredCapabilities().getBrowserName().equals(
+                Browser.CHROME.getDesiredCapabilities().getBrowserName())) {
+            waitUntil(ExpectedConditions.elementToBeClickable(By.id("menu")), 2);
+            getDriver().findElement(By.id("menu")).click();
+            selectMenu("Columns");
+            selectMenu("All columns hidable");
+            waitUntilLoadingIndicatorNotVisible();
+        } else {
+            selectMenuPath("Component", "Columns", "All columns hidable");
+        }
+
+        compareScreen(theme + "|SidebarClosed");
+        getSidebarOpenButton().click();
+
+        compareScreen(theme + "|SidebarOpen");
+
+        new Actions(getDriver()).moveToElement(getColumnHidingToggle(2), 5, 5)
+                .perform();
+
+        compareScreen(theme + "|OnMouseOverNotHiddenToggle");
+
+        getColumnHidingToggle(2).click();
+        getColumnHidingToggle(3).click();
+        getColumnHidingToggle(6).click();
+
+        new Actions(getDriver()).moveToElement(getSidebarOpenButton())
+                .perform();
+        ;
+
+        compareScreen(theme + "|TogglesTriggered");
+
+        new Actions(getDriver()).moveToElement(getColumnHidingToggle(2))
+                .perform();
+        ;
+
+        compareScreen(theme + "|OnMouseOverHiddenToggle");
+
+        getSidebarOpenButton().click();
+
+        compareScreen(theme + "|SidebarClosed2");
+    }
+
+    @Override
+    public List<DesiredCapabilities> getBrowsersToTest() {
+        // phantom JS looks wrong from the beginning, so not tested
+        return getBrowsersExcludingPhantomJS();
+    }
+}

uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/LoadingIndicatorTest.java

@@ -19,7 +19,6 @@ import org.junit.Assert;
 import org.junit.Test;
 import org.openqa.selenium.By;
 import org.openqa.selenium.WebDriver;
-import org.openqa.selenium.WebElement;
 import org.openqa.selenium.support.ui.ExpectedCondition;
 import org.openqa.selenium.support.ui.ExpectedConditions;
 
@@ -83,10 +82,4 @@ public class LoadingIndicatorTest extends GridBasicFeaturesTest {
         });
     }
 
-    private boolean isLoadingIndicatorVisible() {
-        WebElement loadingIndicator = findElement(By
-                .className("v-loading-indicator"));
-
-        return loadingIndicator.isDisplayed();
-    }
 }

uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java

@@ -938,4 +938,24 @@ public abstract class AbstractTB3Test extends ParallelTest {
     protected void click(CheckBoxElement checkbox) {
         checkbox.findElement(By.xpath("input")).click();
     }
+
+    protected boolean isLoadingIndicatorVisible() {
+        WebElement loadingIndicator = findElement(By
+                .className("v-loading-indicator"));
+
+        return loadingIndicator.isDisplayed();
+    }
+
+    protected void waitUntilLoadingIndicatorNotVisible() {
+        waitUntil(new ExpectedCondition<Boolean>() {
+
+            @Override
+            public Boolean apply(WebDriver input) {
+                WebElement loadingIndicator = input.findElement(By
+                        .className("v-loading-indicator"));
+
+                return !loadingIndicator.isDisplayed();
+            }
+        });
+    }
 }