New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Table] MatTableDataSource does not execute filter predicate if filter evaluates to false #9967
Comments
We have a problem with that too, but a little bit different. We want to filter by couple of fields. We have made custom predicate, to provide an object with couple of fields for filtering. But filter should be a string and we have building error. |
Might be better to just filter the data and not use the built in filtering functionality |
Somewhat related, |
i worked around it this way (ugly) const currentFilter = this.tableDataSource.filter;
this.tableDataSource.filter = '------';
this.tableDataSource.filter = currentFilter; |
Not worked for me |
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 angular#19092. Fixes angular#9967.
If changing the handling for the falsy values (e.g. empty string) is not feasible. How about introducing a method to trigger filtering explicitly, no matter |
+1 to this. I have a situation where I am doing a dual-filter on a table. The table shows log entries, which have a bunch of string & number fields, as well as a enum field for the log level type. The user can enter a text filter in an input and can select a minimum level to display. I am setting the datasource filter to the value of the text box: and then also overriding the filter predicate to check that the logs have at least the minimum specified log level:
This works fine if the user enters any text in the filter box, which then triggers the filterPredicate to run. But if there is no text in the filter value, then the filterPredicate does not run at all, which is not desired. It took me about an hour of debugging and searching to realize that the filterPredicate does not run if the filter value is set to a falsey value. You could go 3 ways with this:
I'd like 2 or 3, but can live with any of these. |
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.
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)
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)
Hi @annieyw , thanks for tagging this issue. I like your PR and think it's a good improvement. Maybe I'm reading the PR wrong, but it does not seem to completely address this issue. You changed the null check from: to: If a custom filterPredicate is provided that relies on more than the filter string (or does not rely on the filter string at all), the filterPredicate will still not be run even after your PR. As I mentioned in my comment above, I see 3 different paths forward and would be happy to discuss further! I would even be willing to contribute a PR. |
@crisbeto Can you take another look at this? |
@s4m0r4m4 I think that your second proposal is reasonable (moving the null check inside the predicate), but I suspect that doing so at this point will be a breaking change for some apps. Also it's worth noting this part of the docs:
Since we can't realistically handle all of the different use cases out there, my fix in #19094 tried to provide a reasonable default. |
Yes I agree this would be a breaking change. You make a reasonable point (about it being a simple data source, not necessarily intended for complex setups). As per option 1 above, I'd be ok if you wanted to leave it that way and just clarify in the docs how the filterPredicate behaves, though I do think that the behavior in Option 2 feels more intuitive to me personally. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
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 angular#19092. Fixes angular#9967.
Bug, feature request, or proposal:
Bug
What is the expected behavior?
Filter text should not be required for using a filter when a custom filterPredicate is supplied
What is the current behavior?
Filter text must be truthy in order to use a filterPredicate which does not require filter text
What are the steps to reproduce?
Make a filterPredicate which does not depend on the filter value
What is the use-case or motivation for changing an existing behavior?
Filter predicates which do not require the filter text (for example, checking that a value in the data is past the current date)
Which versions of Angular, Material, OS, TypeScript, browsers are affected?
Current
Is there anything else we should know?
https://github.com/angular/material2/blob/master/src/lib/table/table-data-source.ts#L213
!this.filter ? data : data.filter(obj => this.filterPredicate(obj, this.filter));
Instead of
!this.filter ?
check if a filterPredicate has been suppliedThe text was updated successfully, but these errors were encountered: