-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
fix(unified-search): prevent provider disabling on content filter apply #56620
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
Conversation
|
Not checked now but is it communicated to user? |
IssueWhen users apply date/person filters in unified search, these filters only work on providers that support them. Other providers still return unfiltered results (Previously we tried to avoid sending the search query to these providers at all and this PR will not remove that behavior). Example: Search "photo" with "Last 7 days" filter
QuestionWhat's the expected behavior? A) Current: Mixed results B) Hide incompatible C) Visual separation Any recommendation from @nextcloud/designers Cc: @kra-mo |
Yes man you are on track! I was going to add a comment for designers, my initial solution was not to send these queries at all, it seems that might be problematic because the filter is an ad-on to the SEARCH QUERY. So even though filters are meant to REDUCE results that are already VALID for the SEARCH QUERY in this case we are preventing a search completely for unsupported filters which makes it seem as though there are not results for the SEARCH QUERY to start with. |
C is probably the best. But I don't think they need to be separated that clearly given it'd probably be pretty confusing. I'd just show filtered results first, then unfiltered below. |
2f21663 to
36f7d65
Compare
2b70e1f to
c60db78
Compare
jancborchardt
left a 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.
I’d agree with @kra-mo that C) is probably best, given the current situation.
But in the long run, can’t we require from search providers that they support these filters? E.g. date should be supported by all, as it’s relevant for all. Same with people, if it’s shared with them.
|
@nfebe Not sure what you're asking/what the actual difference is there. Could you clarify? Regardless, the → arrow icons seem weird before the app name. Could the actual icon be shown there? If not, could the arrows at least be made smaller? And the wording could be trimmed down significantly: Additional results Is quite long, and probably incorrect. It could just be a single line: Partial matches |
c60db78 to
f70da6c
Compare
It's just an preview of your implemented recommendation for you feedback. Not a question.
That is unrelated I will send a seperate PR and even app priorities as most users are never searching for apps, some apps results must be shown first.
Done. |
f70da6c to
be61ebd
Compare
|
/compile |
f3f6a9f to
460d335
Compare
When date range or person filters were applied, providers that didn't support these filters were automatically disabled in the UI. This made the in-folder filter appear auto-applied and prevented users from searching non-compatible providers. Remove automatic provider disabling logic from updateDateFilter(), applyPersonFilter(), and removeFilter(). Content filters now apply only to compatible providers via existing compatibility checks while keeping all providers available for selection. Signed-off-by: nfebe <[email protected]>
Show results from providers that don't support active content filters (date/person) in a separate "Additional results" section with a note explaining that some filters may have been ignored. Changes: - Add computed properties to separate filtered/unfiltered results - Track filter compatibility using baseProvider for searchFrom providers - Deduplicate results by resourceUrl across sections - Skip in-folder results when at root to avoid duplicating Files results - Fix providerIsCompatibleWithFilters to check correct filter properties - Add styling for the unfiltered results section Signed-off-by: nfebe <[email protected]>
460d335 to
f035ff3
Compare
|
/compile |
Signed-off-by: nextcloud-command <[email protected]>
|
/backport to stable32 |
|
/backport to stable31 |
|
/backport to stable30 |
@nfebe can we keep this as a follow-up too? |


When date range or person filters were applied, providers that didn't support these filters were automatically disabled in the UI. This made the in-folder filter appear auto-applied and prevented users from searching non-compatible providers.
Remove automatic provider disabling logic from
updateDateFilter(),applyPersonFilter(), andremoveFilter(). Content filters now apply only to compatible providers via existing compatibility checks while keeping all providers available for selection.