Bugfix: Gridview whitespace autosize (#10118)

* Bugfix: Gridview whitespace autosize * Fix large scale * Fix viewoption sizes * Add changelog * Update unittests * WIP * Solution? * WIP * WIP * Remove dev leftover * cleanup code * Calculate max tiles count per row / still needs to be recalculated when the slider value changes * Implement watch for tile-size slider changes * Rename rowCount * Lint, change var to const, remove unnecessary computed * Hide from screen readers * feat: introduce useTileSize composable and watch that for tile sizes * Fix undefined issues * slim mount function * fix: tile pixels and ghost tile count * test: fix viewOptions unit tests * Fixing ResourceTiles.spec.ts tests (WIP) * Fix ResourceTiles.spec.ts * Remove dev leftover --------- Co-authored-by: Jan Ackermann <[email protected]> Co-authored-by: Benedikt Kulmann <[email protected]>
owncloud · Dec 20, 2023 · 16bb36a · 16bb36a
1 parent d85a971
commit 16bb36a
Show file tree

Hide file tree

Showing 13 changed files with 216 additions and 71 deletions.
diff --git a/changelog/unreleased/bugfix-tiles-view-whitespace b/changelog/unreleased/bugfix-tiles-view-whitespace
@@ -0,0 +1,8 @@
+Bugfix: Tilesview has whitespace
+
+We've fixed a bug that caused the tiles-view to have whitespace
+on the right side of the screen which is not optimal for efficiant
+space management.
+
+https://github.com/owncloud/web/pull/10118
+https://github.com/owncloud/web/issues/10040
diff --git a/docs/theming/_index.md b/docs/theming/_index.md
@@ -307,7 +307,9 @@ Size variables get prepended with `--oc-size-`, so e.g. _"icon-default"_ creates
     "icon-default": "",
     "max-height-logo": "",
     "max-width-logo": "",
-    "width-medium": ""
+    "width-medium": "",
+    "tiles-default": "",
+    "tiles-resize-step": ""
   }
 }
 ```
@@ -426,7 +428,9 @@ A full template for your custom theme is provided below, and you can use the ins
             "icon-default": "",
             "max-height-logo": "",
             "max-width-logo": "",
-            "width-medium": ""
+            "width-medium": "",
+            "tiles-default": "",
+            "tiles-resize-step": ""
           },
           "spacing": {
             "xsmall": "",

diff --git a/packages/design-system/src/tokens/ods/size.yaml b/packages/design-system/src/tokens/ods/size.yaml
@@ -24,6 +24,6 @@ size:
     value: 14px
   tiles:
     resize-step:
-      value: 12rem
+      value: 9rem
     default:
-      value: 14rem
+      value: 9rem
diff --git a/packages/web-app-files/src/components/FilesList/ResourceTiles.vue b/packages/web-app-files/src/components/FilesList/ResourceTiles.vue
@@ -34,7 +34,7 @@
         </oc-list>
       </oc-drop>
     </div>
-    <oc-list class="oc-tiles oc-flex" :class="resizable ? 'resizableTiles' : ''">
+    <oc-list class="oc-tiles oc-flex">
       <li v-for="resource in data" :key="resource.id" class="oc-tiles-item has-item-context-menu">
         <oc-tile
           :ref="(el) => (tileRefs.tiles[resource.id] = el)"
@@ -85,6 +85,16 @@
           </template>
         </oc-tile>
       </li>
+      <li
+        v-for="index in ghostTilesCount"
+        :key="`ghost-tile-${index}`"
+        class="ghost-tile"
+        :aria-hidden="true"
+      >
+        <div>
+          {{ viewSize }}
+        </div>
+      </li>
     </oc-list>
     <Teleport v-if="dragItem" to="body">
       <oc-ghost-element ref="ghostElementRef" :preview-items="[dragItem, ...dragSelection]" />
@@ -96,21 +106,37 @@
 </template>
 
 <script lang="ts">
-import { onBeforeUpdate, defineComponent, nextTick, PropType, computed, ref, unref } from 'vue'
+import {
+  computed,
+  defineComponent,
+  nextTick,
+  onBeforeUnmount,
+  onBeforeUpdate,
+  onMounted,
+  PropType,
+  ref,
+  unref,
+  watch
+} from 'vue'
 import { useGettext } from 'vue3-gettext'
 import { Resource, SpaceResource } from '@ownclouders/web-client'
-import { useStore, SortDir, SortField, ViewModeConstants } from '@ownclouders/web-pkg'
-import { ImageDimension } from '@ownclouders/web-pkg'
-import { createFileRouteOptions } from '@ownclouders/web-pkg'
-import { displayPositionedDropdown } from '@ownclouders/web-pkg'
-import { createLocationSpaces } from '@ownclouders/web-pkg'
-import { ContextMenuQuickAction } from '@ownclouders/web-pkg'
-
 // Constants should match what is being used in OcTable/ResourceTable
 // Alignment regarding naming would be an API-breaking change and can
 // Be done at a later point in time?
-import { useResourceRouteResolver } from '@ownclouders/web-pkg'
-import { eventBus } from '@ownclouders/web-pkg'
+import {
+  ContextMenuQuickAction,
+  createFileRouteOptions,
+  createLocationSpaces,
+  displayPositionedDropdown,
+  eventBus,
+  ImageDimension,
+  SortDir,
+  SortField,
+  useResourceRouteResolver,
+  useStore,
+  useTileSize,
+  ViewModeConstants
+} from '@ownclouders/web-pkg'
 
 export default defineComponent({
   name: 'ResourceTiles',
@@ -123,10 +149,6 @@ export default defineComponent({
       type: Array as PropType<Resource[]>,
       default: () => []
     },
-    resizable: {
-      type: Boolean,
-      default: false
-    },
     selectedIds: {
       type: Array,
       default: () => []
@@ -179,6 +201,7 @@ export default defineComponent({
 
     const dragItem = ref()
     const ghostElementRef = ref()
+
     const tileRefs = ref({
       tiles: [],
       dropBtns: []
@@ -308,7 +331,6 @@ export default defineComponent({
       }
       return sizeMap[props.viewSize] ?? 'xlarge'
     })
-
     onBeforeUpdate(() => {
       tileRefs.value = {
         tiles: [],
@@ -363,6 +385,50 @@ export default defineComponent({
       context.emit('fileDropped', resource.id)
     }
 
+    const viewWidth = ref(0)
+    const updateViewWidth = () => {
+      const element = document.getElementById('tiles-view')
+      const style = getComputedStyle(element)
+      const paddingLeft = parseInt(style.getPropertyValue('padding-left'), 10) | 0
+      const paddingRight = parseInt(style.getPropertyValue('padding-right'), 10) | 0
+      viewWidth.value = element.clientWidth - paddingLeft - paddingRight
+    }
+    const { tileSizePixels: tileSizePixelsBase } = useTileSize()
+    const gapSizePixels = computed(() => {
+      return parseFloat(getComputedStyle(document.documentElement).fontSize)
+    })
+    const maxTiles = computed(() => {
+      return unref(tileSizePixelsBase)
+        ? Math.floor(unref(viewWidth) / (unref(tileSizePixelsBase) + unref(gapSizePixels)))
+        : 0
+    })
+    const ghostTilesCount = computed(() => {
+      const remainder = unref(maxTiles) ? props.data.length % unref(maxTiles) : 0
+      if (!remainder) {
+        return 0
+      }
+      return unref(maxTiles) - remainder
+    })
+
+    const tileSizePixels = computed(() => {
+      return unref(viewWidth) / unref(maxTiles) - unref(gapSizePixels)
+    })
+    watch(
+      tileSizePixels,
+      (px: number) => {
+        document.documentElement.style.setProperty(`--oc-size-tiles-actual`, `${px}px`)
+      },
+      { immediate: true }
+    )
+
+    onMounted(() => {
+      window.addEventListener('resize', updateViewWidth)
+      updateViewWidth()
+    })
+    onBeforeUnmount(() => {
+      window.removeEventListener('resize', updateViewWidth)
+    })
+
     return {
       areFileExtensionsShown,
       emitTileClick,
@@ -383,7 +449,8 @@ export default defineComponent({
       dragStart,
       dragSelection,
       fileDropped,
-      setDropStyling
+      setDropStyling,
+      ghostTilesCount
     }
   },
   data() {
@@ -398,24 +465,10 @@ export default defineComponent({
 .oc-tiles {
   column-gap: 1rem;
   display: grid;
-  grid-template-columns: repeat(auto-fill, minmax(auto, var(--oc-size-tiles-default)));
+  grid-template-columns: repeat(auto-fit, minmax(var(--oc-size-tiles-actual), 1fr));
   justify-content: flex-start;
   row-gap: 1rem;
 
-  &.resizableTiles {
-    grid-template-columns: repeat(auto-fill, minmax(auto, var(--oc-size-tiles-resize-step)));
-  }
-
-  @media only screen and (max-width: 640px) {
-    grid-template-columns: 80%;
-    justify-content: center;
-    padding: var(--oc-space-medium) 0;
-
-    &.resizableTiles {
-      grid-template-columns: 80%;
-    }
-  }
-
   &-item-drop-highlight {
     background-color: var(--oc-color-input-border) !important;
   }
@@ -441,4 +494,17 @@ export default defineComponent({
     }
   }
 }
+
+.ghost-tile {
+  display: list-item;
+
+  div {
+    opacity: 0;
+    box-shadow: none;
+    height: 100%;
+    display: flex;
+    flex-flow: column;
+    outline: 1px solid var(--oc-color-border);
+  }
+}
 </style>
diff --git a/packages/web-app-files/src/views/spaces/GenericSpace.vue b/packages/web-app-files/src/views/spaces/GenericSpace.vue
@@ -69,7 +69,6 @@
             v-model:selectedIds="selectedResourcesIds"
             :data="paginatedResources"
             class="oc-px-m oc-pt-l"
-            :resizable="true"
             :target-route-callback="resourceTargetRouteCallback"
             :space="space"
             :drag-drop="true"

diff --git a/packages/web-app-files/src/views/spaces/Projects.vue b/packages/web-app-files/src/views/spaces/Projects.vue
@@ -49,7 +49,6 @@
             v-model:selectedIds="selectedResourcesIds"
             class="oc-px-m"
             :data="paginatedItems"
-            :resizable="true"
             :sort-fields="sortFields"
             :sort-by="sortBy"
             :sort-dir="sortDir"

diff --git a/packages/web-app-files/tests/unit/components/FilesList/ResourceTiles.spec.ts b/packages/web-app-files/tests/unit/components/FilesList/ResourceTiles.spec.ts
@@ -3,6 +3,12 @@ import ResourceTiles from 'web-app-files/src/components/FilesList/ResourceTiles.
 import { sortFields } from 'web-app-files/src/helpers/ui/resourceTiles'
 import { Resource } from '@ownclouders/web-client'
 import { mock } from 'jest-mock-extended'
+jest.mock('@ownclouders/web-pkg', () => ({
+  ...jest.requireActual('@ownclouders/web-pkg'),
+  useTileSize: jest.fn().mockReturnValue({
+    tileSizePixels: 100
+  })
+}))
 
 const spacesResources = [
   {
@@ -24,8 +30,37 @@ const spacesResources = [
 ]
 
 describe('ResourceTiles component', () => {
-  it('renders an array of spaces correctly', () => {
+  const originalGetElementById = document.getElementById
+  const originalGetComputedStyle = window.getComputedStyle
+  beforeEach(() => {
+    const mockElement = {
+      clientWidth: 800
+    }
+    ;(document as any).getElementById = jest.fn((id) => {
+      if (id === 'tiles-view') {
+        return mockElement
+      }
+      return originalGetElementById.call(document, id)
+    })
+    window.getComputedStyle = jest.fn().mockImplementation(() => {
+      return {
+        getPropertyValue: (propName) => {
+          switch (propName) {
+            case '--oc-size-tiles-default':
+              return '9rem'
+            case '--oc-size-tiles-resize-step':
+              return '9rem'
+            default:
+              return originalGetComputedStyle(document.documentElement).getPropertyValue(propName)
+          }
+        },
+        fontSize: '14px'
+      }
+    })
+  })
+  it('renders an array of spaces correctly', async () => {
     const { wrapper } = getWrapper({ data: spacesResources })
+    await wrapper.vm.$nextTick()
     expect(wrapper.html()).toMatchSnapshot()
   })
 

diff --git a/...es/web-app-files/tests/unit/components/FilesList/__snapshots__/ResourceTiles.spec.ts.snap b/...es/web-app-files/tests/unit/components/FilesList/__snapshots__/ResourceTiles.spec.ts.snap
@@ -3,7 +3,7 @@
 exports[`ResourceTiles component renders a footer slot 1`] = `
 <div>
   <!--v-if-->
