Fix keyboard navigating in combo box (#11333).

Extract code which focuses on item after changing the page. Deferring  this method allows to update the list of items before focusing.

Change-Id: I7d249c2abbd5c24ca2d798736e483f2b7dfa59f1

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

@@ -1,12 +1,12 @@
 /*
  * 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
@@ -80,7 +80,7 @@ import com.vaadin.shared.ui.combobox.FilteringMode;
 
 /**
  * Client side implementation of the Select component.
- * 
+ *
  * TODO needs major refactoring (to be extensible etc)
  */
 @SuppressWarnings("deprecation")
@@ -100,7 +100,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
         /**
          * Constructor
-         * 
+         *
          * @param uidl
          *            The UIDL recieved from the server
          */
@@ -149,7 +149,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
         /**
          * Get the option key which represents the item on the server side.
-         * 
+         *
          * @return The key of the item
          */
         public String getOptionKey() {
@@ -158,7 +158,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
         /**
          * Get the URI of the icon. Used when constructing the displayed option.
-         * 
+         *
          * @return
          */
         public String getIconUri() {
@@ -252,7 +252,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
         /**
          * Shows the popup where the user can see the filtered options
-         * 
+         *
          * @param currentSuggestions
          *            The filtered suggestions
          * @param currentPage
@@ -275,6 +275,12 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
              * correctly. This issue manifests when a Combobox is placed in
              * another popupView which also needs to calculate the absoluteTop()
              * to position itself. #9768
+             *
+             * After deferring the showSuggestions method, a problem with
+             * navigating in the combo box occurs. Because of that the method
+             * navigateItemAfterPageChange in ComboBoxConnector class, which
+             * navigates to the exact item after page was changed also was
+             * marked as deferred. #11333
              */
             final SuggestionPopup popup = this;
             Scheduler.get().scheduleDeferred(new ScheduledCommand() {
@@ -332,7 +338,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
         /**
          * Should the next page button be visible to the user?
-         * 
+         *
          * @param active
          */
         private void setNextButtonActive(boolean active) {
@@ -352,7 +358,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
         /**
          * Should the previous page button be visible to the user
-         * 
+         *
          * @param active
          */
         private void setPrevButtonActive(boolean active) {
@@ -511,7 +517,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
          * amount of items are visible at a time and a scrollbar or buttons are
          * visible to change page. If paging is turned of then all options are
          * rendered into the popup menu.
-         * 
+         *
          * @param paging
          *            Should the paging be turned on?
          */
@@ -616,7 +622,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
         /**
          * Was the popup just closed?
-         * 
+         *
          * @return true if popup was just closed
          */
         public boolean isJustClosed() {
@@ -645,7 +651,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
         /**
          * Updates style names in suggestion popup to help theme building.
-         * 
+         *
          * @param uidl
          *            UIDL for the whole combo box
          * @param componentState
@@ -725,7 +731,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
         /**
          * Sets the suggestions rendered in the menu
-         * 
+         *
          * @param suggestions
          *            The suggestions to be rendered in the menu
          */
@@ -920,7 +926,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
     /**
      * TextBox variant used as input element for filter selects, which prevents
      * selecting text when disabled.
-     * 
+     *
      * @since 7.1.5
      */
     public class FilterSelectTextBox extends TextBox {
@@ -1172,7 +1178,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
      * It is invoked during the Constructor and should only be overridden if a
      * custom TextBox shall be used. The overriding method cannot use any
      * instance variables.
-     * 
+     *
      * @since 7.1.5
      * @return TextBox instance used by this VFilterSelect
      */
@@ -1185,7 +1191,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
      * instance. It is invoked during the Constructor and should only be
      * overridden if a custom SuggestionPopup shall be used. The overriding
      * method cannot use any instance variables.
-     * 
+     *
      * @since 7.1.5
      * @return SuggestionPopup instance used by this VFilterSelect
      */
@@ -1213,7 +1219,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
     /**
      * Does the Select have more pages?
-     * 
+     *
      * @return true if a next page exists, else false if the current page is the
      *         last page
      */
@@ -1228,7 +1234,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
     /**
      * Filters the options at a certain page. Uses the text box input as a
      * filter
-     * 
+     *
      * @param page
      *            The page which items are to be filtered
      */
@@ -1238,7 +1244,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
     /**
      * Filters the options at certain page using the given filter
-     * 
+     *
      * @param page
      *            The page to filter
      * @param filter
@@ -1250,7 +1256,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
     /**
      * Filters the options at certain page using the given filter
-     * 
+     *
      * @param page
      *            The page to filter
      * @param filter
@@ -1315,7 +1321,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
     /**
      * Sets the text in the text box.
-     * 
+     *
      * @param text
      *            the text to set in the text box
      */
@@ -1344,7 +1350,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
      * shown in the text box if nothing has been entered.
      * <p>
      * For internal use only. May be removed or replaced in the future.
-     * 
+     *
      * @param text
      *            The text the text box should contain.
      */
@@ -1359,7 +1365,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
     /**
      * Triggered when a suggestion is selected
-     * 
+     *
      * @param suggestion
      *            The suggestion that just got selected.
      */
@@ -1399,7 +1405,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
     /**
      * Sets the icon URI of the selected item. The icon is shown on the left
      * side of the item caption text. Set the URI to null to remove the icon.
-     * 
+     *
      * @param iconUri
      *            The URI of the icon
      */
@@ -1525,7 +1531,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
     /**
      * Triggered when a key is pressed in the text box
-     * 
+     *
      * @param event
      *            The KeyDownEvent
      */
@@ -1570,7 +1576,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
     /**
      * Triggered when a key was pressed in the suggestion popup.
-     * 
+     *
      * @param event
      *            The KeyDownEvent of the key
      */
@@ -1652,7 +1658,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
     /**
      * Triggered when a key was depressed
-     * 
+     *
      * @param event
      *            The KeyUpEvent of the key depressed
      */
@@ -1987,7 +1993,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
     /**
      * Get the width of the select in pixels where the text area and icon has
      * been included.
-     * 
+     *
      * @return The width in pixels
      */
     private int getMainWidth() {
@@ -2004,7 +2010,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
     /**
      * Handles special behavior of the mouse down event
-     * 
+     *
      * @param event
      */
     private void handleMouseDownEvent(Event event) {

client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java

@@ -1,12 +1,12 @@
 /*
  * 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
@@ -19,6 +19,8 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
+import com.google.gwt.core.client.Scheduler;
+import com.google.gwt.core.client.Scheduler.ScheduledCommand;
 import com.vaadin.client.ApplicationConnection;
 import com.vaadin.client.Paintable;
 import com.vaadin.client.UIDL;
@@ -48,7 +50,7 @@ public class ComboBoxConnector extends AbstractFieldConnector implements
 
     /*
      * (non-Javadoc)
-     * 
+     *
      * @see com.vaadin.client.Paintable#updateFromUIDL(com.vaadin.client.UIDL,
      * com.vaadin.client.ApplicationConnection)
      */
@@ -179,28 +181,14 @@ public class ComboBoxConnector extends AbstractFieldConnector implements
             if (!getWidget().popupOpenerClicked
                     && getWidget().selectPopupItemWhenResponseIsReceived != VFilterSelect.Select.NONE) {
                 // we're paging w/ arrows
-                if (getWidget().selectPopupItemWhenResponseIsReceived == VFilterSelect.Select.LAST) {
-                    getWidget().suggestionPopup.menu.selectLastItem();
-                } else {
-                    getWidget().suggestionPopup.menu.selectFirstItem();
-                }
-
-                // This is used for paging so we update the keyboard selection
-                // variable as well.
-                MenuItem activeMenuItem = getWidget().suggestionPopup.menu
-                        .getSelectedItem();
-                getWidget().suggestionPopup.menu
-                        .setKeyboardSelectedItem(activeMenuItem);
-
-                // Update text field to contain the correct text
-                getWidget().setTextboxText(activeMenuItem.getText());
-                getWidget().tb.setSelectionRange(
-                        getWidget().lastFilter.length(),
-                        activeMenuItem.getText().length()
-                                - getWidget().lastFilter.length());
-
-                getWidget().selectPopupItemWhenResponseIsReceived = VFilterSelect.Select.NONE; // reset
+                Scheduler.get().scheduleDeferred(new ScheduledCommand() {
+                    @Override
+                    public void execute() {
+                        navigateItemAfterPageChange();
+                    }
+                });
             }
+
             if (getWidget().updateSelectionWhenReponseIsReceived) {
                 getWidget().suggestionPopup.menu
                         .doPostFilterSelectedItemAction();
@@ -231,6 +219,38 @@ public class ComboBoxConnector extends AbstractFieldConnector implements
         getWidget().initDone = true;
     }
 
+    /*
+     * This method navigates to the proper item in the combobox page. This
+     * should be executed after setSuggestions() method which is called from
+     * vFilterSelect.showSuggestions(). ShowSuggestions() method builds the page
+     * content. As far as setSuggestions() method is called as deferred,
+     * navigateItemAfterPageChange method should be also be called as deferred.
+     * #11333
+     */
+    private void navigateItemAfterPageChange() {
+        if (getWidget().selectPopupItemWhenResponseIsReceived == VFilterSelect.Select.LAST) {
+            getWidget().suggestionPopup.menu.selectLastItem();
+        } else {
+            getWidget().suggestionPopup.menu.selectFirstItem();
+        }
+
+        // This is used for paging so we update the keyboard selection
+        // variable as well.
+        MenuItem activeMenuItem = getWidget().suggestionPopup.menu
+                .getSelectedItem();
+        getWidget().suggestionPopup.menu
+                .setKeyboardSelectedItem(activeMenuItem);
+
+        // Update text field to contain the correct text
+        getWidget().setTextboxText(activeMenuItem.getText());
+        getWidget().tb.setSelectionRange(
+                getWidget().lastFilter.length(),
+                activeMenuItem.getText().length()
+                        - getWidget().lastFilter.length());
+
+        getWidget().selectPopupItemWhenResponseIsReceived = VFilterSelect.Select.NONE; // reset
+    }
+
     private void performSelection(String selectedKey) {
         // some item selected
         for (FilterSelectSuggestion suggestion : getWidget().currentSuggestions) {

uitest/src/com/vaadin/tests/components/combobox/ComboBoxScrollingWithArrows.java

@@ -0,0 +1,72 @@
+/*
+ * 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.combobox;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.AbstractLayout;
+import com.vaadin.ui.ComboBox;
+import com.vaadin.ui.VerticalLayout;
+
+/**
+ * Test UI verifying navigating in combobox via arrow keys.
+ */
+public class ComboBoxScrollingWithArrows extends AbstractTestUI {
+    final String DESCRIPTION = "When positioned on last item in the page and press downArrow key - should open new page and set focus on the first item.";
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#setup(com.vaadin.server.
+     * VaadinRequest)
+     */
+    @Override
+    protected void setup(VaadinRequest request) {
+        VerticalLayout layout = new VerticalLayout();
+        addComponent(layout);
+        addComboBox(layout);
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTestDescription()
+     */
+    @Override
+    protected String getTestDescription() {
+        return DESCRIPTION;
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTicketNumber()
+     */
+    @Override
+    protected Integer getTicketNumber() {
+        return 11333;
+    }
+
+    private void addComboBox(AbstractLayout layout) {
+        ComboBox box = new ComboBox();
+        for (int i = 0; i < 100; i++) {
+            box.addItem("item " + i);
+        }
+        box.setPageLength(10);
+        box.setNullSelectionAllowed(false);
+        layout.addComponent(box);
+    }
+}

uitest/src/com/vaadin/tests/components/combobox/ComboBoxScrollingWithArrowsTest.java

@@ -0,0 +1,83 @@
+/*
+ * 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.combobox;
+
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.By;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * When pressed down key, while positioned on the last item - should show next
+ * page and focus on the first item of the next page.
+ */
+public class ComboBoxScrollingWithArrowsTest extends MultiBrowserTest {
+
+    @Before
+    public void openURL() {
+        openTestURL();
+    }
+
+    @Test
+    public void scrollDownArrowKeyTest() throws InterruptedException {
+        final int ITEMS_PER_PAGE = 10;
+        // Selenium is used instead of TestBench4, because there is no method to
+        // access the popup of the combobox
+        // The method ComboBoxElement.openPopup() opens the popup, but doesn't
+        // provide any way to access the popup and send keys to it.
+        // Ticket #13756
+        WebElement dropDownComboBox = driver.findElement(By
+                .className("v-filterselect-input"));
+        // opens Lookup
+        dropDownComboBox.sendKeys(Keys.DOWN);
+        // go to the last item and then one more
+        for (int i = 0; i < ITEMS_PER_PAGE + 1; i++) {
+            dropDownComboBox.sendKeys(Keys.DOWN);
+        }
+        String expected = "item " + ITEMS_PER_PAGE;// item 10
+
+        List<WebElement> items = driver.findElements(By
+                .className("gwt-MenuItem-selected"));
+        String actual = items.get(0).getText();
+        Assert.assertEquals(expected, actual);
+    }
+
+    @Test
+    public void scrollUpArrowKeyTest() throws InterruptedException {
+        final int ITEMS_PER_PAGE = 10;
+        WebElement dropDownComboBox = driver.findElement(By
+                .className("v-filterselect-input"));
+        // opens Lookup
+        dropDownComboBox.sendKeys(Keys.DOWN);
+        // go to the last item and then one more
+        for (int i = 0; i < ITEMS_PER_PAGE + 1; i++) {
+            dropDownComboBox.sendKeys(Keys.DOWN);
+        }
+        // move to one item up
+        dropDownComboBox.sendKeys(Keys.UP);
+        String expected = "item " + (ITEMS_PER_PAGE - 1);// item 9
+        List<WebElement> items = driver.findElements(By
+                .className("gwt-MenuItem-selected"));
+        String actual = items.get(0).getText();
+        Assert.assertEquals(expected, actual);
+    }
+}