From 1998e9e3186bb0e197d9a08ce2a83cbe4e7acb58 Mon Sep 17 00:00:00 2001 From: Pascal Wengerter Date: Tue, 25 Jul 2023 14:40:22 +0200 Subject: [PATCH] Feature/7600 - Scroll to newly created folder (#9145) * Fix #7600 && #7601 * Fix useFileActionsCreateNewFolder.spec.ts * Fix unit tests --------- Co-authored-by: Dominik Schmidt --- .../enhancement-scroll-to-created-folder | 7 +++++ .../components/FilesList/KeyboardActions.vue | 6 ++-- .../files/useFileActionsCreateNewFolder.ts | 7 ++++- .../useKeyboardActionsSearchTable.ts | 15 +++++---- .../src/composables/scrollTo/useScrollTo.ts | 31 +++++++++---------- .../useFileActionsCreateNewFolder.spec.ts | 8 +++++ .../composables/scrollTo/useScrollTo.spec.ts | 25 +++++++++------ 7 files changed, 63 insertions(+), 36 deletions(-) create mode 100644 changelog/unreleased/enhancement-scroll-to-created-folder diff --git a/changelog/unreleased/enhancement-scroll-to-created-folder b/changelog/unreleased/enhancement-scroll-to-created-folder new file mode 100644 index 00000000000..7c86f33237c --- /dev/null +++ b/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 diff --git a/packages/web-app-files/src/components/FilesList/KeyboardActions.vue b/packages/web-app-files/src/components/FilesList/KeyboardActions.vue index 7dacbfb36dc..06e1e84f887 100644 --- a/packages/web-app-files/src/components/FilesList/KeyboardActions.vue +++ b/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 }) => { diff --git a/packages/web-app-files/src/composables/actions/files/useFileActionsCreateNewFolder.ts b/packages/web-app-files/src/composables/actions/files/useFileActionsCreateNewFolder.ts index 54685c927b8..8fecc07be41 100644 --- a/packages/web-app-files/src/composables/actions/files/useFileActionsCreateNewFolder.ts +++ b/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', { diff --git a/packages/web-app-files/src/composables/keyboardActions/useKeyboardActionsSearchTable.ts b/packages/web-app-files/src/composables/keyboardActions/useKeyboardActionsSearchTable.ts index 84392c63cc4..d2fb9a2016d 100644 --- a/packages/web-app-files/src/composables/keyboardActions/useKeyboardActionsSearchTable.ts +++ b/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) => { 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 } diff --git a/packages/web-app-files/src/composables/scrollTo/useScrollTo.ts b/packages/web-app-files/src/composables/scrollTo/useScrollTo.ts index ad87357bf1c..9362c44f462 100644 --- a/packages/web-app-files/src/composables/scrollTo/useScrollTo.ts +++ b/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)) diff --git a/packages/web-app-files/tests/unit/composables/actions/files/useFileActionsCreateNewFolder.spec.ts b/packages/web-app-files/tests/unit/composables/actions/files/useFileActionsCreateNewFolder.spec.ts index d7389c65eba..5077d02915e 100644 --- a/packages/web-app-files/tests/unit/composables/actions/files/useFileActionsCreateNewFolder.spec.ts +++ b/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({ name: 'files-spaces-generic' }) diff --git a/packages/web-app-files/tests/unit/composables/scrollTo/useScrollTo.spec.ts b/packages/web-app-files/tests/unit/composables/scrollTo/useScrollTo.spec.ts index c818afd4621..2bbcf50a695 100644 --- a/packages/web-app-files/tests/unit/composables/scrollTo/useScrollTo.spec.ts +++ b/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()) + scrollToResource(mockResourceId) expect(htmlPageObject.scrollIntoView).not.toHaveBeenCalled() }, { mocks, provide: mocks, store: defaultStoreMockOptions } @@ -36,6 +42,8 @@ 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() @@ -43,18 +51,17 @@ describe('useScrollTo', () => { getComposableWrapper( () => { const { scrollToResource } = useScrollTo() - scrollToResource(mockDeep()) + 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() @@ -62,8 +69,8 @@ describe('useScrollTo', () => { getComposableWrapper( () => { const { scrollToResource } = useScrollTo() - scrollToResource(mockDeep()) - expect(htmlPageObject.scrollBy).toHaveBeenCalled() + scrollToResource(mockResourceId) + expect(htmlPageObject.scrollIntoView).toHaveBeenCalled() }, { mocks, provide: mocks, store: defaultStoreMockOptions } )