Skip to content

Commit

Permalink
fix(filters): new filterPredicate shouldn't break other column filt…
Browse files Browse the repository at this point in the history
…ers (#1531)

* fix(filters): new `filterPredicate` shouldn't break other column filters
- the previous PR to implement `filterPredicate` that was not caught originally which had the indirect effect of breaking the other filter columns, the issue was caused by the fact that calling a `return` within the `for` loop of all filters was cancelling the next filters because a `return` breaks the loop.
- So the fix is to only call `return` when the `filterPredicate` returns `false` which mean that at point the row data context is officially filtered out, so not inspecting further filters make sense

* fix(demo): improve SQL LIKE with filter of `Ta%30%`
- `Ta%30%` wasn't working correctly before, it should be equivalent to: StartsWith "Ta" and Contains "30" anywhere
  • Loading branch information
ghiscoding authored May 16, 2024
1 parent 828eb8a commit 27777ef
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 3 deletions.
7 changes: 5 additions & 2 deletions examples/vite-demo-vanilla-bundle/src/examples/example14.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,17 @@ export default class Example14 {
if (searchVals?.length) {
const columnId = searchFilterArgs.columnId;
const searchVal = searchVals[0] as string;
const results = searchVal.matchAll(/^%([^%\r\n]+)[^%\r\n]*$|%(.+)%(.*)|(.+)%(.+)|([^%\r\n]+)%$/gi);
const results = searchVal.matchAll(/^%([^%\r\n]+)[^%\r\n]*$|(.*)%(.+)%(.*)|(.+)%(.+)|([^%\r\n]+)%$/gi);
const arrayOfMatches = Array.from(results);
const matches = arrayOfMatches.length ? arrayOfMatches[0] : [];
const [_, endW, contain, containEndW, comboSW, comboEW, startW] = matches;
const [_, endW, containSW, contain, containEndW, comboSW, comboEW, startW] = matches;

if (endW) {
// example: "%001" ends with A
return dataContext[columnId].endsWith(endW);
} else if (containSW && contain) {
// example: "%Ti%001", contains A + ends with B
return dataContext[columnId].startsWith(containSW) && dataContext[columnId].includes(contain);
} else if (contain && containEndW) {
// example: "%Ti%001", contains A + ends with B
return dataContext[columnId].includes(contain) && dataContext[columnId].endsWith(containEndW);
Expand Down
6 changes: 5 additions & 1 deletion packages/common/src/services/filter.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,11 @@ export class FilterService {

// user could provide a custom filter predicate on the column definition
if (typeof columnFilterDef?.filterPredicate === 'function') {
return columnFilterDef.filterPredicate(item, searchColFilter);
const fpResult = columnFilterDef.filterPredicate(item, searchColFilter);
if (!fpResult) {
// only return on false, when row is filtered out and no further filter to be considered
return false;
}
} else {
// otherwise execute built-in filter condition checks
const conditionOptions = this.preProcessFilterConditionOnDataContext(item, searchColFilter, grid);
Expand Down
35 changes: 35 additions & 0 deletions test/cypress/e2e/example14.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,22 @@ describe('Example 14 - Columns Resize by Content', () => {
cy.get(`[style="top: ${GRID_ROW_HEIGHT * 3}px;"] > .slick-cell:nth(1)`).should('contain', 'Task 330');
});

it('should return 14 rows using "Ta%30%" (starts with "Ta" + ends with 30)', () => {
cy.get('.search-filter.filter-title')
.clear()
.type('Ta%30%');

cy.get('[data-test="total-items"]')
.should('contain', 4);

cy.get(`[style="top: ${GRID_ROW_HEIGHT * 0}px;"] > .slick-cell:nth(1)`).should('contain', 'Task 30');
cy.get(`[style="top: ${GRID_ROW_HEIGHT * 1}px;"] > .slick-cell:nth(1)`).should('contain', 'Task 130');
cy.get(`[style="top: ${GRID_ROW_HEIGHT * 2}px;"] > .slick-cell:nth(1)`).should('contain', 'Task 230');
cy.get(`[style="top: ${GRID_ROW_HEIGHT * 3}px;"] > .slick-cell:nth(1)`).should('contain', 'Task 300');
cy.get(`[style="top: ${GRID_ROW_HEIGHT * 4}px;"] > .slick-cell:nth(1)`).should('contain', 'Task 301');
cy.get(`[style="top: ${GRID_ROW_HEIGHT * 5}px;"] > .slick-cell:nth(1)`).should('contain', 'Task 302');
});

it('should return all 400 rows using "Ta%" (starts with "Ta")', () => {
cy.get('.search-filter.filter-title')
.clear()
Expand Down Expand Up @@ -373,5 +389,24 @@ describe('Example 14 - Columns Resize by Content', () => {
cy.get('[data-test="total-items"]')
.should('contain', 0);
});

it('return some rows (not all 400) filtering Title as "%ask%" AND a Duration ">50" to test few filters still working', () => {
cy.get('.search-filter.filter-title')
.clear()
.type('%ask%');

cy.get('[data-test="total-items"]')
.should('contain', 400);

cy.get('.search-filter.filter-duration')
.clear()
.type('>50');

cy.get('[data-test="total-items"]')
.should('not.contain', 0);

cy.get('[data-test="total-items"]')
.should('not.contain', 400);
});
});
});

0 comments on commit 27777ef

Please sign in to comment.