fix: hide already-filtered values from other spreadsheet column filters (#9386) (CP: 25.2)

vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/pom.xml

@@ -47,6 +47,12 @@
             <artifactId>vaadin-checkbox-flow</artifactId>
             <version>${project.version}</version>
         </dependency>
+        <dependency>
+            <groupId>com.vaadin</groupId>
+            <artifactId>vaadin-checkbox-testbench</artifactId>
+            <version>${project.version}</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>com.vaadin</groupId>
             <artifactId>vaadin-combo-box-flow</artifactId>

vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/SpreadsheetTableFixture.java

@@ -18,20 +18,23 @@ public class SpreadsheetTableFixture implements SpreadsheetFixture {
 
     @Override
     public void loadFixture(Spreadsheet spreadsheet) {
-        int maxColumns = 5;
-        int maxRows = 5;
+        int rows = 5; // 1 header row + 4 data rows
+        int columns = 5;
+        int firstRow = 1;
+        int firstColumn = 1;
 
-        for (int column = 1; column < maxColumns + 1; column++) {
-            spreadsheet.createCell(1, column, "Column " + column);
-        }
-
-        for (int row = 2; row < maxRows + 2; row++) {
-            for (int col = 1; col < maxColumns + 1; col++) {
-                spreadsheet.createCell(row, col, row + col);
+        // Label every cell with its position within the table, e.g. "Cell 0:0".
+        // This keeps each value distinct and makes it obvious which row and
+        // column a filter value belongs to.
+        for (int row = 0; row < rows; row++) {
+            for (int col = 0; col < columns; col++) {
+                spreadsheet.createCell(firstRow + row, firstColumn + col,
+                        "Cell " + row + ":" + col);
             }
         }
-        CellRangeAddress range = new CellRangeAddress(1, maxRows, 1,
-                maxColumns);
+
+        CellRangeAddress range = new CellRangeAddress(firstRow,
+                firstRow + rows - 1, firstColumn, firstColumn + columns - 1);
         SpreadsheetTable table = new SpreadsheetFilterTable(spreadsheet, range);
         spreadsheet.registerTable(table);
         spreadsheet.refreshAllCellValues();

vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/test/java/com/vaadin/flow/component/spreadsheet/test/SheetFilterTableIT.java

@@ -10,10 +10,14 @@
 
 import static org.junit.Assert.assertFalse;
 
+import java.util.List;
+
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
+import org.openqa.selenium.By;
 
+import com.vaadin.flow.component.checkbox.testbench.CheckboxGroupElement;
 import com.vaadin.flow.component.spreadsheet.testbench.SheetCellElement;
 import com.vaadin.flow.component.spreadsheet.testbench.SpreadsheetElement;
 import com.vaadin.flow.component.spreadsheet.tests.fixtures.TestFixtures;
@@ -61,8 +65,68 @@ public void sheetWithFilterTable_rowIsRemoved_filterOptionsAvailable() {
         assertSelectAll(cell);
     }
 
+    @Test
+    public void filterColumn_otherColumnOmitsValuesOfHiddenRows() {
+        loadTestFixture(TestFixtures.SpreadsheetTable);
+        final SpreadsheetElement spreadsheet = getSpreadsheet();
+
+        // Before filtering, the first column offers all of its values
+        spreadsheet.getCellAt("B2").popupButtonClick();
+        Assert.assertEquals(
+                List.of("Cell 1:0", "Cell 2:0", "Cell 3:0", "Cell 4:0"),
+                getFilterPopup().getOptions());
+        closeFilterPopup();
+
+        // Filter the second column so that row 1 gets hidden
+        spreadsheet.getCellAt("C2").popupButtonClick();
+        getFilterPopup().deselectByText("Cell 1:1");
+        closeFilterPopup();
+
+        // The first column no longer offers "Cell 1:0", as its row is hidden
+        // by the second column
+        spreadsheet.getCellAt("B2").popupButtonClick();
+        Assert.assertEquals(List.of("Cell 2:0", "Cell 3:0", "Cell 4:0"),
+                getFilterPopup().getOptions());
+    }
+
+    @Test
+    public void filterTwoColumns_eachColumnRetainsOwnFilteredValues() {
+        loadTestFixture(TestFixtures.SpreadsheetTable);
+        final SpreadsheetElement spreadsheet = getSpreadsheet();
+
+        // Filter the first column so that row 1 gets hidden
+        spreadsheet.getCellAt("B2").popupButtonClick();
+        getFilterPopup().deselectByText("Cell 1:0");
+        closeFilterPopup();
+
+        // Filter the second column so that row 2 gets hidden
+        spreadsheet.getCellAt("C2").popupButtonClick();
+        getFilterPopup().deselectByText("Cell 2:1");
+        closeFilterPopup();
+
+        // The first column still offers "Cell 1:0" as an unchecked option, so
+        // its hidden row can be brought back independently of the second
+        // column's filter. Row 2, hidden by the second column, is excluded.
+        spreadsheet.getCellAt("B2").popupButtonClick();
+        Assert.assertEquals(List.of("Cell 1:0", "Cell 3:0", "Cell 4:0"),
+                getFilterPopup().getOptions());
+        Assert.assertEquals(List.of("Cell 3:0", "Cell 4:0"),
+                getFilterPopup().getSelectedTexts());
+    }
+
     private void assertSelectAll(SheetCellElement cell) {
         cell.popupButtonClick();
         Assert.assertTrue(hasOption("(Select All)"));
     }
+
+    private CheckboxGroupElement getFilterPopup() {
+        return $(CheckboxGroupElement.class).single();
+    }
+
+    private void closeFilterPopup() {
+        findElement(By.className("v-window-closebox")).click();
+        // The overlay fades out, so wait until it is actually gone before
+        // opening the next one, otherwise single() would match two overlays.
+        waitUntil(driver -> $(CheckboxGroupElement.class).all().isEmpty());
+    }
 }

vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/ItemFilter.java

@@ -251,32 +251,37 @@ public void updateOptions() {
     }
 
     /**
-     * Gets the currently NOT filtered cell values.
+     * Gets the cell values of this column that are currently visible, i.e.,
+     * excluding any hidden by this column's filter or whose rows are already
+     * hidden by other columns' filters.
      *
-     * @return All unique values currently visible (= not filtered) within this
-     *         column
+     * @return the cell values
      */
     protected Set<String> getVisibleValues() {
-        Set<String> values = new HashSet<>();
-        for (int r = filterRange.getFirstRow(); r <= filterRange
-                .getLastRow(); r++) {
-            if (!filteredRows.contains(r) && !spreadsheet.isRowHidden(r)) {
-                values.add(spreadsheet.getCellValue(
-                        spreadsheet.getCell(r, filterRange.getFirstColumn())));
-            }
+        Set<String> values = getAllValues();
+        for (int r : filteredRows) {
+            values.remove(spreadsheet.getCellValue(
+                    spreadsheet.getCell(r, filterRange.getFirstColumn())));
         }
         return values;
     }
 
     /**
-     * Gets all of the unique values for this filter column.
+     * Gets the cell values of this column, excluding those whose rows are
+     * already hidden by other columns' filters.
      *
-     * @return All unique values within this column
+     * @return the cell values
      */
     protected Set<String> getAllValues() {
+        Set<Integer> otherHidden = filterTable
+                .getRowsHiddenByOtherFilters(this);
+
         Set<String> values = new HashSet<>();
         for (int r = filterRange.getFirstRow(); r <= filterRange
                 .getLastRow(); r++) {
+            if (otherHidden.contains(r)) {
+                continue;
+            }
             values.add(spreadsheet.getCellValue(
                     spreadsheet.getCell(r, filterRange.getFirstColumn())));
         }
@@ -290,9 +295,15 @@ protected Set<String> getAllValues() {
      *            the values that are NOT filtered
      */
     protected void updateFilteredItems(Collection<String> visibleValues) {
+        Set<Integer> otherHidden = filterTable
+                .getRowsHiddenByOtherFilters(this);
+
         filteredRows.clear();
         for (int r = filterRange.getFirstRow(); r <= filterRange
                 .getLastRow(); r++) {
+            if (otherHidden.contains(r)) {
+                continue;
+            }
             String cellValue = spreadsheet.getCellValue(
                     spreadsheet.getCell(r, filterRange.getFirstColumn()));
             if (!visibleValues.contains(cellValue)) {

vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/SpreadsheetFilterTable.java

@@ -151,6 +151,17 @@ public void clearAllFilters() {
             popupButtonToClearButtonMap.get(popupButton).setEnabled(false);
             popupButton.markActive(false);
         }
+
+        // Refresh option lists after all filters cleared, so each ItemFilter
+        // sees the now-empty filteredRows of its siblings.
+        for (var filters : popupButtonToFiltersMap.values()) {
+            for (var filter : filters) {
+                if (filter instanceof ItemFilter itemFilter) {
+                    itemFilter.updateOptions();
+                }
+            }
+        }
+
         getSpreadsheet().setRowsHidden(IntStream
                 .range(filteringRegion.getFirstRow(),
                         filteringRegion.getLastRow() + 1)
@@ -262,6 +273,27 @@ public void onFiltersUpdated() {
                         filteredRows::contains)));
     }
 
+    /**
+     * Gets the union of rows hidden by all filters in this table except the
+     * given one.
+     *
+     * @param self
+     *            Filter to exclude from the union
+     * @return Rows hidden by other filters
+     */
+    Set<Integer> getRowsHiddenByOtherFilters(SpreadsheetFilter self) {
+        Set<Integer> hidden = new HashSet<>();
+        for (HashSet<SpreadsheetFilter> filters : popupButtonToFiltersMap
+                .values()) {
+            for (SpreadsheetFilter filter : filters) {
+                if (!filter.equals(self)) {
+                    hidden.addAll(filter.getFilteredRows());
+                }
+            }
+        }
+        return hidden;
+    }
+
     /**
      * Registers a new filter to this filter table and adds it inside the given
      * pop-up button.