-  <oc-list class="oc-tiles oc-flex"></oc-list>
+  <oc-list class="oc-tiles oc-flex"> </oc-list>
   <!--v-if-->
   <div class="oc-tiles-footer">Hello, ResourceTiles footer!</div>
 </div>
@@ -19,6 +19,21 @@ exports[`ResourceTiles component renders an array of spaces correctly 1`] = `
     <li class="oc-tiles-item has-item-context-menu">
       <oc-tile draggable="false" is-resource-selected="false" resource="[object Object]" resource-icon-size="xlarge" resource-route="[object Object]"></oc-tile>
     </li>
+    <li aria-hidden="true" class="ghost-tile">
+      <div>1</div>
+    </li>
+    <li aria-hidden="true" class="ghost-tile">
+      <div>1</div>
+    </li>
+    <li aria-hidden="true" class="ghost-tile">
+      <div>1</div>
+    </li>
+    <li aria-hidden="true" class="ghost-tile">
+      <div>1</div>
+    </li>
+    <li aria-hidden="true" class="ghost-tile">
+      <div>1</div>
+    </li>
   </oc-list>
   <!--v-if-->
   <div class="oc-tiles-footer"></div>

diff --git a/packages/web-pkg/src/components/ViewOptions.vue b/packages/web-pkg/src/components/ViewOptions.vue
@@ -80,7 +80,6 @@
             max="6"
             class="oc-range"
             data-testid="files-tiles-size-slider"
-            @input="setTilesViewSize"
           />
         </li>
       </oc-list>
@@ -166,15 +165,6 @@ export default defineComponent({
       defaultValue: ViewModeConstants.tilesSizeDefault.toString()
     })
 
-    const setTilesViewSize = () => {
-      const rootStyle = (document.querySelector(':root') as HTMLElement).style
-      const currentSize = rootStyle.getPropertyValue('--oc-size-tiles-resize-step')
-      const newSize = `${(unref(viewSizeQuery) as any) * 12}rem`
-      if (!currentSize || currentSize !== newSize) {
-        rootStyle.setProperty(`--oc-size-tiles-resize-step`, newSize)
-      }
-    }
-
     const setItemsPerPage = (itemsPerPage: string) => {
       return router.replace({
         query: {
@@ -197,24 +187,13 @@ export default defineComponent({
       { immediate: true, deep: true }
     )
 
-    watch(
-      viewSizeQuery,
-      (size) => {
-        if (size) {
-          setTilesViewSize()
-        }
-      },
-      { immediate: true }
-    )
-
     return {
       ViewModeConstants,
       viewModeCurrent: viewModeQuery,
       viewSizeCurrent: viewSizeQuery,
       itemsPerPageCurrent: itemsPerPageQuery,
       queryParamsLoading,
       queryItemAsString,
-      setTilesViewSize,
       setItemsPerPage,
       setViewMode,
       viewOptionsButtonLabel: $gettext('Display customization options of the files list')

diff --git a/packages/web-pkg/src/composables/viewMode/index.ts b/packages/web-pkg/src/composables/viewMode/index.ts
@@ -1,2 +1,3 @@
 export * from './constants'
+export * from './useTileSize'
 export * from './useViewMode'