Feature/7600 - Scroll to newly created folder (#9145)

* Fix #7600 && #7601

* Fix useFileActionsCreateNewFolder.spec.ts

* Fix unit tests

---------

Co-authored-by: Dominik Schmidt <[email protected]>

changelog/unreleased/enhancement-scroll-to-created-folder

@@ -0,0 +1,7 @@
+Enhancement: Scroll to newly created folder
+
+After creating a new folder that gets sorted into the currently displayed resources but outside of the current viewport, we now scroll to the new folder.
+
+https://github.com/owncloud/web/issues/7600
+https://github.com/owncloud/web/issues/7601
+https://github.com/owncloud/web/pulls/8145

packages/web-app-files/src/components/FilesList/KeyboardActions.vue

@@ -103,7 +103,7 @@ export default defineComponent({
       resetSelectionCursor()
       store.dispatch('Files/resetFileSelection')
       store.commit('Files/ADD_FILE_SELECTION', { id: nextId })
-      scrollToResource({ id: nextId } as any)
+      scrollToResource(nextId)
     }
     const handleCtrlClickAction = (resource) => {
       store.dispatch('Files/toggleFileSelection', { id: resource.id })
@@ -130,7 +130,7 @@ export default defineComponent({
         // select
         store.commit('Files/ADD_FILE_SELECTION', { id: nextResourceId })
       }
-      scrollToResource({ id: nextResourceId } as any)
+      scrollToResource(nextResourceId)
       selectionCursor.value = unref(selectionCursor) - 1
     }
     const handleShiftDownAction = () => {
@@ -146,7 +146,7 @@ export default defineComponent({
         // select
         store.commit('Files/ADD_FILE_SELECTION', { id: nextResourceId })
       }
-      scrollToResource({ id: nextResourceId } as any)
+      scrollToResource(nextResourceId)
       selectionCursor.value = unref(selectionCursor) + 1
     }
     const handleShiftClickAction = ({ resource, skipTargetSelection }) => {

packages/web-app-files/src/composables/actions/files/useFileActionsCreateNewFolder.ts

@@ -1,6 +1,6 @@
 import { Resource, SpaceResource } from 'web-client/src/helpers'
 import { Store } from 'vuex'
-import { computed, unref } from 'vue'
+import { computed, nextTick, unref } from 'vue'
 import { useClientService, useRouter, useStore } from 'web-pkg/src/composables'
 import { FileAction } from 'web-pkg/src/composables/actions'
 import { useGettext } from 'vue3-gettext'
@@ -9,6 +9,7 @@ import { join } from 'path'
 import { WebDAV } from 'web-client/src/webdav'
 import { isLocationSpacesActive } from 'web-app-files/src/router'
 import { getIndicators } from 'web-app-files/src/helpers/statusIndicators'
+import { useScrollTo } from '../../scrollTo/useScrollTo'
 
 export const useFileActionsCreateNewFolder = ({
   store,
@@ -17,6 +18,7 @@ export const useFileActionsCreateNewFolder = ({
   store = store || useStore()
   const router = useRouter()
   const { $gettext } = useGettext()
+  const { scrollToResource } = useScrollTo()
 
   const clientService = useClientService()
   const currentFolder = computed((): Resource => store.getters['Files/currentFolder'])
@@ -72,6 +74,9 @@ export const useFileActionsCreateNewFolder = ({
       store.dispatch('showMessage', {
         title: $gettext('"%{folderName}" was created successfully', { folderName })
       })
+
+      await nextTick()
+      scrollToResource(resource.id, { forceScroll: true })
     } catch (error) {
       console.error(error)
       store.dispatch('showErrorMessage', {

packages/web-app-files/src/composables/keyboardActions/useKeyboardActionsSearchTable.ts

@@ -1,11 +1,12 @@
-import { computed, onBeforeUnmount, onMounted, ref, unref } from 'vue'
+import { Ref, computed, nextTick, onBeforeUnmount, onMounted, ref, unref } from 'vue'
 import { useStore } from 'web-pkg/src/composables'
 import { useScrollTo } from 'web-app-files/src/composables/scrollTo'
 import { Key, ModifierKey } from 'web-pkg/src/composables/keyboardActions'
 import { eventBus } from 'web-pkg'
 import { useGettext } from 'vue3-gettext'
+import { Resource } from 'web-client/src'
 
-export const useKeyboardActionsSearchTable = (keyActions, paginatedResources) => {
+export const useKeyboardActionsSearchTable = (keyActions, paginatedResources: Ref<Resource[]>) => {
   const store = useStore()
   const { scrollToResource } = useScrollTo()
   const selectionCursor = ref(0)
@@ -54,9 +55,11 @@ export const useKeyboardActionsSearchTable = (keyActions, paginatedResources) =>
       return
     }
     resetSelectionCursor()
-    await store.dispatch('Files/resetFileSelection')
+    store.dispatch('Files/resetFileSelection')
+    await nextTick()
     store.commit('Files/ADD_FILE_SELECTION', { id: nextId })
-    scrollToResource({ id: nextId } as any)
+    await nextTick()
+    scrollToResource(nextId)
   }
 
   const getNextResourceId = (previous = false) => {
@@ -99,7 +102,7 @@ export const useKeyboardActionsSearchTable = (keyActions, paginatedResources) =>
       // select
       store.commit('Files/ADD_FILE_SELECTION', { id: nextResourceId })
     }
-    scrollToResource({ id: nextResourceId } as any)
+    scrollToResource(nextResourceId)
     selectionCursor.value = unref(selectionCursor) - 1
   }
   const handleShiftDownAction = () => {
@@ -115,7 +118,7 @@ export const useKeyboardActionsSearchTable = (keyActions, paginatedResources) =>
       // select
       store.commit('Files/ADD_FILE_SELECTION', { id: nextResourceId })
     }
-    scrollToResource({ id: nextResourceId } as any)
+    scrollToResource(nextResourceId)
     selectionCursor.value = unref(selectionCursor) + 1
   }
 

packages/web-app-files/src/composables/scrollTo/useScrollTo.ts

@@ -1,12 +1,13 @@
 import { computed, unref } from 'vue'
 import { Resource } from 'web-client/src'
-import { eventBus, useStore } from 'web-pkg/src'
-import { queryItemAsString } from 'web-pkg/src/composables/appDefaults'
+import { queryItemAsString } from 'web-pkg/src/composables/appDefaults/useAppNavigation'
+import { useStore } from 'web-pkg/src/composables/store/useStore'
+import { eventBus } from 'web-pkg/src/services'
 import { useRouteQuery } from 'web-pkg/src/composables'
 import { SideBarEventTopics } from 'web-pkg/src/composables/sideBar'
 
 export interface ScrollToResult {
-  scrollToResource(resource: Resource): void
+  scrollToResource(resourceId: Resource['id'], options?: { forceScroll?: boolean }): void
   scrollToResourceFromRoute(resources: Resource[]): void
 }
 
@@ -21,29 +22,25 @@ export const useScrollTo = (): ScrollToResult => {
     return queryItemAsString(unref(detailsQuery))
   })
 
-  const scrollToResource = (resource) => {
+  const scrollToResource = (resourceId: Resource['id'], options = { forceScroll: false }) => {
     const resourceElement = document.querySelectorAll(
-      `[data-item-id='${resource.id}']`
+      `[data-item-id='${resourceId}']`
     )[0] as HTMLElement
 
     if (!resourceElement) {
       return
     }
 
-    // bottom reached
-    if (resourceElement.getBoundingClientRect().bottom > window.innerHeight) {
-      resourceElement.scrollIntoView(false)
-      return
-    }
-
-    const topbarElement = document.getElementsByClassName('files-topbar')[0] as HTMLElement
+    const topbarElement = document.getElementById('files-app-bar')
     // topbar height + th height + height of one row = offset needed when scrolling top
     const topOffset = topbarElement.offsetHeight + resourceElement.offsetHeight * 2
 
-    // top reached
-    if (resourceElement.getBoundingClientRect().top < topOffset) {
-      const fileListWrapperElement = document.getElementsByClassName('files-view-wrapper')[0]
-      fileListWrapperElement.scrollBy(0, -resourceElement.offsetHeight)
+    if (
+      resourceElement.getBoundingClientRect().bottom > window.innerHeight ||
+      resourceElement.getBoundingClientRect().top < topOffset ||
+      options.forceScroll
+    ) {
+      resourceElement.scrollIntoView({ behavior: 'smooth', block: 'center' })
     }
   }
 
@@ -55,7 +52,7 @@ export const useScrollTo = (): ScrollToResult => {
     const resource = unref(resources).find((r) => r.id === unref(scrollTo))
     if (resource) {
       store.commit('Files/SET_FILE_SELECTION', [resource])
-      scrollToResource(resource)
+      scrollToResource(resource.id, { forceScroll: true })
 
       if (unref(details)) {
         eventBus.publish(SideBarEventTopics.openWithPanel, unref(details))

packages/web-app-files/tests/unit/composables/actions/files/useFileActionsCreateNewFolder.spec.ts

@@ -10,6 +10,10 @@ import {
   defaultStoreMockOptions,
   getComposableWrapper
 } from 'web-test-helpers/src'
+import { useScrollToMock } from 'web-app-files/tests/mocks/useScrollToMock'
+import { useScrollTo } from 'web-app-files/src/composables/scrollTo'
+
+jest.mock('web-app-files/src/composables/scrollTo')
 
 describe('useFileActionsCreateNewFolder', () => {
   describe('checkFolderName', () => {
@@ -46,6 +50,8 @@ describe('useFileActionsCreateNewFolder', () => {
               title: '"myfolder" was created successfully'
             })
           )
+
+          // expect scrolltoresource to have been called
         }
       })
     })
@@ -96,6 +102,8 @@ function getWrapper({
     options: { storeOptions: typeof defaultStoreMockOptions }
   ) => void
 }) {
+  jest.mocked(useScrollTo).mockImplementation(() => useScrollToMock())
+
   const mocks = {
     ...defaultComponentMocks({
       currentRoute: mock<RouteLocation>({ name: 'files-spaces-generic' })

packages/web-app-files/tests/unit/composables/scrollTo/useScrollTo.spec.ts

@@ -6,6 +6,11 @@ import { defaultComponentMocks } from 'web-test-helpers/src/mocks/defaultCompone
 import { defaultStoreMockOptions } from 'web-test-helpers/src/mocks/store/defaultStoreMockOptions'
 import { getComposableWrapper, RouteLocation } from 'web-test-helpers'
 
+const mockResourceId = 'fakeResourceId'
+const mockFilesTopBar = {
+  offsetHeight: 75
+}
+
 describe('useScrollTo', () => {
   it('should be valid', () => {
     expect(useScrollTo).toBeDefined()
@@ -18,16 +23,17 @@ describe('useScrollTo', () => {
       offsetHeight: 100
     })
 
-    it('calls does nothing when no element was found', () => {
+    it('does nothing when no element was found', () => {
       const htmlPageObject = getHTMLPageObject()
       jest.spyOn(document, 'querySelectorAll').mockImplementation(() => [] as any)
+      jest.spyOn(document, 'getElementById').mockImplementation(() => mockFilesTopBar as any)
 
       const mocks = defaultComponentMocks()
 
       getComposableWrapper(
         () => {
           const { scrollToResource } = useScrollTo()
-          scrollToResource(mockDeep<Resource>())
+          scrollToResource(mockResourceId)
           expect(htmlPageObject.scrollIntoView).not.toHaveBeenCalled()
         },
         { mocks, provide: mocks, store: defaultStoreMockOptions }
@@ -36,34 +42,35 @@ describe('useScrollTo', () => {
     it('calls "scrollIntoView" when the page bottom is reached', () => {
       const htmlPageObject = getHTMLPageObject()
       jest.spyOn(document, 'querySelectorAll').mockImplementation(() => [htmlPageObject] as any)
+      jest.spyOn(document, 'getElementById').mockImplementation(() => mockFilesTopBar as any)
+
       window.innerHeight = 100
 
       const mocks = defaultComponentMocks()
 
       getComposableWrapper(
         () => {
           const { scrollToResource } = useScrollTo()
-          scrollToResource(mockDeep<Resource>())
+          scrollToResource(mockResourceId)
           expect(htmlPageObject.scrollIntoView).toHaveBeenCalled()
         },
         { mocks, provide: mocks, store: defaultStoreMockOptions }
       )
     })
-    it('calls "scrollBy" when the page top is reached', () => {
+    it('calls "scrollIntoView" when the page top is reached', () => {
       const htmlPageObject = getHTMLPageObject()
       jest.spyOn(document, 'querySelectorAll').mockImplementation(() => [htmlPageObject] as any)
-      jest
-        .spyOn(document, 'getElementsByClassName')
-        .mockImplementation(() => [htmlPageObject] as any)
+      jest.spyOn(document, 'getElementById').mockImplementation(() => mockFilesTopBar as any)
+
       window.innerHeight = 500
 
       const mocks = defaultComponentMocks()
 
       getComposableWrapper(
         () => {
           const { scrollToResource } = useScrollTo()
-          scrollToResource(mockDeep<Resource>())
-          expect(htmlPageObject.scrollBy).toHaveBeenCalled()
+          scrollToResource(mockResourceId)
+          expect(htmlPageObject.scrollIntoView).toHaveBeenCalled()
         },
         { mocks, provide: mocks, store: defaultStoreMockOptions }
       )