Fix table focusing when scrolled from the server (#10522)

Change-Id: Ib18a60ae7e41af8c6c119e5b2e12e4fd5bf1069c

client/src/com/vaadin/client/ui/VScrollTable.java

@@ -1081,19 +1081,18 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                         selected = true;
                         keyboardSelectionOverRowFetchInProgress = true;
                     }
+                    if (selected) {
+                        if (focusedRow == null
+                                || !selectedRowKeys.contains(focusedRow
+                                        .getKey())) {
+                            // The focus is no longer on a selected row,
+                            // move focus to first selected row
+                            setRowFocus(row);
+                        }
+                    }
                     if (selected != row.isSelected()) {
                         row.toggleSelection();
 
-                        if (selected) {
-                            if (focusedRow == null
-                                    || !selectedRowKeys.contains(focusedRow
-                                            .getKey())) {
-                                // The focus is no longer on a selected row,
-                                // move focus to first selected row
-                                setRowFocus(row);
-                            }
-                        }
-
                         if (!isSingleSelectMode() && !selected) {
                             // Update selection range in case a row is
                             // unselected from the middle of a range - #8076
@@ -1101,6 +1100,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                         }
                     }
                 }
+
             }
         }
         unSyncedselectionsBeforeRowFetch = null;
@@ -5301,17 +5301,12 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
              */
             public boolean isInViewPort() {
                 int absoluteTop = getAbsoluteTop();
-                int scrollPosition = scrollBodyPanel.getAbsoluteTop()
-                        + scrollBodyPanel.getScrollPosition();
-                if (absoluteTop < scrollPosition) {
-                    return false;
-                }
-                int maxVisible = scrollPosition
-                        + scrollBodyPanel.getOffsetHeight() - getOffsetHeight();
-                if (absoluteTop > maxVisible) {
-                    return false;
-                }
-                return true;
+                int absoluteBottom = absoluteTop + getOffsetHeight();
+                int viewPortTop = scrollBodyPanel.getAbsoluteTop();
+                int viewPortBottom = viewPortTop
+                        + scrollBodyPanel.getOffsetHeight();
+                return absoluteBottom > viewPortTop
+                        && absoluteTop < viewPortBottom;
             }
 
             /**

uitest/src/com/vaadin/tests/components/table/FocusOnSelectedItem.java

@@ -0,0 +1,84 @@
+/*
+ * Copyright 2000-2013 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 com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Table;
+
+/**
+ * Test to see if the correct row gets the focus when the row is selected from
+ * the serverside and forces the table to scroll down
+ * 
+ * @author Vaadin Ltd
+ */
+public class FocusOnSelectedItem extends AbstractTestUI {
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#setup(com.vaadin.server.
+     * VaadinRequest)
+     */
+    @Override
+    protected void setup(VaadinRequest request) {
+
+        final Table table = new Table();
+        table.setSelectable(true);
+        table.setImmediate(true);
+
+        table.addContainerProperty("Property", String.class, null);
+
+        for (int i = 0; i < 200; i++) {
+            table.addItem(new String[] { "Item " + i }, "Item " + i);
+        }
+        addComponent(table);
+
+        Button button = new Button("Select");
+        button.addClickListener(new Button.ClickListener() {
+            @Override
+            public void buttonClick(ClickEvent event) {
+                table.setValue("Item 198");
+                table.setCurrentPageFirstItemId("Item 198");
+                table.focus();
+            }
+        });
+        addComponent(button);
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTestDescription()
+     */
+    @Override
+    protected String getTestDescription() {
+        return "Test whether the selected row retains focus.";
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTicketNumber()
+     */
+    @Override
+    protected Integer getTicketNumber() {
+        return 10522;
+    }
+
+}

uitest/src/com/vaadin/tests/components/table/FocusOnSelectedItemTest.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2000-2013 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 org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.TableElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * Test to see if the correct row gets the focus when the row is selected from
+ * the serverside and forces the table to scroll down
+ * 
+ * @author Vaadin Ltd
+ */
+public class FocusOnSelectedItemTest extends MultiBrowserTest {
+
+    @Test
+    public void selectAndScrollFocusesSelectedRow() {
+        openTestURL();
+
+        WebElement selectButton = $(ButtonElement.class).caption("Select")
+                .first();
+        selectButton.click();
+        WebElement supposedlyFocusedRow = null;
+        WebElement selectedRow = null;
+        WebElement focusedStyleRow = null;
+
+        Assert.assertTrue("No row was selected",
+                isElementPresent(By.className("v-selected")));
+
+        selectedRow = getDriver().findElement(By.className("v-selected"));
+
+        supposedlyFocusedRow = $(TableElement.class).first().getCell(198, 0);
+
+        Assert.assertTrue("Incorrect row was selected", selectedRow
+                .getLocation().getY() == supposedlyFocusedRow.getLocation()
+                .getY());
+
+        Assert.assertTrue("No row had the focused style.",
+                isElementPresent(By.className("v-table-focus")));
+
+        focusedStyleRow = getDriver()
+                .findElement(By.className("v-table-focus"));
+        Assert.assertTrue("Incorrect row has the focused style.", selectedRow
+                .getLocation().getY() == focusedStyleRow.getLocation().getY());
+
+    }
+}