Fixes a bug with escalator.scrollToRow and spacers (#16644)

Change-Id: I9e148cf81d393cc489c20bf0265f4a3bc6fed069

5 files changed, 55 insertions, 13 deletions

diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java
index efa777111a..679b452fde 100644
--- a/client/src/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/com/vaadin/client/widgets/Escalator.java
@@ -1150,9 +1150,9 @@ public class Escalator extends Widget implements RequiresResize,
         public void scrollToRow(final int rowIndex,
                 final ScrollDestination destination, final double padding) {
 
-            getLogger().warning("[[spacers]] scrollToRow");
-
-            final double targetStartPx = body.getDefaultRowHeight() * rowIndex;
+            final double targetStartPx = (body.getDefaultRowHeight() * rowIndex)
+                    + body.spacerContainer
+                            .getSpacerHeightsSumUntilIndex(rowIndex);
             final double targetEndPx = targetStartPx
                     + body.getDefaultRowHeight();
 
diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index 6ab6c6b1a4..cf0e54156a 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -51,7 +51,6 @@ import com.vaadin.data.RpcDataProviderExtension.DataProviderKeyMapper;
 import com.vaadin.data.Validator.InvalidValueException;
 import com.vaadin.data.fieldgroup.DefaultFieldGroupFieldFactory;
 import com.vaadin.data.fieldgroup.FieldGroup;
-import com.vaadin.data.fieldgroup.FieldGroup.BindException;
 import com.vaadin.data.fieldgroup.FieldGroup.CommitException;
 import com.vaadin.data.fieldgroup.FieldGroupFieldFactory;
 import com.vaadin.data.sort.Sort;
@@ -2647,7 +2646,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
          * Getting a field before the editor has been opened depends on special
          * support from the {@link FieldGroup} in use. Using this method with a
          * user-provided <code>FieldGroup</code> might cause
-         * {@link BindException} to be thrown.
+         * {@link com.vaadin.data.fieldgroup.FieldGroup.BindException
+         * BindException} to be thrown.
          * 
          * @return the bound field; or <code>null</code> if the respective
          *         column is not editable
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java
index 61c75b6162..04c0933866 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java
@@ -33,6 +33,10 @@ import com.vaadin.tests.tb3.MultiBrowserTest;
 
 @TestCategory("escalator")
 public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest {
+
+    private static final String LOGICAL_ROW_ATTRIBUTE_NAME = "vLogicalRow";
+    private static final String SPACER_CSS_CLASS = "v-escalator-spacer";
+
     protected static final String COLUMNS_AND_ROWS = "Columns and Rows";
 
     protected static final String COLUMNS = "Columns";
@@ -52,6 +56,8 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest
     protected static final String BODY_ROWS = "Body Rows";
     protected static final String FOOTER_ROWS = "Footer Rows";
 
+    protected static final String SCROLL_TO = "Scroll to...";
+
     protected static final String REMOVE_ALL_INSERT_SCROLL = "Remove all, insert 30 and scroll 40px";
 
     protected static final String GENERAL = "General";
@@ -69,12 +75,15 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest
     protected static final String COLUMN_SPANNING = "Column spanning";
     protected static final String COLSPAN_NORMAL = "Apply normal colspan";
     protected static final String COLSPAN_NONE = "Apply no colspan";
+    protected static final String SET_100PX = "Set 100px";
     protected static final String SPACERS = "Spacers";
+    protected static final String REMOVE = "Remove";
+
     protected static final String ROW_MINUS1 = "Row -1";
     protected static final String ROW_1 = "Row 1";
+    protected static final String ROW_25 = "Row 25";
+    protected static final String ROW_75 = "Row 75";
     protected static final String ROW_99 = "Row 99";
-    protected static final String SET_100PX = "Set 100px";
-    protected static final String REMOVE = "Remove";
 
     @Override
     protected Class<?> getUIClass() {
@@ -173,15 +182,16 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest
     private TestBenchElement getRow(String sectionTag, int row) {
         TestBenchElement escalator = getEscalator();
         WebElement tableSection = escalator.findElement(By.tagName(sectionTag));
-        By xpath;
 
+        String xpathExpression = "tr[not(@class='" + SPACER_CSS_CLASS + "')]";
         if (row >= 0) {
             int fromFirst = row + 1;
-            xpath = By.xpath("tr[" + fromFirst + "]");
+            xpathExpression += "[" + fromFirst + "]";
         } else {
             int fromLast = Math.abs(row + 1);
-            xpath = By.xpath("tr[last() - " + fromLast + "]");
+            xpathExpression += "[last() - " + fromLast + "]";
         }
+        By xpath = By.xpath(xpathExpression);
         if (tableSection != null
                 && ((TestBenchElement) tableSection).isElementPresent(xpath)) {
             return (TestBenchElement) tableSection.findElement(xpath);
@@ -281,7 +291,7 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest
     }
 
     private List<WebElement> getSpacers() {
-        return getEscalator().findElements(By.className("v-escalator-spacer"));
+        return getEscalator().findElements(By.className(SPACER_CSS_CLASS));
     }
 
     protected boolean spacersAreFoundInDom() {
@@ -295,8 +305,8 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest
         System.out.println("size: " + spacers.size());
         for (WebElement spacer : spacers) {
             System.out.println(spacer + ", " + logicalRowIndex);
-            Boolean isInDom = (Boolean) executeScript(
-                    "return arguments[0]['vLogicalRow'] === arguments[1]",
+            Boolean isInDom = (Boolean) executeScript("return arguments[0]['"
+                    + LOGICAL_ROW_ATTRIBUTE_NAME + "'] === arguments[1]",
                     spacer, logicalRowIndex);
             if (isInDom) {
                 return spacer;
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java
index da3472aebf..c3468b373e 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java
@@ -267,6 +267,24 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest {
                 + "-1 spacer", 0, getScrollTop());
     }
 
+    @Test
+    public void scrollToRowWorksProperlyWithSpacers() throws Exception {
+        selectMenuPath(FEATURES, SPACERS, ROW_MINUS1, SET_100PX);
+        selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX);
+
+        /*
+         * we check for row -2 instead of -1, because escalator has the one row
+         * buffered underneath the footer
+         */
+        selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, SCROLL_TO, ROW_75);
+        Thread.sleep(500);
+        assertEquals("Row 75: 0,75", getBodyCell(-2, 0).getText());
+
+        selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, SCROLL_TO, ROW_25);
+        Thread.sleep(500);
+        assertEquals("Row 25: 0,25", getBodyCell(0, 0).getText());
+    }
+
     private static double[] getElementDimensions(WebElement element) {
         /*
          * we need to parse the style attribute, since using getCssValue gets a
diff --git a/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java b/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java
index dc86b89167..0d4aa305d9 100644
--- a/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java
+++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java
@@ -16,6 +16,7 @@ import com.vaadin.client.widget.escalator.RowContainer.BodyRowContainer;
 import com.vaadin.client.widget.escalator.Spacer;
 import com.vaadin.client.widget.escalator.SpacerUpdater;
 import com.vaadin.client.widgets.Escalator;
+import com.vaadin.shared.ui.grid.ScrollDestination;
 
 public class EscalatorBasicClientFeaturesWidget extends
         PureGWTTestApplication<Escalator> {
@@ -571,6 +572,19 @@ public class EscalatorBasicClientFeaturesWidget extends
                         escalator.setScrollTop(40);
                     }
                 }, menupath);
+
+        String[] scrollToRowMenuPath = new String[menupath.length + 1];
+        System.arraycopy(menupath, 0, scrollToRowMenuPath, 0, menupath.length);
+        scrollToRowMenuPath[scrollToRowMenuPath.length - 1] = "Scroll to...";
+        for (int i = 0; i < 100; i += 25) {
+            final int rowIndex = i;
+            addMenuCommand("Row " + i, new ScheduledCommand() {
+                @Override
+                public void execute() {
+                    escalator.scrollToRow(rowIndex, ScrollDestination.ANY, 0);
+                }
+            }, scrollToRowMenuPath);
+        }
     }
 
     private void createRowsMenu(final RowContainer container, String[] menupath) {