-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feature #5948 Add mode to search all/any terms #5949
Conversation
916436b
to
55b6417
Compare
IMO the default search mode should be Must I set ALL_TERMS as default? |
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.
If I understand your PR correctly, ALL_TERMS
would be correct search mode by default. Correct me if I am wrong. It means: Condition A and Condition B must match in order to display. Correct? If yes, I'd second your opinion here. Otherwise, maybe the owner wants to have the last word here. 🤔
Yes, it is. When you type once word ANY_TERMS and ALL_TERMS are identical. According my documentation: $crud
// match any terms (default mode)
// term1 in (field1 or field2) or term2 in (field1 or field2)
->setSearchMode(SearchMode::ANY_TERMS)
// force to match all the terms
// term1 in (field1 or field2) and term2 in (field1 or field2)
->setSearchMode(SearchMode::ALL_TERMS)
; |
Then I absolutely agree with you. 👍 Thank you very much for explaining it once again. |
1e5ea0c
to
f542df6
Compare
I just pushed a new commit to set ALL_TERMS as default. I've kept it as a separate commit in case the owner wants to use ANY_TERMS as default like now. |
f542df6
to
636329b
Compare
This is finally merged! Roland, thanks for this contribution. I played a bit with it and it worked nicely. I hope it's useful for other people too. Cheers! |
Follow up to #5948
This PR add a searchMode option in Crud to set if the search is an AND or OR operator between terms.