Skip to content

Commit

Permalink
fix(table): filter predicate not called for falsy values (#19094)
Browse files Browse the repository at this point in the history
We had a simple falsy check to determine whether to call the filter predicate. I'm guessing it was written this way so that if somebody were to clear a filter input, all items will be shown. This seems a bit too loose since it could catch things like 0 which technically aren't allowed due to the `string` type, but could still end up inside the data source if values are proxied in directly through the view.

Ideally we'd just call the predicate for all values and let it decide whether or not to filter, but I left in special cases for undefined, null and empty strings, because I expect a lot of apps to be broken if we were to change the behavior.

Fixes #19092.
Fixes #9967.

(cherry picked from commit 6ec1551)
  • Loading branch information
crisbeto authored and annieyw committed Nov 7, 2020
1 parent fab6880 commit acd8e74
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
8 changes: 8 additions & 0 deletions src/material-experimental/mdc-table/table.spec.ts
Expand Up @@ -352,6 +352,14 @@ describe('MDC-based MatTable', () => {
['a_2', 'b_2', 'c_2'],
['Footer A', 'Footer B', 'Footer C'],
]);

// Change the filter to a falsy value that might come in from the view.
dataSource.filter = 0 as any;
fixture.detectChanges();
expectTableToMatchContent(tableElement, [
['Column A', 'Column B', 'Column C'],
['Footer A', 'Footer B', 'Footer C'],
]);
}));

it('should not match concatenated words', fakeAsync(() => {
Expand Down
4 changes: 2 additions & 2 deletions src/material/table/table-data-source.ts
Expand Up @@ -261,8 +261,8 @@ export class MatTableDataSource<T> extends DataSource<T> {
// If there is a filter string, filter out data that does not contain it.
// Each data object is converted to a string using the function defined by filterTermAccessor.
// May be overridden for customization.
this.filteredData =
!this.filter ? data : data.filter(obj => this.filterPredicate(obj, this.filter));
this.filteredData = (this.filter == null || this.filter === '') ? data :
data.filter(obj => this.filterPredicate(obj, this.filter));

if (this.paginator) { this._updatePaginator(this.filteredData.length); }

Expand Down
8 changes: 8 additions & 0 deletions src/material/table/table.spec.ts
Expand Up @@ -353,6 +353,14 @@ describe('MatTable', () => {
['a_2', 'b_2', 'c_2'],
['Footer A', 'Footer B', 'Footer C'],
]);

// Change the filter to a falsy value that might come in from the view.
dataSource.filter = 0 as any;
fixture.detectChanges();
expectTableToMatchContent(tableElement, [
['Column A', 'Column B', 'Column C'],
['Footer A', 'Footer B', 'Footer C'],
]);
}));

it('should not match concatenated words', fakeAsync(() => {
Expand Down

0 comments on commit acd8e74

Please sign in to comment.