Merge pull request #53824 from nextcloud/fix/FileList-render

apps/files/src/components/FilesListHeader.vue

@@ -12,6 +12,8 @@
 import type { Folder, Header, View } from '@nextcloud/files'
 import type { PropType } from 'vue'
 
+import logger from '../logger.ts'
+
 /**
  * This component is used to render custom
  * elements provided by an API. Vue doesn't allow
@@ -51,8 +53,12 @@ export default {
 		},
 	},
 	mounted() {
-		console.debug('Mounted', this.header.id)
+		logger.debug(`Mounted ${this.header.id} FilesListHeader`, { header: this.header })
 		this.header.render(this.$refs.mount as HTMLElement, this.currentFolder, this.currentView)
 	},
+
+	destroyed() {
+		logger.debug(`Destroyed ${this.header.id} FilesListHeader`, { header: this.header })
+	},
 }
 </script>

apps/files/src/components/FilesListVirtual.vue

@@ -48,6 +48,11 @@
 				:nodes="nodes" />
 		</template>
 
+		<!-- Body replacement if no files are available -->
+		<template #empty>
+			<slot name="empty" />
+		</template>
+
 		<!-- Tfoot-->
 		<template #footer>
 			<FilesListTableFooter :current-view="currentView"
@@ -65,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'
@@ -81,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'
@@ -90,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',
@@ -152,7 +156,6 @@ export default defineComponent({
 			FileEntry,
 			FileEntryGrid,
 			scrollToIndex: 0,
-			openFileId: null as number|null,
 		}
 	},
 
@@ -217,39 +220,26 @@ export default defineComponent({
 		isNoneSelected() {
 			return this.selectedNodes.length === 0
 		},
+
+		isEmpty() {
+			return this.nodes.length === 0
+		},
 	},
 
 	watch: {
-		fileId: {
-			handler(fileId) {
-				this.scrollToFile(fileId, false)
-			},
-			immediate: true,
+		// If nodes gets populated and we have a fileId,
+		// an openFile or openDetails, we fire the appropriate actions.
+		isEmpty() {
+			this.handleOpenQueries()
 		},
-
-		openFile: {
-			handler(openFile) {
-				if (!openFile || !this.fileId) {
-					return
-				}
-
-				this.handleOpenFile(this.fileId)
-			},
-			immediate: true,
+		fileId() {
+			this.handleOpenQueries()
 		},
-
-		openDetails: {
-			handler(openDetails) {
-				// wait for scrolling and updating the actions to settle
-				this.$nextTick(() => {
-					if (!openDetails || !this.fileId) {
-						return
-					}
-
-					this.openSidebarForFile(this.fileId)
-				})
-			},
-			immediate: true,
+		openFile() {
+			this.handleOpenQueries()
+		},
+		openDetails() {
+			this.handleOpenQueries()
 		},
 	},
 
@@ -279,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.
@@ -288,7 +305,7 @@ export default defineComponent({
 				sidebarAction.exec(node, this.currentView, this.currentFolder.path)
 				return
 			}
-			logger.error(`Failed to open sidebar on file ${fileId}, file isn't cached yet !`, { fileId, node })
+			logger.warn(`Failed to open sidebar on file ${fileId}, file isn't cached yet !`, { fileId, node })
 		},
 
 		scrollToFile(fileId: number|null, warn = true) {
@@ -304,6 +321,7 @@ export default defineComponent({
 				}
 
 				this.scrollToIndex = Math.max(0, index)
+				logger.debug('Scrolling to file ' + fileId, { fileId, index })
 			}
 		},
 
@@ -368,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) {
@@ -474,6 +490,8 @@ export default defineComponent({
 	--icon-preview-size: 32px;
 
 	--fixed-block-start-position: var(--default-clickable-area);
+	display: flex;
+	flex-direction: column;
 	overflow: auto;
 	height: 100%;
 	will-change: scroll-position;
@@ -521,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 {
@@ -570,6 +595,16 @@ export default defineComponent({
 			top: var(--fixed-block-start-position);
 		}
 
+		// Empty content
+		.files-list__empty {
+			display: flex;
+			flex-direction: column;
+			align-items: center;
+			justify-content: center;
+			width: 100%;
+			height: 100%;
+		}
+
 		tr {
 			position: relative;
 			display: flex;

apps/files/src/components/VirtualList.vue

@@ -20,7 +20,18 @@
 			<slot name="header-overlay" />
 		</div>
 
-		<table class="files-list__table" :class="{ 'files-list__table--with-thead-overlay': !!$scopedSlots['header-overlay'] }">
+		<div v-if="dataSources.length === 0"
+			class="files-list__empty">
+			<slot name="empty" />
+		</div>
+
+		<table :aria-hidden="dataSources.length === 0"
+			:inert="dataSources.length === 0"
+			class="files-list__table"
+			:class="{
+				'files-list__table--with-thead-overlay': !!$scopedSlots['header-overlay'],
+				'files-list__table--hidden': dataSources.length === 0,
+			}">
 			<!-- Accessibility table caption for screen readers -->
 			<caption v-if="caption" class="hidden-visually">
 				{{ caption }}
@@ -309,7 +320,7 @@ export default defineComponent({
 
 	methods: {
 		scrollTo(index: number) {
-			if (!this.$el) {
+			if (!this.$el || this.index === index) {
 				return
 			}
 

apps/files/src/views/FilesList.vue

@@ -73,87 +73,92 @@
 		<!-- Drag and drop notice -->
 		<DragAndDropNotice v-if="!loading && canUpload && currentFolder" :current-folder="currentFolder" />
 
-		<!-- Initial loading -->
-		<NcLoadingIcon v-if="loading && !isRefreshing"
+		<!--
+			Initial current view loading0. This should never happen,
+			views are supposed to be registered far earlier in the lifecycle.
+			In case the URL is bad or a view is missing, we show a loading icon.
+		-->
+		<NcLoadingIcon v-if="!currentView"
 			class="files-list__loading-icon"
 			:size="38"
 			:name="t('files', 'Loading current folder')" />
 
-		<!-- Empty content placeholder -->
-		<template v-else-if="!loading && isEmptyDir && currentFolder && currentView">
-			<div class="files-list__before">
-				<!-- Headers -->
-				<FilesListHeader v-for="header in headers"
-					:key="header.id"
-					:current-folder="currentFolder"
-					:current-view="currentView"
-					:header="header" />
-			</div>
-			<!-- Empty due to error -->
-			<NcEmptyContent v-if="error" :name="error" data-cy-files-content-error>
-				<template #action>
-					<NcButton type="secondary" @click="fetchContent">
-						<template #icon>
-							<IconReload :size="20" />
-						</template>
-						{{ t('files', 'Retry') }}
-					</NcButton>
-				</template>
-				<template #icon>
-					<IconAlertCircleOutline />
-				</template>
-			</NcEmptyContent>
-			<!-- Custom empty view -->
-			<div v-else-if="currentView?.emptyView" class="files-list__empty-view-wrapper">
-				<div ref="customEmptyView" />
-			</div>
-			<!-- Default empty directory view -->
-			<NcEmptyContent v-else
-				:name="currentView?.emptyTitle || t('files', 'No files in here')"
-				:description="currentView?.emptyCaption || t('files', 'Upload some content or sync with your devices!')"
-				data-cy-files-content-empty>
-				<template v-if="directory !== '/'" #action>
-					<!-- Uploader -->
-					<UploadPicker v-if="canUpload && !isQuotaExceeded"
-						allow-folders
-						class="files-list__header-upload-button"
-						:content="getContent"
-						:destination="currentFolder"
-						:forbidden-characters="forbiddenCharacters"
-						multiple
-						@failed="onUploadFail"
-						@uploaded="onUpload" />
-					<NcButton v-else :to="toPreviousDir" type="primary">
-						{{ t('files', 'Go back') }}
-					</NcButton>
-				</template>
-				<template #icon>
-					<NcIconSvgWrapper :svg="currentView.icon" />
-				</template>
-			</NcEmptyContent>
-		</template>
-
-		<!-- File list -->
+		<!-- File list - always mounted -->
 		<FilesListVirtual v-else
 			ref="filesListVirtual"
 			:current-folder="currentFolder"
 			:current-view="currentView"
 			:nodes="dirContentsSorted"
-			:summary="summary" />
+			:summary="summary">
+			<template #empty>
+				<!-- Initial loading -->
+				<NcLoadingIcon v-if="loading && !isRefreshing"
+					class="files-list__loading-icon"
+					:size="38"
+					:name="t('files', 'Loading current folder')" />
+
+				<!-- Empty due to error -->
+				<NcEmptyContent v-else-if="error" :name="error" data-cy-files-content-error>
+					<template #action>
+						<NcButton type="secondary" @click="fetchContent">
+							<template #icon>
+								<IconReload :size="20" />
+							</template>
+							{{ t('files', 'Retry') }}
+						</NcButton>
+					</template>
+					<template #icon>
+						<IconAlertCircleOutline />
+					</template>
+				</NcEmptyContent>
+
+				<!-- Custom empty view -->
+				<div v-else-if="currentView?.emptyView" class="files-list__empty-view-wrapper">
+					<div ref="customEmptyView" />
+				</div>
+
+				<!-- Default empty directory view -->
+				<NcEmptyContent v-else
+					:name="currentView?.emptyTitle || t('files', 'No files in here')"
+					:description="currentView?.emptyCaption || t('files', 'Upload some content or sync with your devices!')"
+					data-cy-files-content-empty>
+					<template v-if="directory !== '/'" #action>
+						<!-- Uploader -->
+						<UploadPicker v-if="canUpload && !isQuotaExceeded"
+							allow-folders
+							class="files-list__header-upload-button"
+							:content="getContent"
+							:destination="currentFolder"
+							:forbidden-characters="forbiddenCharacters"
+							multiple
+							@failed="onUploadFail"
+							@uploaded="onUpload" />
+						<NcButton v-else :to="toPreviousDir" type="primary">
+							{{ t('files', 'Go back') }}
+						</NcButton>
+					</template>
+					<template #icon>
+						<NcIconSvgWrapper :svg="currentView?.icon" />
+					</template>
+				</NcEmptyContent>
+			</template>
+		</FilesListVirtual>
 	</NcAppContent>
 </template>
 
 <script lang="ts">
-import type { ContentsWithRoot, FileListAction, Folder, INode } from '@nextcloud/files'
+import type { ContentsWithRoot, FileListAction, INode } from '@nextcloud/files'
 import type { Upload } from '@nextcloud/upload'
 import type { CancelablePromise } from 'cancelable-promise'
 import type { ComponentPublicInstance } from 'vue'
 import type { Route } from 'vue-router'
 import type { UserConfig } from '../types.ts'
 
+import { getCurrentUser } from '@nextcloud/auth'
 import { getCapabilities } from '@nextcloud/capabilities'
 import { emit, subscribe, unsubscribe } from '@nextcloud/event-bus'
-import { Node, Permission, sortNodes, getFileListActions } from '@nextcloud/files'
+import { Folder, Node, Permission, sortNodes, getFileListActions } from '@nextcloud/files'
+import { getRemoteURL, getRootPath } from '@nextcloud/files/dav'
 import { translate as t } from '@nextcloud/l10n'
 import { join, dirname, normalize, relative } from 'path'
 import { showError, showSuccess, showWarning } from '@nextcloud/dialogs'
@@ -179,23 +184,22 @@ import ListViewIcon from 'vue-material-design-icons/FormatListBulletedSquare.vue
 import ViewGridIcon from 'vue-material-design-icons/ViewGrid.vue'
 
 import { action as sidebarAction } from '../actions/sidebarAction.ts'
-import { getSummaryFor } from '../utils/fileUtils.ts'
-import { humanizeWebDAVError } from '../utils/davUtils.ts'
-import { useFileListHeaders } from '../composables/useFileListHeaders.ts'
 import { useFileListWidth } from '../composables/useFileListWidth.ts'
-import { useFilesStore } from '../store/files.ts'
-import { useFiltersStore } from '../store/filters.ts'
 import { useNavigation } from '../composables/useNavigation.ts'
-import { usePathsStore } from '../store/paths.ts'
 import { useRouteParameters } from '../composables/useRouteParameters.ts'
 import { useActiveStore } from '../store/active.ts'
+import { useFilesStore } from '../store/files.ts'
+import { useFiltersStore } from '../store/filters.ts'
+import { usePathsStore } from '../store/paths.ts'
 import { useSelectionStore } from '../store/selection.ts'
 import { useUploaderStore } from '../store/uploader.ts'
 import { useUserConfigStore } from '../store/userconfig.ts'
 import { useViewConfigStore } from '../store/viewConfig.ts'
+import { humanizeWebDAVError } from '../utils/davUtils.ts'
+import { getSummaryFor } from '../utils/fileUtils.ts'
+import { defaultView } from '../utils/filesViews.ts'
 import BreadCrumbs from '../components/BreadCrumbs.vue'
 import DragAndDropNotice from '../components/DragAndDropNotice.vue'
-import FilesListHeader from '../components/FilesListHeader.vue'
 import FilesListVirtual from '../components/FilesListVirtual.vue'
 import filesSortingMixin from '../mixins/filesSorting.ts'
 import logger from '../logger.ts'
@@ -208,7 +212,6 @@ export default defineComponent({
 	components: {
 		BreadCrumbs,
 		DragAndDropNotice,
-		FilesListHeader,
 		FilesListVirtual,
 		LinkIcon,
 		ListViewIcon,
@@ -259,7 +262,6 @@ export default defineComponent({
 			directory,
 			fileId,
 			fileListWidth,
-			headers: useFileListHeaders(),
 			t,
 
 			activeStore,
@@ -325,12 +327,23 @@ export default defineComponent({
 		/**
 		 * The current folder.
 		 */
