Common search improvements (#7586)

owncloud · Sep 6, 2022 · 307ecee · 307ecee
1 parent de72c25
commit 307ecee
Show file tree

Hide file tree

Showing 8 changed files with 385 additions and 331 deletions.
diff --git a/changelog/unreleased/enhancement-search-improvements b/changelog/unreleased/enhancement-search-improvements
@@ -0,0 +1,10 @@
+Enhancement: Search improvements
+
+We've improved the search, it will show now if no results according to term was found or if the results exceeds the
+search limit.
+We've also navigate to the last page while clicking the x  in the search input field.
+
+https://github.com/owncloud/web/pull/7586
+https://github.com/owncloud/web/issues/5644
+https://github.com/owncloud/web/issues/7587
+
diff --git a/packages/web-app-files/src/components/Search/List.vue b/packages/web-app-files/src/components/Search/List.vue
@@ -1,52 +1,59 @@
 <template>
-  <div class="files-search-result">
-    <app-bar :has-bulk-actions="true" />
-    <no-content-message v-if="!paginatedResources.length" class="files-empty" icon="folder">
-      <template #message>
-        <p class="oc-text-muted">
-          <span v-if="!!$route.query.term" v-translate>No resource found</span>
-          <span v-else v-translate>No search term entered</span>
-        </p>
+  <div class="files-search-result oc-flex">
+    <files-view-wrapper>
+      <app-bar :has-bulk-actions="false" :side-bar-open="sideBarOpen" />
+      <app-loading-spinner v-if="loading" />
+      <template v-else>
+        <no-content-message v-if="!paginatedResources.length" class="files-empty" icon="folder">
+          <template #message>
+            <p class="oc-text-muted">
+              <span v-if="!!$route.query.term" v-translate>No resource found</span>
+              <span v-else v-translate>No search term entered</span>
+            </p>
+          </template>
+        </no-content-message>
+        <resource-table
+          v-else
+          v-model="selectedResourcesIds"
+          class="files-table"
+          :class="{ 'files-table-squashed': false }"
+          :resources="paginatedResources"
+          :target-route="resourceTargetLocation"
+          :are-paths-displayed="true"
+          :are-thumbnails-displayed="displayThumbnails"
+          :has-actions="true"
+          :is-selectable="false"
+          @fileClick="$_fileActions_triggerDefaultAction"
+          @rowMounted="rowMounted"
+        >
+          <template #contextMenu="{ resource }">
+            <context-actions v-if="isResourceInSelection(resource)" :items="selectedResources" />
+          </template>
+          <template #footer>
+            <pagination :pages="paginationPages" :current-page="paginationPage" />
+            <div
+              v-if="searchResultExceedsLimit"
+              class="oc-text-nowrap oc-text-center oc-width-1-1 oc-my-s"
+              v-text="searchResultExceedsLimitText"
+            />
+            <list-info
+              v-else-if="paginatedResources.length > 0"
+              class="oc-width-1-1 oc-my-s"
+              :files="totalFilesCount.files"
+              :folders="totalFilesCount.folders"
+              :size="totalFilesSize"
+            />
+          </template>
+        </resource-table>
       </template>
-    </no-content-message>
-    <resource-table
-      v-else
-      v-model="selectedResources"
-      class="files-table"
-      :class="{ 'files-table-squashed': false }"
-      :resources="paginatedResources"
-      :target-route="resourceTargetLocation"
-      :are-paths-displayed="true"
-      :are-thumbnails-displayed="displayThumbnails"
-      :has-actions="true"
-      :is-selectable="false"
-      @fileClick="$_fileActions_triggerDefaultAction"
-      @rowMounted="rowMounted"
-    >
-      <template #contextMenu="{ resource }">
-        <context-actions v-if="isResourceInSelection(resource)" :items="selectedResources" />
-      </template>
-      <template #footer>
-        <pagination :pages="paginationPages" :current-page="paginationPage" />
-        <div
-          v-if="searchResultExceedsLimit"
-          class="oc-text-nowrap oc-text-center oc-width-1-1 oc-my-s"
-          v-text="searchResultExceedsLimitText"
-        />
-        <list-info
-          v-else-if="paginatedResources.length > 0"
-          class="oc-width-1-1 oc-my-s"
-          :files="totalFilesCount.files"
-          :folders="totalFilesCount.folders"
-          :size="totalFilesSize"
-        />
-      </template>
-    </resource-table>
+    </files-view-wrapper>
+    <side-bar :open="sideBarOpen" :active-panel="sideBarActivePanel" />
   </div>
 </template>
 
 <script lang="ts">
 import { useResourcesViewDefaults } from '../../composables'
+import AppLoadingSpinner from 'web-pkg/src/components/AppLoadingSpinner.vue'
 import { VisibilityObserver } from 'web-pkg/src/observer'
 import { ImageType, ImageDimension } from '../../constants'
 import { createLocationSpaces } from '../../router'
@@ -65,18 +72,34 @@ import MixinFilesListScrolling from '../../mixins/filesListScrolling'
 import { searchLimit } from '../../search/sdk/list'
 import { Resource } from 'web-client'
 import { useStore } from 'web-pkg/src/composables'
+import FilesViewWrapper from '../FilesViewWrapper.vue'
+import SideBar from '../../components/SideBar/SideBar.vue'
 
 const visibilityObserver = new VisibilityObserver()
 
 export default defineComponent({
-  components: { AppBar, ContextActions, ListInfo, Pagination, NoContentMessage, ResourceTable },
+  components: {
+    AppBar,
+    SideBar,
+    AppLoadingSpinner,
+    ContextActions,
+    ListInfo,
+    Pagination,
+    NoContentMessage,
+    ResourceTable,
+    FilesViewWrapper
+  },
   mixins: [MixinFileActions, MixinFilesListFilter, MixinFilesListScrolling],
   props: {
     searchResult: {
       type: Object,
       default: function () {
         return { range: null, values: [] }
       }
+    },
+    loading: {
+      type: Boolean,
+      default: false
     }
   },
   setup() {
@@ -91,7 +114,7 @@ export default defineComponent({
   },
   computed: {
     ...mapGetters(['configuration']),
-    ...mapGetters('Files', ['totalFilesCount', 'totalFilesSize']),
+    ...mapGetters('Files', ['highlightedFile', 'totalFilesCount', 'totalFilesSize']),
     displayThumbnails() {
       return !this.configuration?.options?.disablePreviews
     },
@@ -102,7 +125,7 @@ export default defineComponent({
       return this.searchResult.range
     },
     rangeItems() {
-      return this.searchResult.range?.split('/')[1]
+      return parseInt(this.searchResult.range?.split('/')[1] || 0)
     },
     searchResultExceedsLimit() {
       return !this.rangeSupported || (this.rangeItems && this.rangeItems > searchLimit)
@@ -127,6 +150,10 @@ export default defineComponent({
   watch: {
     searchResult: {
       handler: function () {
+        if (!this.searchResult) {
+          return
+        }
+
         this.CLEAR_CURRENT_FILES_LIST()
         this.LOAD_FILES({
           currentFolder: null,

diff --git a/packages/web-app-files/src/search/sdk/preview.ts b/packages/web-app-files/src/search/sdk/preview.ts
@@ -9,6 +9,8 @@ import VueRouter from 'vue-router'
 import { DavProperties } from 'web-pkg/src/constants'
 import { Store } from 'vuex'
 
+export const previewSearchLimit = 5
+
 export default class Preview implements SearchPreview {
   public readonly component: Component
   private readonly cache: Cache<string, SearchResult>
@@ -42,7 +44,7 @@ export default class Preview implements SearchPreview {
     const areHiddenFilesShown = this.store.state.Files?.areHiddenFilesShown
     const { range, results } = await clientService.owncloudSdk.files.search(
       term,
-      5, // todo: add configuration option, other places need that too... needs consolidation
+      previewSearchLimit, // todo: add configuration option, other places need that too... needs consolidation
       DavProperties.Default
     )
     const resources = results.reduce((acc, plainResource) => {
@@ -59,7 +61,6 @@ export default class Preview implements SearchPreview {
 
       return acc
     }, [])
-
     return this.cache.set(term, { range, values: resources })
   }
 

diff --git a/packages/web-app-files/tests/unit/components/Search/List.spec.js b/packages/web-app-files/tests/unit/components/Search/List.spec.js
@@ -135,6 +135,7 @@ function createStore(activeFiles) {
           selectedIds: []
         },
         getters: {
+          highlightedFile: () => activeFiles[0],
           activeFiles: () => activeFiles,
           selectedFiles: () => [],
           totalFilesCount: () => ({ files: activeFiles.length, folders: 0 }),

diff --git a/packages/web-app-files/tests/unit/components/Search/__snapshots__/List.spec.js.snap b/packages/web-app-files/tests/unit/components/Search/__snapshots__/List.spec.js.snap
@@ -1,31 +1,41 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`List component when no resource is found should show no-content-message component 1`] = `
-<div class="files-search-result">
-  <app-bar-stub breadcrumbs="" breadcrumbscontextactionsitems="" hasbulkactions="true" hassidebartoggle="true" hasviewoptions="true"></app-bar-stub>
-  <div class="oc-height-1-1 oc-flex oc-flex-column oc-flex-center oc-flex-middle oc-text-center files-empty">
-    <div class="oc-icon oc-icon-xxl oc-icon-passive">
-      <!---->
+<div class="files-search-result oc-flex">
+  <div class="files-view-wrapper oc-width-expand">
+    <div id="files-view">
+      <app-bar-stub breadcrumbs="" breadcrumbscontextactionsitems="" hassidebartoggle="true" hasviewoptions="true"></app-bar-stub>
+      <div class="oc-height-1-1 oc-flex oc-flex-column oc-flex-center oc-flex-middle oc-text-center files-empty">
+        <div class="oc-icon oc-icon-xxl oc-icon-passive">
+          <!---->
+        </div>
+        <div class="oc-text-muted oc-text-xlarge">
+          <p class="oc-text-muted"><span data-msgid="No search term entered" data-current-language="en_US">No search term entered</span></p>
+        </div>
+        <div class="oc-text-muted"></div>
+      </div>
     </div>
-    <div class="oc-text-muted oc-text-xlarge">
-      <p class="oc-text-muted"><span data-msgid="No search term entered" data-current-language="en_US">No search term entered</span></p>
-    </div>
-    <div class="oc-text-muted"></div>
   </div>
+  <!---->
 </div>
 `;
 
 exports[`List component when no search term is entered should show no-content-message component 1`] = `
-<div class="files-search-result">
-  <app-bar-stub breadcrumbs="" breadcrumbscontextactionsitems="" hasbulkactions="true" hassidebartoggle="true" hasviewoptions="true"></app-bar-stub>
-  <div class="oc-height-1-1 oc-flex oc-flex-column oc-flex-center oc-flex-middle oc-text-center files-empty">
-    <div class="oc-icon oc-icon-xxl oc-icon-passive">
-      <!---->
-    </div>
-    <div class="oc-text-muted oc-text-xlarge">
-      <p class="oc-text-muted"><span data-msgid="No search term entered" data-current-language="en_US">No search term entered</span></p>
+<div class="files-search-result oc-flex">
+  <div class="files-view-wrapper oc-width-expand">
+    <div id="files-view">
+      <app-bar-stub breadcrumbs="" breadcrumbscontextactionsitems="" hassidebartoggle="true" hasviewoptions="true"></app-bar-stub>
+      <div class="oc-height-1-1 oc-flex oc-flex-column oc-flex-center oc-flex-middle oc-text-center files-empty">
+        <div class="oc-icon oc-icon-xxl oc-icon-passive">
+          <!---->
+        </div>
+        <div class="oc-text-muted oc-text-xlarge">
+          <p class="oc-text-muted"><span data-msgid="No search term entered" data-current-language="en_US">No search term entered</span></p>
+        </div>
+        <div class="oc-text-muted"></div>
+      </div>
     </div>
-    <div class="oc-text-muted"></div>
   </div>
+  <!---->
 </div>
 `;
diff --git a/packages/web-app-search/src/portals/SearchBar.vue b/packages/web-app-search/src/portals/SearchBar.vue
@@ -49,6 +49,15 @@
               :search-result="searchResultValue"
             />
           </li>
+          <li
+            v-if="showNoMatches"
+            id="no-matches"
+            class="oc-text-center oc-text-muted"
+            v-text="$gettext('No result')"
+          ></li>
+          <li v-if="showMoreMatches" id="more-matches" class="oc-text-center oc-text-muted">
+            {{ moreMatchesText }}
+          </li>
         </template>
       </ul>
     </div>
@@ -78,6 +87,32 @@ export default {
   },
 
   computed: {
+    rangeSupported() {
+      return this.searchResult.range
+    },
+
+    rangeItems() {
+      return parseInt(this.searchResult.range?.split('/')[1] || 0)
+    },
+
+    showMoreMatches() {
+      return this.rangeSupported && this.rangeItems > this.searchResult.values.length
+    },
+
+    moreMatchesText() {
+      const moreCount = this.rangeItems - this.searchResult.values.length
+      return this.$gettextInterpolate(
+        this.$ngettext('%{moreCount} more result', '%{moreCount} more results', moreCount),
+        {
+          moreCount
+        }
+      )
+    },
+
+    showNoMatches() {
+      return this.searchResult?.values?.length === 0
+    },
+
     availableProviders() {
       return this.providerStore.availableProviders
     },
@@ -88,27 +123,26 @@ export default {
   },
 
   watch: {
-    $route() {
-      if (this.activeProvider && !this.activeProvider.available) {
-        this.activeProvider = undefined
-      }
-    }
-  },
-
-  mounted() {
-    if (!this.availableProviders.length) {
-      return
-    }
-
-    const input = this.$el.getElementsByTagName('input')[0]
-    const routeTerm = get(this, '$route.query.term')
-
-    if (!input || !routeTerm) {
-      return
+    $route: {
+      handler(r, o) {
+        if (!!o && this.activeProvider && !this.activeProvider.available) {
+          this.activeProvider = undefined
+        }
+        this.$nextTick(() => {
+          if (!this.availableProviders.length) {
+            return
+          }
+          const routeTerm = get(r, 'query.term')
+          const input = this.$el.getElementsByTagName('input')[0]
+          if (!input || !routeTerm) {
+            return
+          }
+          this.term = routeTerm
+          input.value = routeTerm
+        })
+      },
+      immediate: true
     }
-
-    this.term = routeTerm
-    input.value = routeTerm
   },
 
   asyncComputed: {
@@ -153,6 +187,7 @@ export default {
     resetProvider() {
       this.optionsVisible = false
       this.availableProviders.forEach((provider) => provider.reset())
+      this.$router.go(-1)
     },
     activateProvider(provider) {
       this.optionsVisible = false
@@ -301,7 +336,6 @@ export default {
 
     li {
       padding: 15px 10px;
-      cursor: pointer;
       position: relative;
       font-size: var(--oc-font-size-small);