-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[DataGrid] Add "does not equal" and "does not contain" filter operators #14489
[DataGrid] Add "does not equal" and "does not contain" filter operators #14489
Conversation
Localization writing tips ✍️Seems you are updating localization 🌍 files. Thank you for contributing to the localization! 🎉 To make your PR perfect, here is a list of elements to check: ✔️
Deploy preview: https://deploy-preview-14489--material-ui-x.netlify.app/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made that change a1e27b8
This is likely an issue for some translated operator labels too, but not sure of the best way to account for those right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is likely an issue for some translated operator labels too, but not sure of the best way to account for those right now.
Agreed, I think we should be mindful of this when finalizing the new filter panel design.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
Thanks for doing the benchmarks, it looks like having explicit filter operators for negations is a better option for an average user 👍🏻
const filterRegex = new RegExp(escapeRegExp(filterItemValue), 'i'); | ||
return (value): boolean => { | ||
return value != null ? !filterRegex.test(String(value)) : true; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great to extract a function that will handle both contains
and doesNotContain
operators since the only difference is the opposite return value.
Same for the other two negated operators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense - will create a follow up PR for this improvement 👍
Adds "does not equal" and "does not contain" filter operators.
Some UX benchmarking was done to see whether there should be a separate negation modifier for the "does not" filters. In this case, it made sense to follow the filtering experience of similar tools and list the "does not" filters in the same field. See Figma.
Fixes #13300