Refactor ComboBox pending selection handling (#19929)

Move the handling of selection when a navigation operation
pends on server reply to the connector, with a callback
that actually performs the selection.

Change-Id: I3fa95cda6d7e02ce5aa4140ed341b2d1ba74abfc

2 files changed, 62 insertions, 80 deletions

diff --git a/client/src/com/vaadin/client/ui/VFilterSelect.java b/client/src/com/vaadin/client/ui/VFilterSelect.java
index f2a002a0d9..7f23b6f84a 100644
--- a/client/src/com/vaadin/client/ui/VFilterSelect.java
+++ b/client/src/com/vaadin/client/ui/VFilterSelect.java
@@ -941,7 +941,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
                 // null is not visible on pages != 0, and not visible when
                 // filtering: handle separately
                 connector.requestFirstPage();
-                afterUpdateClientVariables();
 
                 suggestionPopup.hide();
                 return;
@@ -990,7 +989,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
                      */
                     lastNewItemString = enteredItemValue;
                     connector.sendNewItem(enteredItemValue);
-                    afterUpdateClientVariables();
                 }
             } else if (item != null
                     && !"".equals(lastFilter)
@@ -1255,14 +1253,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
     /** For internal use only. May be removed or replaced in the future. */
     public String lastFilter = "";
 
-    /** For internal use only. May be removed or replaced in the future. */
-    public enum Select {
-        NONE, FIRST, LAST
-    }
-
-    /** For internal use only. May be removed or replaced in the future. */
-    public Select selectPopupItemWhenResponseIsReceived = Select.NONE;
-
     /**
      * The current suggestion selected from the dropdown. This is one of the
      * values in currentSuggestions except when filtering, in this case
@@ -1509,7 +1499,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
         setWaitingForFilteringResponse(true);
         connector.requestPage(filter, page);
-        afterUpdateClientVariables();
 
         lastFilter = filter;
         currentPage = page;
@@ -1630,7 +1619,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
         if (!(newKey.equals(selectedOptionKey) || ("".equals(newKey) && selectedOptionKey == null))) {
             selectedOptionKey = newKey;
             connector.sendSelection(selectedOptionKey);
-            afterUpdateClientVariables();
 
             // currentPage = -1; // forget the page
         }
@@ -1879,7 +1867,12 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
     private void selectPrevPage() {
         if (currentPage > 0) {
             filterOptions(currentPage - 1, lastFilter);
-            setSelectPopupItemWhenResponseIsReceived(Select.LAST);
+            connector.setNavigateAfterPageReceivedCallback(new Runnable() {
+                @Override
+                public void run() {
+                    suggestionPopup.selectLastItem();
+                }
+            });
         }
     }
 
@@ -1889,7 +1882,12 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
     private void selectNextPage() {
         if (hasNextPage()) {
             filterOptions(currentPage + 1, lastFilter);
-            setSelectPopupItemWhenResponseIsReceived(Select.FIRST);
+            connector.setNavigateAfterPageReceivedCallback(new Runnable() {
+                @Override
+                public void run() {
+                    suggestionPopup.selectFirstItem();
+                }
+            });
         }
     }
 
@@ -2073,9 +2071,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
         }
         addStyleDependentName("focus");
 
-        if (connector.sendFocusEvent()) {
-            afterUpdateClientVariables();
-        }
+        connector.sendFocusEvent();
 
         connector.getConnection().getVTooltip()
                 .showAssistive(connector.getTooltipInfo(getElement()));
@@ -2134,9 +2130,7 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
         }
         removeStyleDependentName("focus");
 
-        if (connector.sendBlurEvent()) {
-            afterUpdateClientVariables();
-        }
+        connector.sendBlurEvent();
     }
 
     /*
@@ -2316,16 +2310,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
         AriaHelper.bindCaption(tb, captionElement);
     }
 
-    /*
-     * Anything that should be set after the client updates the server.
-     */
-    private void afterUpdateClientVariables() {
-        // We need this here to be consistent with the all the calls.
-        // Then set your specific selection type only after
-        // client.updateVariable() method call.
-        setSelectPopupItemWhenResponseIsReceived(Select.NONE);
-    }
-
     @Override
     public boolean isWorkPending() {
         return isWaitingForFilteringResponse()
@@ -2383,28 +2367,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
     /**
      * For internal use only - this method will be removed in the future.
      * 
-     * @return enum Select indicating which item (if any) to select when a new
-     *         page of data is received
-     */
-    public Select getSelectPopupItemWhenResponseIsReceived() {
-        return selectPopupItemWhenResponseIsReceived;
-    }
-
-    /**
-     * For internal use only - this method will be removed in the future.
-     * 
-     * @param selectPopupItemWhenResponseIsReceived
-     *            enum Select indicating which item (if any) to select when a
-     *            new page of data is received
-     */
-    public void setSelectPopupItemWhenResponseIsReceived(
-            Select selectPopupItemWhenResponseIsReceived) {
-        this.selectPopupItemWhenResponseIsReceived = selectPopupItemWhenResponseIsReceived;
-    }
-
-    /**
-     * For internal use only - this method will be removed in the future.
-     * 
      * @return true if the user has requested opening the popup
      */
     public boolean isPopupOpenerClicked() {
diff --git a/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java b/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java
index 5e2d4336d8..7b66bcef6f 100644
--- a/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java
+++ b/client/src/com/vaadin/client/ui/combobox/ComboBoxConnector.java
@@ -54,6 +54,8 @@ public class ComboBoxConnector extends AbstractFieldConnector implements
 
     private boolean immediate;
 
+    private Runnable pageChangeCallback;
+
     @Override
     protected void init() {
         super.init();
@@ -214,16 +216,8 @@ public class ComboBoxConnector extends AbstractFieldConnector implements
 
             getWidget().setWaitingForFilteringResponse(false);
 
-            if (!getWidget().isPopupOpenerClicked()
-                    && getWidget().getSelectPopupItemWhenResponseIsReceived() != VFilterSelect.Select.NONE) {
-
-                // we're paging w/ arrows
-                Scheduler.get().scheduleDeferred(new ScheduledCommand() {
-                    @Override
-                    public void execute() {
-                        navigateItemAfterPageChange();
-                    }
-                });
+            if (!getWidget().isPopupOpenerClicked()) {
+                navigateItemAfterPageChange();
             }
 
             if (getWidget().isUpdateSelectionWhenReponseIsReceived()) {
@@ -263,17 +257,21 @@ public class ComboBoxConnector extends AbstractFieldConnector implements
      * #11333
      */
     private void navigateItemAfterPageChange() {
-        if (getWidget().getSelectPopupItemWhenResponseIsReceived() == VFilterSelect.Select.LAST) {
-            getWidget().suggestionPopup.selectLastItem();
-        } else {
-            getWidget().suggestionPopup.selectFirstItem();
+        if (pageChangeCallback != null) {
+            // pageChangeCallback is not reset here but after any server request
+            // in case you are in between two requests both changing the page
+            // back and forth
+
+            // we're paging w/ arrows
+            Scheduler.get().scheduleDeferred(new ScheduledCommand() {
+                @Override
+                public void execute() {
+                    if (pageChangeCallback != null) {
+                        pageChangeCallback.run();
+                    }
+                }
+            });
         }
-
-        // If you're in between 2 requests both changing the page back and
-        // forth, you don't want this here, instead you need it before any
-        // other request.
-        // getWidget().selectPopupItemWhenResponseIsReceived =
-        // VFilterSelect.Select.NONE; // reset
     }
 
     private void performSelection(String selectedKey) {
@@ -384,6 +382,7 @@ public class ComboBoxConnector extends AbstractFieldConnector implements
      */
     public void sendNewItem(String itemValue) {
         rpc.createNewItem(itemValue);
+        afterSendRequestToServer();
     }
 
     /**
@@ -415,6 +414,7 @@ public class ComboBoxConnector extends AbstractFieldConnector implements
      */
     public void requestPage(String filter, int page) {
         rpc.requestPage(filter, page);
+        afterSendRequestToServer();
     }
 
     /**
@@ -429,6 +429,7 @@ public class ComboBoxConnector extends AbstractFieldConnector implements
      */
     public void sendSelection(String selection) {
         rpc.setSelectedItem(selection);
+        afterSendRequestToServer();
     }
 
     /**
@@ -441,15 +442,13 @@ public class ComboBoxConnector extends AbstractFieldConnector implements
      * versions.
      * 
      * @since
-     * @return true if an event was sent (there are registered listeners), false
-     *         otherwise
      */
-    public boolean sendFocusEvent() {
+    public void sendFocusEvent() {
         boolean registeredListeners = hasEventListener(EventId.FOCUS);
         if (registeredListeners) {
             focusAndBlurRpc.focus();
+            afterSendRequestToServer();
         }
-        return registeredListeners;
     }
 
     /**
@@ -462,15 +461,36 @@ public class ComboBoxConnector extends AbstractFieldConnector implements
      * versions.
      * 
      * @since
-     * @return true if an event was sent (there are registered listeners), false
-     *         otherwise
      */
-    public boolean sendBlurEvent() {
+    public void sendBlurEvent() {
         boolean registeredListeners = hasEventListener(EventId.BLUR);
         if (registeredListeners) {
             focusAndBlurRpc.blur();
+            afterSendRequestToServer();
         }
-        return registeredListeners;
+    }
+
+    /**
+     * Set a callback that is invoked when a page change occurs if there have
+     * not been intervening requests to the server. The callback is reset when
+     * any additional request is made to the server.
+     * 
+     * @since
+     * @param callback
+     */
+    public void setNavigateAfterPageReceivedCallback(Runnable callback) {
+        pageChangeCallback = callback;
+
+    }
+
+    /*
+     * Anything that should be set after the client updates the server.
+     */
+    private void afterSendRequestToServer() {
+        // We need this here to be consistent with the all the calls.
+        // Then set your specific selection type only after
+        // a server request method call.
+        pageChangeCallback = null;
     }
 
 }