-
-
Notifications
You must be signed in to change notification settings - Fork 4.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
Add app-based filtering to global search #41669
Conversation
82577ee
to
b430abb
Compare
/compile amend / |
b430abb
to
a26d1df
Compare
/backport to stable28 |
a26d1df
to
b0e28a8
Compare
/compile amend / |
Emit search queries from the global search modal that would trigger list filtering in various apps that support it. Signed-off-by: fenn-cs <[email protected]> Signed-off-by: nextcloud-command <[email protected]>
b0e28a8
to
a67905a
Compare
From a UX perspective i feel this is the wrong approach to filter the app from within the modal, so I hope #41609 is also considered soonish. |
Well, I have already over-stated this point, especially after implementing and interacting with this. The priority communicated is that we first restore the "original" behavior which is what this PR is about. So I hope, we undo this asap and improve on what Marco has proposed already. I think even in its current state that's way more decent and at least we have the toggle for users to just go back to the old search. |
@@ -227,6 +238,9 @@ export default { | |||
this.results = [] | |||
return | |||
} | |||
if (this.supportFiltering()) { | |||
emit('nextcloud:unified-search.search', { query }) |
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.
On second thought, why restrict?
If the app doesn't subscribe to the event, it will not do anything right?
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.
- For the very reason you stated, emitting it does not do anything so, no need to emit it when it's not needed.
- Code clarity, it makes it clear why, where and when the event needs to be emitted, otherwise it could pass that it is just another step in the search procedure
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.
no need to emit it when it's not needed.
I think we don't know that. Other applications out there could use this mechanism.
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.
Yep, would agree, especially for community apps we don't maintain or developers working on that I'd vote for always emitting.
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.
Would open a follow up PR then!
This partially solves #41484 |
Emit search queries from the global search modal that would trigger list filtering in various apps that support it.