-
Notifications
You must be signed in to change notification settings - Fork 264
v0.29: filter
updates
#1869
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
v0.29: filter
updates
#1869
Conversation
Co-authored-by: Clémentine Urquizar - curqui <[email protected]>
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.
Thanks :-) I had some minor nitpicks but I think it is well explained!
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.
perfect 👍 thanks :-)
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.
- Since we have time, do you think we should restructure the "Filter expressions" section?
- Instead of listing each operator as a bullet, we could convert it to a sub-heading with an example, like Loïc has done in the spec. It will make it a lot easier to explain each filter and it looks a lot more organised
- We should update the example (
/learn/advanced/filtering_and_faceted_search.html#example-2
) to add theEXISTS
andIN
operators
Co-authored-by: Maryam <[email protected]>
Thanks for the feedback, @maryamsulemani97—as we discussed over slack, your suggestions are great but the current structure for the operators doesn't really support adding exhaustive information. We will review the whole filters guide soon (#1902) and keep the current version, warts and all, for the release. |
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.
🛸
bors merge |
Build succeeded: |
Closes #1853