-
-
Notifications
You must be signed in to change notification settings - Fork 835
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: revamp search #3893
feat: revamp search #3893
Conversation
* refactor: move gambits to frontend * Apply fixes from StyleCI * test: GambitManager
* chore: drop remaining backend regex gambits * refactor: merge filterer & searcher concept * refactor: adapt extenders * refactor: no longer need to push gambits to `q` * refactor: filters to gambits * refactor: drop shred `Query` namespace * Apply fixes from StyleCI * chore: cleanup * Apply fixes from StyleCI
* chore: leftover gambit references on the backend * chore: namespace
* feat: first iteration of search drivers * feat: indexer API & tweaks * feat: changes after POC driver * fix: properly fire custom observables * chore: remove debugging code * fix: phpstan * fix: custom eloquent events * chore: drop POC usage * test: indexer extender API * fix: extension searcher fails without filters * fix: phpstan * fix: frontend created gambit
# Conflicts: # framework/core/src/Settings/SettingsServiceProvider.php
* feat: allow accessing total search results * feat: allow replacing filters * chore: phpstan
Feel free to leave separate reviews in each PR for easier back n fourth if necessary. |
Also, proof of concept search driver here: #3902 (comment) |
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.
Aside from it likely taking me a while to get used to, I really do like this alot. Great job @SychO9 :)
Just need to resolve the conflict, then good to marge. Are you able to tackle that @SychO9 ? |
# Conflicts: # framework/core/src/Search/GambitManager.php
Closes #3884
Changes proposed in this pull request:
Part of the roadmap, each commit has a separate PR. Changes are made one at a time for easier review/readability.
feel free to review within the related PR even if it's marked as merged.
Necessity
Confirmed
composer test
).Required changes: