Fix Table stuck scroll position after setting scroll to last page (#14147)

Change-Id: I9c39db5c2ccafd495a1ecc8b28687eeeebe3b6c2

5 files changed, 280 insertions, 2 deletions

diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java
index 9b0bc3d730..270661fe31 100644
--- a/client/src/com/vaadin/client/ui/VScrollTable.java
+++ b/client/src/com/vaadin/client/ui/VScrollTable.java
@@ -1185,7 +1185,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
 
         @Override
         public void execute() {
-            if (firstvisible > 0) {
+            if (firstvisible >= 0) {
                 firstRowInViewPort = firstvisible;
                 if (firstvisibleOnLastPage > -1) {
                     scrollBodyPanel
@@ -7925,4 +7925,8 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
             addCloseHandler.removeHandler();
         }
     }
+
+    public void totalRowsChanged() {
+        firstvisibleOnLastPage = -1;
+    }
 }
diff --git a/client/src/com/vaadin/client/ui/table/TableConnector.java b/client/src/com/vaadin/client/ui/table/TableConnector.java
index d37fd36522..04a2dfc3c4 100644
--- a/client/src/com/vaadin/client/ui/table/TableConnector.java
+++ b/client/src/com/vaadin/client/ui/table/TableConnector.java
@@ -146,6 +146,9 @@ public class TableConnector extends AbstractHasComponentsConnector implements
         getWidget().updatePageLength(uidl);
 
         getWidget().updateFirstVisibleAndScrollIfNeeded(uidl);
+        if (totalRowsChanged == true) {
+            getWidget().totalRowsChanged();
+        }
 
         getWidget().showRowHeaders = uidl.getBooleanAttribute("rowheaders");
         getWidget().showColHeaders = uidl.getBooleanAttribute("colheaders");
diff --git a/server/src/com/vaadin/ui/Table.java b/server/src/com/vaadin/ui/Table.java
index a8265662ea..82e87eb10d 100644
--- a/server/src/com/vaadin/ui/Table.java
+++ b/server/src/com/vaadin/ui/Table.java
@@ -1513,7 +1513,7 @@ public class Table extends AbstractSelect implements Action.Container,
                  * position not be updated correctly when the lazy rows are
                  * finally rendered.
                  */
-                currentPageFirstItemIndexOnLastPage = indexOnLastPage;
+                currentPageFirstItemIndexOnLastPage = currentPageFirstItemIndex;
             }
 
         } else {
diff --git a/uitest/src/com/vaadin/tests/components/table/TableScrollAfterAddRow.java b/uitest/src/com/vaadin/tests/components/table/TableScrollAfterAddRow.java
new file mode 100644
index 0000000000..d1d6edaa67
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/table/TableScrollAfterAddRow.java
@@ -0,0 +1,167 @@
+/*
+ * 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.table;
+
+/**
+ * 
+ * @author Vaadin Ltd
+ */
+import com.vaadin.data.Item;
+import com.vaadin.data.util.IndexedContainer;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.NativeButton;
+import com.vaadin.ui.Table;
+import com.vaadin.ui.VerticalLayout;
+
+/*
+ * 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.
+ */
+
+/**
+ * 
+ * @author Vaadin Ltd
+ */
+@SuppressWarnings("serial")
+public class TableScrollAfterAddRow extends AbstractTestUI {
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.ui.UI#init(com.vaadin.server.VaadinRequest)
+     */
+    @Override
+    protected void setup(VaadinRequest request) {
+
+        final int totalRows = 100;
+
+        final VerticalLayout layout = new VerticalLayout();
+
+        final IndexedContainer datasource = new IndexedContainer();
+
+        datasource.addContainerProperty("value", Integer.class, -1);
+        for (int i = 0; i < totalRows; i++) {
+            addRow(datasource);
+        }
+
+        final Table table = new Table();
+        table.setContainerDataSource(datasource);
+        layout.addComponent(table);
+        addComponent(layout);
+
+        final Label label = new Label("");
+        layout.addComponent(label);
+
+        NativeButton addRowButton = new NativeButton("Add row",
+                new NativeButton.ClickListener() {
+
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        addRow(datasource);
+                    }
+                });
+
+        NativeButton jumpToLastRowButton = new NativeButton("Jump to last row",
+                new NativeButton.ClickListener() {
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        jumpToLastRow(table);
+                    }
+                });
+        NativeButton jumpTo15thRowButton = new NativeButton("Jump to 15th row",
+                new NativeButton.ClickListener() {
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        jumpToFifteenthRow(table);
+                    }
+                });
+        NativeButton jumpToFirstRowButton = new NativeButton(
+                "Jump to first row", new NativeButton.ClickListener() {
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        jumpToFirstRow(table);
+                    }
+                });
+
+        NativeButton updateLabelButton = new NativeButton("UpdateLabel",
+                new NativeButton.ClickListener() {
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        label.setValue(Integer.toString(table
+                                .getCurrentPageFirstItemIndex()));
+                    }
+                });
+        layout.addComponent(addRowButton);
+        layout.addComponent(jumpToLastRowButton);
+        layout.addComponent(jumpTo15thRowButton);
+        layout.addComponent(jumpToFirstRowButton);
+        layout.addComponent(updateLabelButton);
+    }
+
+    private void jumpToFifteenthRow(Table table) {
+        table.setCurrentPageFirstItemIndex(14);
+    }
+
+    private void jumpToLastRow(Table table) {
+        int visibleRows = table.getContainerDataSource().size();
+        table.setCurrentPageFirstItemIndex(visibleRows - 1);
+    }
+
+    private void jumpToFirstRow(Table table) {
+        table.setCurrentPageFirstItemIndex(0);
+    }
+
+    private void addRow(IndexedContainer datasource) {
+        int rowNumber = datasource.size();
+        Item row = datasource.addItem(rowNumber);
+        row.getItemProperty("value").setValue(rowNumber);
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTestDescription()
+     */
+    @Override
+    protected String getTestDescription() {
+        // TODO Auto-generated method stub
+        return "";
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTicketNumber()
+     */
+    @Override
+    protected Integer getTicketNumber() {
+        return 14147;
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/components/table/TableScrollAfterAddRowTest.java b/uitest/src/com/vaadin/tests/components/table/TableScrollAfterAddRowTest.java
new file mode 100644
index 0000000000..a020ace6b0
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/table/TableScrollAfterAddRowTest.java
@@ -0,0 +1,104 @@
+/*
+ * 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.table;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.JavascriptExecutor;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.commands.TestBenchCommandExecutor;
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.LabelElement;
+import com.vaadin.testbench.screenshot.ImageComparison;
+import com.vaadin.testbench.screenshot.ReferenceNameGenerator;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * 
+ * @author Vaadin Ltd
+ */
+public class TableScrollAfterAddRowTest extends MultiBrowserTest {
+
+    @Before
+    public void init() {
+        openTestURL();
+    }
+
+    @Test
+    public void testJumpToFirstRow() {
+        jumpToFifteenthRow();
+        jumpToFirstRow();
+        assertEquals("0", getCurrentPageFirstItemIndex());
+    }
+
+    @Test
+    public void testAddRowAfterJumpToLastRow() throws InterruptedException {
+        jumpToLastRow();
+        addRow();
+        sleep(200);
+        assertEquals("85", getCurrentPageFirstItemIndex());
+    }
+
+    @Test
+    public void testAddRowAfterJumpingToLastRowAndScrollingUp()
+            throws InterruptedException {
+        jumpToLastRow();
+        scrollUp();
+        addRow();
+        sleep(200);
+        Assert.assertNotEquals("86", getCurrentPageFirstItemIndex());
+    }
+
+    private void scrollUp() {
+        WebElement actualElement = getDriver().findElement(
+                By.className("v-table-body-wrapper"));
+        JavascriptExecutor js = new TestBenchCommandExecutor(getDriver(),
+                new ImageComparison(), new ReferenceNameGenerator());
+        js.executeScript("arguments[0].scrollTop = " + 30, actualElement);
+    }
+
+    private String getCurrentPageFirstItemIndex() {
+        ButtonElement updateLabelButton = $(ButtonElement.class).get(4);
+        LabelElement label = $(LabelElement.class).get(1);
+        updateLabelButton.click();
+        return label.getText();
+    }
+
+    private void addRow() {
+        ButtonElement button = $(ButtonElement.class).get(0);
+        button.click();
+    }
+
+    private void jumpToFirstRow() {
+        ButtonElement button = $(ButtonElement.class).get(3);
+        button.click();
+    }
+
+    private void jumpToFifteenthRow() {
+        ButtonElement button = $(ButtonElement.class).get(2);
+        button.click();
+    }
+
+    private void jumpToLastRow() {
+        ButtonElement button = $(ButtonElement.class).get(1);
+        button.click();
+    }
+}