Skip to content

Add app-based filtering to global search #41669

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.

Sign up for GitHub

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

Merged
AndyScherzinger merged 1 commit into from
Nov 23, 2023
Merged

Add app-based filtering to global search #41669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions core/src/views/GlobalSearchModal.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<NcModal v-if="isVisible"
id="global-search"
<NcModal id="global-search"
ref="globalSearchModal"
:name="t('core', 'Global search')"
:show.sync="isVisible"
:clear-view-delay="0"
Expand Down Expand Up @@ -115,6 +115,14 @@
</div>
</div>
</div>
<div v-if="supportFiltering()" class="global-search-modal__results">
<NcButton @click="closeModal">
{{ t('core', 'Filter in current view') }}
<template #icon>
<FilterIcon :size="20" />
</template>
</NcButton>
</div>
</div>
</NcModal>
</template>
Expand All @@ -125,6 +133,7 @@ import AccountGroup from 'vue-material-design-icons/AccountGroup.vue'
import CalendarRangeIcon from 'vue-material-design-icons/CalendarRange.vue'
import CustomDateRangeModal from '../components/GlobalSearch/CustomDateRangeModal.vue'
import DotsHorizontalIcon from 'vue-material-design-icons/DotsHorizontal.vue'
import FilterIcon from 'vue-material-design-icons/Filter.vue'
import FilterChip from '../components/GlobalSearch/SearchFilterChip.vue'
import FlaskEmpty from 'vue-material-design-icons/FlaskEmpty.vue'
import ListBox from 'vue-material-design-icons/ListBox.vue'
Expand All @@ -140,6 +149,7 @@ import SearchableList from '../components/GlobalSearch/SearchableList.vue'
import SearchResult from '../components/GlobalSearch/SearchResult.vue'

import debounce from 'debounce'
import { emit } from '@nextcloud/event-bus'
import { getProviders, search as globalSearch, getContacts } from '../services/GlobalSearchService.js'

export default {
Expand All @@ -150,6 +160,7 @@ export default {
CalendarRangeIcon,
CustomDateRangeModal,
DotsHorizontalIcon,
FilterIcon,
FilterChip,
FlaskEmpty,
ListBox,
Expand Down Expand Up @@ -227,6 +238,9 @@ export default {
this.results = []
return
}
if (this.supportFiltering()) {
emit('nextcloud:unified-search.search', { query })
Copy link
Collaborator

@Altahrim Altahrim Nov 23, 2023

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?

Copy link
Contributor Author

@nfebe nfebe Nov 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. For the very reason you stated, emitting it does not do anything so, no need to emit it when it's not needed.
  2. 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

Copy link
Collaborator

@Altahrim Altahrim Nov 23, 2023

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.

Copy link
Member

@juliusknorr juliusknorr Nov 23, 2023 •
edited
Loading

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.

Copy link
Contributor Author

@nfebe nfebe Nov 23, 2023

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!

}
const newResults = []
const providersToSearch = this.filteredProviders.length > 0 ? this.filteredProviders : this.providers
const searchProvider = (provider, filters) => {
Expand Down Expand Up @@ -490,8 +504,16 @@ export default {
this.updateDateFilter()
},
closeModal() {
this.$refs.globalSearchModal.close()
this.searchQuery = ''
},
supportFiltering() {
/* Hard coded apps for the moment this would be improved in coming updates. */
const providerPaths = ['/settings/users', '/apps/files', '/apps/deck']
const currentPath = window.location.pathname.replace('/index.php', '')
const containsProvider = providerPaths.some(path => currentPath.includes(path))
return containsProvider
},
},
}
</script>
Expand Down
4 changes: 2 additions & 2 deletions dist/core-global-search.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/core-global-search.js.map
View file
Open in desktop

Large diffs are not rendered by default.