-		currentFolder(): Folder | undefined {
-			if (!this.currentView) {
-				return
+		currentFolder(): Folder {
+			// Temporary fake folder to use until we have the first valid folder
+			// fetched and cached. This allow us to mount the FilesListVirtual
+			// at all time and avoid unmount/mount and undesired rendering issues.
+			const dummyFolder = new Folder({
+				id: 0,
+				source: getRemoteURL() + getRootPath(),
+				root: getRootPath(),
+				owner: getCurrentUser()?.uid || null,
+				permissions: Permission.NONE,
+			})
+
+			if (!this.currentView?.id) {
+				return dummyFolder
 			}
 
-			return this.filesStore.getDirectoryByPath(this.currentView.id, this.directory)
+			return this.filesStore.getDirectoryByPath(this.currentView.id, this.directory) || dummyFolder
 		},
 
 		dirContents(): Node[] {
@@ -342,7 +355,7 @@ export default defineComponent({
 		/**
 		 * The current directory contents.
 		 */
-		dirContentsSorted() {
+		dirContentsSorted(): INode[] {
 			if (!this.currentView) {
 				return []
 			}
@@ -597,10 +610,21 @@ export default defineComponent({
 			const currentView = this.currentView
 
 			if (!currentView) {
-				logger.debug('The current view doesn\'t exists or is not ready.', { currentView })
+				logger.debug('The current view does not exists or is not ready.', { currentView })
+
+				// If we still haven't a valid view, let's wait for the page to load
+				// then try again. Else redirect to the default view
+				window.addEventListener('DOMContentLoaded', () => {
+					if (!this.currentView) {
+						logger.warn('No current view after DOMContentLoaded, redirecting to the default view')
+						window.OCP.Files.Router.goToRoute(null, { view: defaultView() })
+					}
+				}, { once: true })
 				return
 			}
 
+			logger.debug('Fetching contents for directory', { dir, currentView })
+
 			// If we have a cancellable promise ongoing, cancel it
 			if (this.promise && 'cancel' in this.promise) {
 				this.promise.cancel()

cypress/e2e/files/FilesUtils.ts

@@ -293,6 +293,12 @@ export function enableGridMode() {
 export function calculateViewportHeight(rows: number): Cypress.Chainable<number> {
 	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) => {

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<number, string>()
-	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<number, string>()
-	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<HTMLElement>, expected = true) {
 function notBeOverlappedByTableHeader($el: JQuery<HTMLElement>) {
 	return beOverlappedByTableHeader($el, false)
 }
+
+function initFilesAndViewport(fileIds: Map<number, string>, gridMode = false): Cypress.Chainable<number> {
+	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)
+			})
+	})
+}