fix: Force client-side filter update if it's not empty

Since the DataCommunicator erases the combo box non-empty filter after each request, the filter needs to be set again to DataCommunicator even if it's the same as last sent. Fixes: #388
vaadin · Nov 17, 2020 · f6d8da3 · f6d8da3
1 parent 7fa86ce
commit f6d8da3
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 1 deletion.
diff --git a/...ntegration-tests/src/main/java/com/vaadin/flow/component/combobox/test/FilteringPage.java b/...ntegration-tests/src/main/java/com/vaadin/flow/component/combobox/test/FilteringPage.java
@@ -20,6 +20,7 @@
 
 import com.vaadin.flow.component.combobox.ComboBox;
 import com.vaadin.flow.component.combobox.ComboBox.ItemFilter;
+import com.vaadin.flow.component.combobox.dataview.ComboBoxLazyDataView;
 import com.vaadin.flow.component.html.Div;
 import com.vaadin.flow.component.html.NativeButton;
 import com.vaadin.flow.data.provider.CallbackDataProvider;
@@ -107,6 +108,39 @@ public FilteringPage() {
                 filter -> filter.isEmpty() ? 0 : 1);
         comboBoxWithEmptyFilterReturnsNone.setId("empty-filter-returns-none");
         add(new Div(), comboBoxWithEmptyFilterReturnsNone);
+
+        createComboBoxWithMultiplePagesAndSourceSwitchers();
+    }
+
+    private void createComboBoxWithMultiplePagesAndSourceSwitchers() {
+        // addTitle("Callback data provider with custom page size 42");
+        ComboBox<String> comboBox = new ComboBox<>();
+        comboBox.setId("combo-box-with-filtered-items");
+
+        List<String> items = LazyLoadingPage.generateStrings(500);
+
+        ComboBoxLazyDataView<String> dataView = comboBox
+                .setItems(
+                        query -> items.stream()
+                                .filter(item -> item
+                                        .contains(query.getFilter().orElse("")))
+                                .skip(query.getOffset())
+                                .limit(query.getLimit()),
+                        query -> (int) items.stream()
+                                .filter(item -> item
+                                        .contains(query.getFilter().orElse("")))
+                                .count());
+
+        NativeButton switchToUnknown = new NativeButton(
+                "Switch To Unknown Item Count",
+                click -> dataView.setItemCountUnknown());
+        switchToUnknown.setId("switch-to-unknown-item-count");
+
+        NativeButton switchToInMemory = new NativeButton(
+                "Switch To In-memory Items", click -> comboBox.setItems(items));
+        switchToInMemory.setId("switch-to-in-memory-items");
+
+        add(new Div(), comboBox, switchToUnknown, switchToInMemory);
     }
 
 }
diff --git a/...-integration-tests/src/test/java/com/vaadin/flow/component/combobox/test/FilteringIT.java b/...-integration-tests/src/test/java/com/vaadin/flow/component/combobox/test/FilteringIT.java
@@ -30,13 +30,17 @@
 public class FilteringIT extends AbstractComboBoxIT {
 
     private ComboBoxElement box;
+    private ComboBoxElement comboBoxWithFilteredItems;
 
     @Before
     public void init() {
         open();
         waitUntil(driver -> findElements(By.tagName("vaadin-combo-box"))
                 .size() > 0);
         box = $(ComboBoxElement.class).first();
+
+        comboBoxWithFilteredItems = $(ComboBoxElement.class)
+                .id("combo-box-with-filtered-items");
     }
 
     @Test
@@ -165,6 +169,61 @@ public void configureEmptyFilterToReturnNoItems_useCaseWorks() {
         assertItemsNotLoaded();
     }
 
+    @Test
+    public void unknownItemCountLazyLoadingFiltering_applyFilter_allPagesContainFilteredItems() {
+        clickButton("switch-to-unknown-item-count");
+        verifyFilteredItems(comboBoxWithFilteredItems);
+    }
+
+    @Test
+    public void inMemoryItemsFiltering_applyFilter_allPagesContainFilteredItems() {
+        clickButton("switch-to-in-memory-items");
+        verifyFilteredItems(comboBoxWithFilteredItems);
+    }
+
+    @Test
+    public void definedItemCountLazyLoadingFiltering_applyFilter_allPagesContainFilteredItems() {
+        verifyFilteredItems(comboBoxWithFilteredItems);
+    }
+
+    private void verifyFilteredItems(ComboBoxElement comboBoxElement) {
+        comboBoxElement.openPopup();
+        comboBoxElement.setFilter("1");
+
+        // Verify items on page 1
+        waitForItems(comboBoxElement,
+                items -> "Item 1".equals(getItemLabel(items, 0))
+                        && "Item 10".equals(getItemLabel(items, 1))
+                        && "Item 130".equals(getItemLabel(items, 49)));
+
+        scrollToItem(comboBoxElement, 49);
+
+        // Verify items on page 2
+        waitForItems(comboBoxElement,
+                items -> "Item 131".equals(getItemLabel(items, 50))
+                        && "Item 180".equals(getItemLabel(items, 99)));
+
+        scrollToItem(comboBoxElement, 99);
+
+        // Verify items on page 3
+        waitForItems(comboBoxElement,
+                items -> "Item 181".equals(getItemLabel(items, 100))
+                        && "Item 321".equals(getItemLabel(items, 149)));
+
+        scrollToItem(comboBoxElement, 175);
+
+        // Verify items on page 4
+        waitForItems(comboBoxElement,
+                items -> "Item 331".equals(getItemLabel(items, 150))
+                        && "Item 491".equals(getItemLabel(items, 175)));
+
+        // filtered items: 1, 10, 11, .. 19, 21, 31, .. 91, 100, 101, .. 199,
+        // 201, 210, .. 291, 301, .. 391, 401, .. 491. Total count = 176
+        assertLoadedItemsCount(
+                "Unexpected items count after applying filter = '1'", 176,
+                comboBoxElement);
+    }
+
     private void assertItemsNotLoaded() {
         try {
             waitUntil(driver -> {

diff --git a/...rent/vaadin-combo-box-flow/src/main/java/com/vaadin/flow/component/combobox/ComboBox.java b/...rent/vaadin-combo-box-flow/src/main/java/com/vaadin/flow/component/combobox/ComboBox.java
@@ -876,7 +876,12 @@ public <C> void setDataProvider(DataProvider<T, C> dataProvider,
                         convertOrNull.apply(getFilterString()), false);
 
         filterSlot = filter -> {
-            if (!Objects.equals(filter, lastFilter)) {
+            if (!Objects.equals(filter, lastFilter) ||
+                    // Since the data communicator erases the combo box
+                    // non-empty filter after each request, the filter needs
+                    // to be set again to data communicator even if the same
+                    // as last sent.
+                    !filter.isEmpty()) {
                 DataCommunicator.Filter<C> objectFilter =
                         new DataCommunicator.Filter<C>(
                                 convertOrNull.apply(filter), filter.isEmpty());