-
Notifications
You must be signed in to change notification settings - Fork 155
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
feat: Property filter enum tokens #2739
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2739 +/- ##
=======================================
Coverage 96.21% 96.22%
=======================================
Files 761 761
Lines 21502 21584 +82
Branches 7354 7397 +43
=======================================
+ Hits 20688 20769 +81
+ Misses 806 762 -44
- Partials 8 53 +45 ☔ View full report in Codecov by Sentry. |
9e32565
to
ec57f06
Compare
ec57f06
to
af7c2bf
Compare
af7c2bf
to
128fbd4
Compare
128fbd4
to
9853c8a
Compare
9853c8a
to
085c7b9
Compare
f592e4f
to
d16f901
Compare
d16f901
to
3d8604c
Compare
3d8604c
to
680473e
Compare
9b9c3dd
to
a8f6987
Compare
0fbd924
to
10eff20
Compare
10eff20
to
f0b493a
Compare
@@ -12636,7 +12636,7 @@ The \`operation\` property has two valid values: "and", "or", and controls the j | |||
The \`tokens\` property is an array of objects that will be displayed to the user beneath the filtering input. When \`enableTokenGroups=true\`, the | |||
\`tokenGroups\` property is used instead, which supports nested tokens. | |||
Each token has the following properties: | |||
* value [string]: The string value of the token to be used as a filter. | |||
* value [unknown]: The value of the token to be used as a filter. Can be null or string for default tokens, string[] for enum tokens, and anything for tokens with custom forms. |
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.
Just to double-check that this API change is OK (the doc text update suggests yes), and this will not cause issues to existing customers (do we want to run a dry run?)
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.
I did make a dry-run for the type update in collection hooks. In this PR no type was updated, but will do a dry-run as well, just in case.
The value has been essentially unknown since introduction of custom forms. The current PR should have no impact to the existing customers but those making something with the query and wanting to adopt the feature would need to ensure the type is correctly handled at runtime.
114af6e
to
dd8a730
Compare
Description
Adds first-class support for multi-choice (enum) tokens in property filter.
Rel: [Tz3OAr3i2ni1], [0aVwAjMOimcN]
Depends on:
How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md
.CONTRIBUTING.md
.Security
checkSafeUrl
function.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.