Client side filter fix. (#192)

* Fix client-side filter.

src/main/resources/META-INF/resources/frontend/comboBoxConnector.js

@@ -9,7 +9,6 @@ window.Vaadin.Flow.comboBoxConnector = {
 
     let pageCallbacks = {};
     let cache = {};
-    let firstPage;
     let lastFilter = '';
 
     comboBox.size = 0; // To avoid NaN here and there before we get proper data
@@ -20,21 +19,32 @@ window.Vaadin.Flow.comboBoxConnector = {
         throw 'Invalid pageSize';
       }
 
+      if (comboBox._clientSideFilter) {
+        // For clientside filter we first make sure we have all data which we also
+        // filter based on comboBox.filter. While later we only filter clientside data.
+
+        if (cache[0]) {
+          performClientSideFilter(cache[0], callback)
+          return;
+
+        } else {
+          // If client side filter is enabled then we need to first ask all data
+          // and filter it on client side, otherwise next time when user will
+          // input another filter, eg. continue to type, the local cache will be only
+          // what was received for the first filter, which may not be the whole
+          // data from server (keep in mind that client side filter is enabled only
+          // when the items count does not exceed one page).
+          params.filter = "";
+        }
+      }
+
       const filterChanged = params.filter !== lastFilter;
       if (filterChanged) {
+        pageCallbacks = {};
         cache = {};
         lastFilter = params.filter;
       }
 
-      if (comboBox._clientSideFilter && firstPage) {
-        // Data size is less than page size and client has all the data,
-        // so client-side filtering is used
-        const filteredItems = firstPage.filter(item =>
-          comboBox.$connector.filter(item, comboBox.filter));
-        callback(filteredItems, filteredItems.size);
-        return;
-      }
-
       if (cache[params.page]) {
         // This may happen after skipping pages by scrolling fast
         commitPage(params.page, callback);
@@ -110,15 +120,24 @@ window.Vaadin.Flow.comboBoxConnector = {
     }
 
     comboBox.$connector.updateSize = function (newSize) {
-      comboBox.size = newSize;
-    };
+      if (!comboBox._clientSideFilter) {
+        // FIXME: It may be that this size set is unnecessary, since when
+        // providing data to combobox via callback we may use data's size.
+        // However, if this size reflect the whole data size, including
+        // data not fetched yet into client side, and combobox expect it
+        // to be set as such, the at least, we don't need it in case the
+        // filter is clientSide only, since it'll increase the height of
+        // the popup at only at first user filter to this size, while the
+        // filtered items count are less.
+        comboBox.size = newSize;
+      }
+    }
 
     comboBox.$connector.reset = function () {
       pageCallbacks = {};
       cache = {};
-      firstPage = undefined;
       comboBox.clearCache();
-    };
+    }
 
     comboBox.$connector.confirm = function (id, filter) {
 
@@ -146,13 +165,35 @@ window.Vaadin.Flow.comboBoxConnector = {
 
     const commitPage = function (page, callback) {
       let data = cache[page];
-      delete cache[page];
 
-      if (page == 0) {
-        // Keep the data for client-side filtering
-        firstPage = data;
+      if (comboBox._clientSideFilter) {
+        performClientSideFilter(data, callback)
+
+      } else {
+        // Remove the data if server-side filtering, but keep it for client-side
+        // filtering
+        delete cache[page];
+
+        // FIXME: It may be that we ought to provide data.length instead of
+        // comboBox.size and remove updateSize function.
+        callback(data, comboBox.size);
+      }
+    }
+
+    // Perform filter on client side (here) using the items from specified page
+    // and submitting the filtered items to specified callback.
+    // The filter used is the one from combobox, not the lastFilter stored since
+    // that may not reflect user's input.
+    const performClientSideFilter = function (page, callback) {
+
+      let filteredItems = page;
+
+      if (comboBox.filter) {
+        filteredItems = page.filter(item =>
+          comboBox.$connector.filter(item, comboBox.filter));
       }
-      callback(data, comboBox.size);
+
+      callback(filteredItems, filteredItems.length);
     }
   }
 }

src/test/java/com/vaadin/flow/component/combobox/test/AbstractComboBoxIT.java

@@ -158,7 +158,7 @@ protected String getSelectedItemLabel(WebElement combo) {
     /**
      * Wait for the items of the specified combobox to fulfill the specified
      * condition.
-     * 
+     *
      * @param combo
      *            The combobox element.
      * @param condition

src/test/java/com/vaadin/flow/component/combobox/test/AutoFocusFilterIT.java

@@ -24,6 +24,7 @@
 
 @TestPath("auto-focus-filter")
 public class AutoFocusFilterIT extends AbstractComboBoxIT {
+
     @Before
     public void init() {
         open();

src/test/java/com/vaadin/flow/component/combobox/test/ClientSideFilterIT.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright 2000-2018 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.flow.component.combobox.test;
+
+import com.vaadin.flow.component.combobox.ComboBoxElementUpdated;
+import com.vaadin.flow.component.combobox.testbench.ComboBoxElement;
+import com.vaadin.flow.testutil.TestPath;
+import org.junit.Before;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.Keys;
+
+@TestPath("clientside-filter")
+public class ClientSideFilterIT extends AbstractComboBoxIT {
+    @Before
+    public void init() {
+        open();
+        waitUntil(driver -> findElements(By.tagName("vaadin-combo-box"))
+                .size() > 0);
+    }
+
+    @Test
+    public void filter_itemsShouldBeThere() {
+        // First combobox.
+        ComboBoxElement comboBox = $(ComboBoxElementUpdated.class).first();
+
+        comboBox.sendKeys("2");
+
+        waitForItems(comboBox, items -> items.size() == 1
+                && "Option 2".equals(getItemLabel(items, 0)));
+
+        comboBox.sendKeys(Keys.BACK_SPACE);
+
+        waitForItems(comboBox, items -> items.size() == 4);
+
+        comboBox.sendKeys("3");
+
+        waitForItems(comboBox, items -> items.size() == 1
+                && "Option 3".equals(getItemLabel(items, 0)));
+
+        // Second combobox.
+        comboBox = $(ComboBoxElementUpdated.class).get(1);
+
+        comboBox.sendKeys("mo");
+
+        waitForItems(comboBox,
+                items -> items.size() == 1
+                        && "Mozilla Firefox".equals(getItemLabel(items, 0)));
+
+        comboBox.closePopup();
+        comboBox.openPopup();
+
+        waitForItems(comboBox,
+                items -> items.size() == 5
+                        && "Google Chrome".equals(getItemLabel(items, 0))
+                        && "Mozilla Firefox".equals(getItemLabel(items, 1))
+                        && "Opera".equals(getItemLabel(items, 2))
+                        && "Apple Safari".equals(getItemLabel(items, 3))
+                        && "Microsoft Edge".equals(getItemLabel(items, 4)));
+
+    }
+
+}

src/test/java/com/vaadin/flow/component/combobox/test/ClientSideFilterPage.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright 2000-2018 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.flow.component.combobox.test;
+
+import com.vaadin.flow.component.combobox.ComboBox;
+import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.component.html.Hr;
+import com.vaadin.flow.router.Route;
+
+@Route("clientside-filter")
+public class ClientSideFilterPage extends Div {
+
+    public ClientSideFilterPage() {
+        ComboBox<String> cb = new ComboBox<>("Choose option", "Option 2",
+                "Option 3", "Option 4", "Option 5");
+        this.add(cb);
+        cb.focus();
+
+        this.add(new Hr());
+
+        ComboBox<String> testBox = new ComboBox<>("Browsers");
+        testBox.setItems("Google Chrome", "Mozilla Firefox", "Opera",
+                "Apple Safari", "Microsoft Edge");
+        this.add(testBox);
+
+    }
+}

src/test/java/com/vaadin/flow/component/combobox/test/SetItemsLaterIT.java

@@ -29,7 +29,7 @@
 import java.util.Map;
 
 @TestPath("set-items-later")
-public class SetItemsLaterIT extends AbstractComponentIT {
+public class SetItemsLaterIT extends AbstractComboBoxIT {
     @Before
     public void init() {
         open();
@@ -42,21 +42,17 @@ public void clickButton_comboBoxShouldContainsItems() {
         open();
         ComboBoxElement comboBox = $(ComboBoxElementUpdated.class).first();
 
-        List<Map<String, ?>> items = (List<Map<String, ?>>) executeScript(
-                "return arguments[0].filteredItems", comboBox);
-
-        Assert.assertNull("Items must be null.", items);
+        waitForItems(comboBox, items -> items == null);
 
         WebElement button = findElement(By.tagName("button"));
         button.click();
 
-        items = (List<Map<String, ?>>) executeScript(
-                "return arguments[0].filteredItems", comboBox);
-
-        Assert.assertNotNull("Items must not be null.", items);
+        comboBox.openPopup();
 
-        Assert.assertEquals("ComboBox must contain 2 items.",
-                2, items.size());
+        waitForItems(comboBox, items -> items != null && items.size() == 2
+                && "foo".equals(getItemLabel(items, 0))
+                && "bar".equals(getItemLabel(items, 1))
+        );
 
     }