From 8f3019cf86438484c4d8ec6896952d07574edf43 Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Mon, 7 Jul 2025 10:41:18 +0200 Subject: [PATCH] fix(files): VirtualList rendering for scrolling calculations Signed-off-by: skjnldsv --- .../files/src/components/FilesListVirtual.vue | 85 +++++++++++------- apps/files/src/components/VirtualList.vue | 10 ++- cypress/e2e/files/FilesUtils.ts | 6 ++ cypress/e2e/files/scrolling.cy.ts | 86 +++++++++---------- 4 files changed, 105 insertions(+), 82 deletions(-) diff --git a/apps/files/src/components/FilesListVirtual.vue b/apps/files/src/components/FilesListVirtual.vue index 9645b3fd42b..9022a597d08 100644 --- a/apps/files/src/components/FilesListVirtual.vue +++ b/apps/files/src/components/FilesListVirtual.vue @@ -70,7 +70,6 @@ import type { UserConfig } from '../types' import type { Node as NcNode } from '@nextcloud/files' import type { ComponentPublicInstance, PropType } from 'vue' -import type { Location } from 'vue-router' import { Folder, Permission, View, getFileActions, FileType } from '@nextcloud/files' import { showError } from '@nextcloud/dialogs' @@ -86,6 +85,7 @@ import { useFileListWidth } from '../composables/useFileListWidth.ts' import { useRouteParameters } from '../composables/useRouteParameters.ts' import { useSelectionStore } from '../store/selection.js' import { useUserConfigStore } from '../store/userconfig.ts' +import logger from '../logger.ts' import FileEntry from './FileEntry.vue' import FileEntryGrid from './FileEntryGrid.vue' @@ -95,7 +95,6 @@ import FilesListTableFooter from './FilesListTableFooter.vue' import FilesListTableHeader from './FilesListTableHeader.vue' import FilesListTableHeaderActions from './FilesListTableHeaderActions.vue' import VirtualList from './VirtualList.vue' -import logger from '../logger.ts' export default defineComponent({ name: 'FilesListVirtual', @@ -230,28 +229,17 @@ export default defineComponent({ watch: { // If nodes gets populated and we have a fileId, // an openFile or openDetails, we fire the appropriate actions. - isEmpty(isEmpty: boolean) { - if (isEmpty || !this.fileId) { - return - } - - logger.debug('FilesListVirtual: nodes populated, checking for requested fileId, openFile or openDetails again', { - fileId: this.fileId, - openFile: this.openFile, - openDetails: this.openDetails, - }) - - if (this.openFile) { - this.handleOpenFile(this.fileId) - } - - if (this.openDetails) { - this.openSidebarForFile(this.fileId) - } - - if (this.fileId) { - this.scrollToFile(this.fileId, false) - } + isEmpty() { + this.handleOpenQueries() + }, + fileId() { + this.handleOpenQueries() + }, + openFile() { + this.handleOpenQueries() + }, + openDetails() { + this.handleOpenQueries() }, }, @@ -281,6 +269,33 @@ export default defineComponent({ }, methods: { + handleOpenQueries() { + // If the list is empty, or we don't have a fileId, + // there's nothing to be done. + if (this.isEmpty || !this.fileId) { + return + } + + logger.debug('FilesListVirtual: checking for requested fileId, openFile or openDetails', { + nodes: this.nodes, + fileId: this.fileId, + openFile: this.openFile, + openDetails: this.openDetails, + }) + + if (this.openFile) { + this.handleOpenFile(this.fileId) + } + + if (this.openDetails) { + this.openSidebarForFile(this.fileId) + } + + if (this.fileId) { + this.scrollToFile(this.fileId, false) + } + }, + openSidebarForFile(fileId) { // Open the sidebar for the given URL fileid // iif we just loaded the app. @@ -306,6 +321,7 @@ export default defineComponent({ } this.scrollToIndex = Math.max(0, index) + logger.debug('Scrolling to file ' + fileId, { fileId, index }) } }, @@ -370,15 +386,13 @@ export default defineComponent({ } // The file is either a folder or has no default action other than downloading // in this case we need to open the details instead and remove the route from the history - const query = this.$route.query - delete query.openfile - query.opendetails = '' - logger.debug('Ignore `openfile` query and replacing with `opendetails` for ' + node.path, { node }) - await this.$router.replace({ - ...(this.$route as Location), - query, - }) + window.OCP.Files.Router.goToRoute( + null, + this.$route.params, + { ...this.$route.query, openfile: undefined, opendetails: '' }, + true, // silent update of the URL + ) }, onDragOver(event: DragEvent) { @@ -525,6 +539,13 @@ export default defineComponent({ // Hide the table header below the overlay margin-block-start: calc(-1 * var(--row-height)); } + + // Visually hide the table when there are no files + &--hidden { + visibility: hidden; + z-index: -1; + opacity: 0; + } } .files-list__filters { diff --git a/apps/files/src/components/VirtualList.vue b/apps/files/src/components/VirtualList.vue index d9000a38073..4f9d8096580 100644 --- a/apps/files/src/components/VirtualList.vue +++ b/apps/files/src/components/VirtualList.vue @@ -25,11 +25,13 @@ - + :class="{ + 'files-list__table--with-thead-overlay': !!$scopedSlots['header-overlay'], + 'files-list__table--hidden': dataSources.length === 0, + }">
{{ caption }} @@ -318,7 +320,7 @@ export default defineComponent({ methods: { scrollTo(index: number) { - if (!this.$el) { + if (!this.$el || this.index === index) { return } diff --git a/cypress/e2e/files/FilesUtils.ts b/cypress/e2e/files/FilesUtils.ts index 970eb636f58..b138d161600 100644 --- a/cypress/e2e/files/FilesUtils.ts +++ b/cypress/e2e/files/FilesUtils.ts @@ -293,6 +293,12 @@ export function enableGridMode() { export function calculateViewportHeight(rows: number): Cypress.Chainable { cy.visit('/apps/files') + cy.get('[data-cy-files-list]') + .should('be.visible') + + cy.get('[data-cy-files-list-tbody] tr', { timeout: 5000 }) + .and('be.visible') + return cy.get('[data-cy-files-list]') .should('be.visible') .then((filesList) => { diff --git a/cypress/e2e/files/scrolling.cy.ts b/cypress/e2e/files/scrolling.cy.ts index b86f9b796ba..f7a4ef683f5 100644 --- a/cypress/e2e/files/scrolling.cy.ts +++ b/cypress/e2e/files/scrolling.cy.ts @@ -3,36 +3,23 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { User } from '@nextcloud/cypress' import { calculateViewportHeight, enableGridMode, getRowForFile } from './FilesUtils.ts' import { beFullyInViewport, notBeFullyInViewport } from '../core-utils.ts' -describe('files: Scrolling to selected file in file list', { testIsolation: true }, () => { +describe('files: Scrolling to selected file in file list', () => { const fileIds = new Map() - let user: User let viewportHeight: number before(() => { - cy.createRandomUser().then(($user) => { - user = $user - - cy.rm(user, '/welcome.txt') - for (let i = 1; i <= 10; i++) { - cy.uploadContent(user, new Blob([]), 'text/plain', `/${i}.txt`) - .then((response) => fileIds.set(i, Number.parseInt(response.headers['oc-fileid']).toString())) - } - - cy.login(user) - cy.viewport(1200, 800) - // Calculate height to ensure that those 10 elements can not be rendered in one list (only 6 will fit the screen) - calculateViewportHeight(6) - .then((height) => { viewportHeight = height }) - }) + initFilesAndViewport(fileIds) + .then((_viewportHeight) => { + cy.log(`Saving viewport height to ${_viewportHeight}px`) + viewportHeight = _viewportHeight + }) }) beforeEach(() => { cy.viewport(1200, viewportHeight) - cy.login(user) }) it('Can see first file in list', () => { @@ -123,41 +110,17 @@ describe('files: Scrolling to selected file in file list', { testIsolation: true } }) -describe('files: Scrolling to selected file in file list (GRID MODE)', { testIsolation: true }, () => { +describe('files: Scrolling to selected file in file list (GRID MODE)', () => { const fileIds = new Map() - let user: User let viewportHeight: number before(() => { - cy.wrap(Cypress.automation('remote:debugger:protocol', { - command: 'Network.clearBrowserCache', - })) - - cy.createRandomUser().then(($user) => { - user = $user - - cy.rm(user, '/welcome.txt') - for (let i = 1; i <= 12; i++) { - cy.uploadContent(user, new Blob([]), 'text/plain', `/${i}.txt`) - .then((response) => fileIds.set(i, Number.parseInt(response.headers['oc-fileid']).toString())) - } - - // Set grid mode - cy.login(user) - cy.visit('/apps/files') - enableGridMode() - - // 768px width will limit the columns to 3 - cy.viewport(768, 800) - // Calculate height to ensure that those 12 elements can not be rendered in one list (only 3 will fit the screen) - calculateViewportHeight(3) - .then((height) => { viewportHeight = height }) - }) + initFilesAndViewport(fileIds, true) + .then((_viewportHeight) => { viewportHeight = _viewportHeight }) }) beforeEach(() => { cy.viewport(768, viewportHeight) - cy.login(user) }) // First row @@ -288,3 +251,34 @@ function beOverlappedByTableHeader($el: JQuery, expected = true) { function notBeOverlappedByTableHeader($el: JQuery) { return beOverlappedByTableHeader($el, false) } + +function initFilesAndViewport(fileIds: Map, gridMode = false): Cypress.Chainable { + return cy.createRandomUser().then((user) => { + cy.rm(user, '/welcome.txt') + + // Create files with names 1.txt, 2.txt, ..., 10.txt + const count = gridMode ? 12 : 10 + for (let i = 1; i <= count; i++) { + cy.uploadContent(user, new Blob([]), 'text/plain', `/${i}.txt`) + .then((response) => fileIds.set(i, Number.parseInt(response.headers['oc-fileid']).toString())) + } + + cy.login(user) + cy.viewport(1200, 800) + + cy.visit('/apps/files') + + // If grid mode is requested, enable it + if (gridMode) { + enableGridMode() + } + + // Calculate height to ensure that those 10 elements can not be rendered in one list (only 6 will fit the screen, 3 in grid mode) + return calculateViewportHeight(gridMode ? 3 : 6) + .then((height) => { + // Set viewport height to the calculated height + cy.log(`Setting viewport height to ${height}px`) + cy.wrap(height) + }) + }) +}