Merge pull request #46768 from nextcloud/fix/files-actions

fix(files): Provide default file action for file entry name (on click action)

apps/files/src/actions/downloadAction.spec.ts

@@ -4,7 +4,7 @@
  */
 import { action } from './downloadAction'
 import { expect } from '@jest/globals'
-import { File, Folder, Permission, View, FileAction } from '@nextcloud/files'
+import { File, Folder, Permission, View, FileAction, DefaultType } from '@nextcloud/files'
 
 const view = {
 	id: 'files',
@@ -23,7 +23,7 @@ describe('Download action conditions tests', () => {
 		expect(action.id).toBe('download')
 		expect(action.displayName([], view)).toBe('Download')
 		expect(action.iconSvgInline([], view)).toBe('<svg>SvgMock</svg>')
-		expect(action.default).toBeUndefined()
+		expect(action.default).toBe(DefaultType.DEFAULT)
 		expect(action.order).toBe(30)
 	})
 })

apps/files/src/actions/downloadAction.ts

@@ -4,9 +4,9 @@
  */
 import type { ShareAttribute } from '../../../files_sharing/src/sharing'
 
-import { FileAction, Permission, Node, FileType, View } from '@nextcloud/files'
+import { FileAction, Permission, Node, FileType, View, DefaultType } from '@nextcloud/files'
+import { t } from '@nextcloud/l10n'
 import { generateUrl } from '@nextcloud/router'
-import { translate as t } from '@nextcloud/l10n'
 
 import ArrowDownSvg from '@mdi/svg/svg/arrow-down.svg?raw'
 
@@ -46,6 +46,8 @@ const isDownloadable = function(node: Node) {
 
 export const action = new FileAction({
 	id: 'download',
+	default: DefaultType.DEFAULT,
+
 	displayName: () => t('files', 'Download'),
 	iconSvgInline: () => ArrowDownSvg,
 

apps/files/src/components/FileEntry/FileEntryActions.vue

@@ -79,10 +79,10 @@
 import type { PropType, ShallowRef } from 'vue'
 import type { FileAction, Node, View } from '@nextcloud/files'
 
-import { DefaultType, NodeStatus, getFileActions } from '@nextcloud/files'
+import { DefaultType, NodeStatus } from '@nextcloud/files'
 import { showError, showSuccess } from '@nextcloud/dialogs'
 import { translate as t } from '@nextcloud/l10n'
-import { defineComponent } from 'vue'
+import { defineComponent, inject } from 'vue'
 
 import NcActionButton from '@nextcloud/vue/dist/Components/NcActionButton.js'
 import NcActions from '@nextcloud/vue/dist/Components/NcActions.js'
@@ -95,9 +95,6 @@ import CustomElementRender from '../CustomElementRender.vue'
 import { useNavigation } from '../../composables/useNavigation'
 import logger from '../../logger.js'
 
-// The registered actions list
-const actions = getFileActions()
-
 export default defineComponent({
 	name: 'FileEntryActions',
 
@@ -136,10 +133,12 @@ export default defineComponent({
 
 	setup() {
 		const { currentView } = useNavigation()
+		const enabledFileActions = inject<FileAction[]>('enabledFileActions', [])
 
 		return {
 			// The file list is guaranteed to be only shown with active view
 			currentView: currentView as ShallowRef<View>,
+			enabledFileActions,
 		}
 	},
 
@@ -158,23 +157,12 @@ export default defineComponent({
 			return this.source.status === NodeStatus.LOADING
 		},
 
-		// Sorted actions that are enabled for this node
-		enabledActions() {
-			if (this.source.status === NodeStatus.FAILED) {
-				return []
-			}
-
-			return actions
-				.filter(action => !action.enabled || action.enabled([this.source], this.currentView))
-				.sort((a, b) => (a.order || 0) - (b.order || 0))
-		},
-
 		// Enabled action that are displayed inline
 		enabledInlineActions() {
 			if (this.filesListWidth < 768 || this.gridMode) {
 				return []
 			}
-			return this.enabledActions.filter(action => action?.inline?.(this.source, this.currentView))
+			return this.enabledFileActions.filter(action => action?.inline?.(this.source, this.currentView))
 		},
 
 		// Enabled action that are displayed inline with a custom render function
@@ -182,12 +170,7 @@ export default defineComponent({
 			if (this.gridMode) {
 				return []
 			}
-			return this.enabledActions.filter(action => typeof action.renderInline === 'function')
-		},
-
-		// Default actions
-		enabledDefaultActions() {
-			return this.enabledActions.filter(action => !!action?.default)
+			return this.enabledFileActions.filter(action => typeof action.renderInline === 'function')
 		},
 
 		// Actions shown in the menu
@@ -202,7 +185,7 @@ export default defineComponent({
 				// Showing inline first for the NcActions inline prop
 				...this.enabledInlineActions,
 				// Then the rest
-				...this.enabledActions.filter(action => action.default !== DefaultType.HIDDEN && typeof action.renderInline !== 'function'),
+				...this.enabledFileActions.filter(action => action.default !== DefaultType.HIDDEN && typeof action.renderInline !== 'function'),
 			].filter((value, index, self) => {
 				// Then we filter duplicates to prevent inline actions to be shown twice
 				return index === self.findIndex(action => action.id === value.id)
@@ -216,7 +199,7 @@ export default defineComponent({
 		},
 
 		enabledSubmenuActions() {
-			return this.enabledActions
+			return this.enabledFileActions
 				.filter(action => action.parent)
 				.reduce((arr, action) => {
 					if (!arr[action.parent!]) {
@@ -305,14 +288,6 @@ export default defineComponent({
 				}
 			}
 		},
-		execDefaultAction(event) {
-			if (this.enabledDefaultActions.length > 0) {
-				event.preventDefault()
-				event.stopPropagation()
-				// Execute the first default action if any
-				this.enabledDefaultActions[0].exec(this.source, this.currentView, this.currentDir)
-			}
-		},
 
 		isMenu(id: string) {
 			return this.enabledSubmenuActions[id]?.length > 0

apps/files/src/components/FileEntry/FileEntryName.vue

@@ -37,19 +37,20 @@
 </template>
 
 <script lang="ts">
-import type { Node } from '@nextcloud/files'
+import type { FileAction, Node } from '@nextcloud/files'
 import type { PropType } from 'vue'
 
 import axios, { isAxiosError } from '@nextcloud/axios'
 import { showError, showSuccess } from '@nextcloud/dialogs'
 import { emit } from '@nextcloud/event-bus'
-import { FileType, NodeStatus, Permission } from '@nextcloud/files'
+import { FileType, NodeStatus } from '@nextcloud/files'
 import { translate as t } from '@nextcloud/l10n'
-import { defineComponent } from 'vue'
+import { defineComponent, inject } from 'vue'
 
 import NcTextField from '@nextcloud/vue/dist/Components/NcTextField.js'
 
 import { useNavigation } from '../../composables/useNavigation'
+import { useRouteParameters } from '../../composables/useRouteParameters.ts'
 import { useRenamingStore } from '../../store/renaming.ts'
 import { getFilenameValidity } from '../../utils/filenameValidity.ts'
 import logger from '../../logger.js'
@@ -96,10 +97,15 @@ export default defineComponent({
 
 	setup() {
 		const { currentView } = useNavigation()
+		const { directory } = useRouteParameters()
 		const renamingStore = useRenamingStore()
 
+		const defaultFileAction = inject<FileAction | undefined>('defaultFileAction')
+
 		return {
 			currentView,
+			defaultFileAction,
+			directory,
 
 			renamingStore,
 		}
@@ -139,32 +145,20 @@ export default defineComponent({
 				}
 			}
 
-			const enabledDefaultActions = this.$parent?.$refs?.actions?.enabledDefaultActions
-			if (enabledDefaultActions?.length > 0) {
-				const action = enabledDefaultActions[0]
-				const displayName = action.displayName([this.source], this.currentView)
+			if (this.defaultFileAction && this.currentView) {
+				const displayName = this.defaultFileAction.displayName([this.source], this.currentView)
 				return {
-					is: 'a',
+					is: 'button',
 					params: {
+						'aria-label': displayName,
 						title: displayName,
-						role: 'button',
-						tabindex: '0',
-					},
-				}
-			}
-
-			if (this.source?.permissions & Permission.READ) {
-				return {
-					is: 'a',
-					params: {
-						download: this.source.basename,
-						href: this.source.source,
-						title: t('files', 'Download file {name}', { name: `${this.basename}${this.extension}` }),
 						tabindex: '0',
 					},
 				}
 			}
 
+			// nothing interactive here, there is no default action
+			// so if not even the download action works we only can show the list entry
 			return {
 				is: 'span',
 			}
@@ -280,12 +274,15 @@ export default defineComponent({
 				// Reset the renaming store
 				this.stopRenaming()
 				this.$nextTick(() => {
-					this.$refs.basename?.focus()
+					const nameContainter = this.$refs.basename as HTMLElement | undefined
+					nameContainter?.focus()
 				})
 			} catch (error) {
 				logger.error('Error while renaming file', { error })
+				// Rename back as it failed
 				this.source.rename(oldName)
-				this.$refs.renameInput?.focus()
+				// And ensure we reset to the renaming state
+				this.startRenaming()
 
 				if (isAxiosError(error)) {
 					// TODO: 409 means current folder does not exist, redirect ?
@@ -293,7 +290,7 @@ export default defineComponent({
 						showError(t('files', 'Could not rename "{oldName}", it does not exist any more', { oldName }))
 						return
 					} else if (error?.response?.status === 412) {
-						showError(t('files', 'The name "{newName}" is already used in the folder "{dir}". Please choose a different name.', { newName, dir: this.currentDir }))
+						showError(t('files', 'The name "{newName}" is already used in the folder "{dir}". Please choose a different name.', { newName, dir: this.directory }))
 						return
 					}
 				}
@@ -309,3 +306,16 @@ export default defineComponent({
 	},
 })
 </script>
+
+<style scoped lang="scss">
+button.files-list__row-name-link {
+	background-color: unset;
+	border: none;
+	font-weight: normal;
+
+	&:active {
+		// No active styles - handled by the row entry
+		background-color: unset !important;
+	}
+}
+</style>

apps/files/src/components/FileEntryMixin.ts

@@ -3,11 +3,11 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-import type { ComponentPublicInstance, PropType } from 'vue'
+import type { PropType } from 'vue'
 import type { FileSource } from '../types.ts'
 
 import { showError } from '@nextcloud/dialogs'
-import { FileType, Permission, Folder, File as NcFile, NodeStatus, Node } from '@nextcloud/files'
+import { FileType, Permission, Folder, File as NcFile, NodeStatus, Node, getFileActions } from '@nextcloud/files'
 import { translate as t } from '@nextcloud/l10n'
 import { generateUrl } from '@nextcloud/router'
 import { vOnClickOutside } from '@vueuse/components'
@@ -19,10 +19,11 @@ import { getDragAndDropPreview } from '../utils/dragUtils.ts'
 import { hashCode } from '../utils/hashUtils.ts'
 import { dataTransferToFileTree, onDropExternalFiles, onDropInternalFiles } from '../services/DropService.ts'
 import logger from '../logger.js'
-import FileEntryActions from '../components/FileEntry/FileEntryActions.vue'
 
 Vue.directive('onClickOutside', vOnClickOutside)
 
+const actions = getFileActions()
+
 export default defineComponent({
 	props: {
 		source: {
@@ -47,6 +48,13 @@ export default defineComponent({
 		},
 	},
 
+	provide() {
+		return {
+			defaultFileAction: this.defaultFileAction,
+			enabledFileActions: this.enabledFileActions,
+		}
+	},
+
 	data() {
 		return {
 			loading: '',
@@ -178,6 +186,23 @@ export default defineComponent({
 				color: `color-mix(in srgb, var(--color-main-text) ${ratio}%, var(--color-text-maxcontrast))`,
 			}
 		},
+
+		/**
+		 * Sorted actions that are enabled for this node
+		 */
+		enabledFileActions() {
+			if (this.source.status === NodeStatus.FAILED) {
+				return []
+			}
+
+			return actions
+				.filter(action => !action.enabled || action.enabled([this.source], this.currentView))
+				.sort((a, b) => (a.order || 0) - (b.order || 0))
+		},
+
+		defaultFileAction() {
+			return this.enabledFileActions.find((action) => action.default !== undefined)
+		},
 	},
 
 	watch: {
@@ -261,8 +286,15 @@ export default defineComponent({
 				return false
 			}
 
-			const actions = this.$refs.actions as ComponentPublicInstance<typeof FileEntryActions>
-			actions.execDefaultAction(event)
+			if (this.defaultFileAction) {
+				event.preventDefault()
+				event.stopPropagation()
+				// Execute the first default action if any
+				this.defaultFileAction.exec(this.source, this.currentView, this.currentDir)
+			} else {
+				// fallback to open in current tab
+				window.open(generateUrl('/f/{fileId}', { fileId: this.fileid }), '_self')
+			}
 		},
 
 		openDetailsIfAvailable(event) {

apps/files/src/components/FilesListVirtual.vue

@@ -600,24 +600,26 @@ export default defineComponent({
 			// Take as much space as possible
 			flex: 1 1 auto;
 
-			a {
+			button.files-list__row-name-link {
 				display: flex;
 				align-items: center;
+				text-align: start;
 				// Fill cell height and width
 				width: 100%;
 				height: 100%;
 				// Necessary for flex grow to work
 				min-width: 0;
+				margin: 0;
 
 				// Already added to the inner text, see rule below
 				&:focus-visible {
-					outline: none;
+					outline: none !important;
 				}
 
 				// Keyboard indicator a11y
 				&:focus .files-list__row-name-text {
-					outline: 2px solid var(--color-main-text) !important;
-					border-radius: 20px;
+					outline: var(--border-width-input-focused) solid var(--color-main-text) !important;
+					border-radius: var(--border-radius-element);
 				}
 				&:focus:not(:focus-visible) .files-list__row-name-text {
 					outline: none !important;
@@ -627,7 +629,7 @@ export default defineComponent({
 			.files-list__row-name-text {
 				color: var(--color-main-text);
 				// Make some space for the outline
-				padding: 5px 10px;
+				padding: var(--default-grid-baseline) calc(2 * var(--default-grid-baseline));
 				margin-left: -10px;
 				// Align two name and ext
 				display: inline-flex;
@@ -791,10 +793,6 @@ tbody.files-list__tbody.files-list__tbody--grid {
 			height: var(--icon-preview-size);
 		}
 
-		a.files-list__row-name-link {
-			height: var(--name-height);
-		}
-
 		.files-list__row-name-text {
 			margin: 0;
 			// Ensure that the outline is not too close to the text.

cypress.config.ts

@@ -2,6 +2,7 @@
  * SPDX-FileCopyrightText: 2022 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
+import type { Configuration } from 'webpack'
 import {
 	applyChangesToNextcloud,
 	configureNextcloud,
@@ -11,8 +12,8 @@ import {
 } from './cypress/dockerNode'
 import { defineConfig } from 'cypress'
 import cypressSplit from 'cypress-split'
+import { removeDirectory } from 'cypress-delete-downloads-folder'
 import webpackPreprocessor from '@cypress/webpack-preprocessor'
-import type { Configuration } from 'webpack'
 
 import webpackConfig from './webpack.config.js'
 
@@ -63,6 +64,8 @@ export default defineConfig({
 
 			on('file:preprocessor', webpackPreprocessor({ webpackOptions: webpackConfig as Configuration }))
 
+			on('task', { removeDirectory })
+
 			// Disable spell checking to prevent rendering differences
 			on('before:browser:launch', (browser, launchOptions) => {
 				if (browser.family === 'chromium' && browser.name !== 'electron') {

cypress/e2e/files/files-download.cy.ts

@@ -0,0 +1,145 @@
+/*!
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import type { User } from '@nextcloud/cypress'
+import { getRowForFile, navigateToFolder, triggerActionForFile } from './FilesUtils'
+import { deleteDownloadsFolderBeforeEach } from 'cypress-delete-downloads-folder'
+
+describe('files: Download files using file actions', { testIsolation: true }, () => {
+	let user: User
+
+	deleteDownloadsFolderBeforeEach()
+
+	beforeEach(() => {
+		cy.createRandomUser().then(($user) => {
+			user = $user
+		})
+	})
+
+	it('can download file', () => {
+		cy.uploadContent(user, new Blob(['<content>']), 'text/plain', '/file.txt')
+		cy.login(user)
+		cy.visit('/apps/files')
+
+		getRowForFile('file.txt').should('be.visible')
+
+		triggerActionForFile('file.txt', 'download')
+
+		const downloadsFolder = Cypress.config('downloadsFolder')
+		cy.readFile(`${downloadsFolder}/file.txt`, { timeout: 15000 })
+			.should('exist')
+			.and('have.length.gt', 8)
+			.and('equal', '<content>')
+	})
+
+	/**
+	 * Regression test of https://github.com/nextcloud/server/issues/44855
+	 */
+	it('can download file with hash name', () => {
+		cy.uploadContent(user, new Blob(['<content>']), 'text/plain', '/#file.txt')
+		cy.login(user)
+		cy.visit('/apps/files')
+
+		triggerActionForFile('#file.txt', 'download')
+		const downloadsFolder = Cypress.config('downloadsFolder')
+		cy.readFile(`${downloadsFolder}/#file.txt`, { timeout: 15000 })
+			.should('exist')
+			.and('have.length.gt', 8)
+			.and('equal', '<content>')
+	})
+
+	/**
+	 * Regression test of https://github.com/nextcloud/server/issues/44855
+	 */
+	it('can download file from folder with hash name', () => {
+		cy.mkdir(user, '/#folder')
+			.uploadContent(user, new Blob(['<content>']), 'text/plain', '/#folder/file.txt')
+		cy.login(user)
+		cy.visit('/apps/files')
+
+		navigateToFolder('#folder')
+		// All are visible by default
+		getRowForFile('file.txt').should('be.visible')
+
+		triggerActionForFile('file.txt', 'download')
+		const downloadsFolder = Cypress.config('downloadsFolder')
+		cy.readFile(`${downloadsFolder}/file.txt`, { timeout: 15000 })
+			.should('exist')
+			.and('have.length.gt', 8)
+			.and('equal', '<content>')
+	})
+})
+
+describe('files: Download files using default action', { testIsolation: true }, () => {
+	let user: User
+
+	deleteDownloadsFolderBeforeEach()
+
+	beforeEach(() => {
+		cy.createRandomUser().then(($user) => {
+			user = $user
+		})
+	})
+
+	it('can download file', () => {
+		cy.uploadContent(user, new Blob(['<content>']), 'text/plain', '/file.txt')
+		cy.login(user)
+		cy.visit('/apps/files')
+
+		getRowForFile('file.txt')
+			.should('be.visible')
+			.findByRole('button', { name: 'Download' })
+			.click()
+
+		const downloadsFolder = Cypress.config('downloadsFolder')
+		cy.readFile(`${downloadsFolder}/file.txt`, { timeout: 15000 })
+			.should('exist')
+			.and('have.length.gt', 8)
+			.and('equal', '<content>')
+	})
+
+	/**
+	 * Regression test of https://github.com/nextcloud/server/issues/44855
+	 */
+	it('can download file with hash name', () => {
+		cy.uploadContent(user, new Blob(['<content>']), 'text/plain', '/#file.txt')
+		cy.login(user)
+		cy.visit('/apps/files')
+
+		getRowForFile('#file.txt')
+			.should('be.visible')
+			.findByRole('button', { name: 'Download' })
+			.click()
+
+		const downloadsFolder = Cypress.config('downloadsFolder')
+		cy.readFile(`${downloadsFolder}/#file.txt`, { timeout: 15000 })
+			.should('exist')
+			.and('have.length.gt', 8)
+			.and('equal', '<content>')
+	})
+
+	/**
+	 * Regression test of https://github.com/nextcloud/server/issues/44855
+	 */
+	it('can download file from folder with hash name', () => {
+		cy.mkdir(user, '/#folder')
+			.uploadContent(user, new Blob(['<content>']), 'text/plain', '/#folder/file.txt')
+		cy.login(user)
+		cy.visit('/apps/files')
+
+		navigateToFolder('#folder')
+		// All are visible by default
+		getRowForFile('file.txt')
+			.should('be.visible')
+			.findByRole('button', { name: 'Download' })
+			.click()
+
+		const downloadsFolder = Cypress.config('downloadsFolder')
+		cy.readFile(`${downloadsFolder}/file.txt`, { timeout: 15000 })
+			.should('exist')
+			.and('have.length.gt', 8)
+			.and('equal', '<content>')
+	})
+})

cypress/e2e/files_sharing/file-request.cy.ts

@@ -4,7 +4,7 @@
  */
 
 import type { User } from '@nextcloud/cypress'
-import { createFolder, getRowForFile } from '../files/FilesUtils'
+import { createFolder, getRowForFile, navigateToFolder } from '../files/FilesUtils'
 import { createFileRequest, enterGuestName } from './FilesSharingUtils'
 
 describe('Files', { testIsolation: true }, () => {
@@ -53,7 +53,9 @@ describe('Files', { testIsolation: true }, () => {
 	it('Check the uploaded file', () => {
 		cy.login(user)
 		cy.visit(`/apps/files/files?dir=/${folderName}`)
-		getRowForFile('Guest').should('be.visible').get('a[data-cy-files-list-row-name-link]').click()
+		getRowForFile('Guest')
+			.should('be.visible')
+		navigateToFolder('Guest')
 		getRowForFile('file.txt').should('be.visible')
 	})
 })

cypress/e2e/files_sharing/files-shares-view.cy.ts

@@ -35,7 +35,7 @@ describe('files_sharing: Files view', { testIsolation: true }, () => {
 		// see the shared folder
 		getRowForFile('folder').should('be.visible')
 		// click on the folder should open it in files
-		getRowForFile('folder').findByRole('button', { name: 'folder' }).click()
+		getRowForFile('folder').findByRole('button', { name: /open in files/i }).click()
 		// See the URL has changed
 		cy.url().should('match', /apps\/files\/files\/.+dir=\/folder/)
 		// Content of the shared folder
@@ -50,7 +50,7 @@ describe('files_sharing: Files view', { testIsolation: true }, () => {
 		// see the shared folder
 		getRowForFile('folder').should('be.visible')
 		// click on the folder should open it in files
-		getRowForFile('folder').findByRole('button', { name: 'folder' }).click()
+		getRowForFile('folder').findByRole('button', { name: /open in files/i }).click()
 		// See the URL has changed
 		cy.url().should('match', /apps\/files\/files\/.+dir=\/folder/)
 		// Content of the shared folder

package-lock.json

@@ -119,6 +119,7 @@
         "css-loader": "^6.8.1",
         "cypress": "^13.13.1",
         "cypress-axe": "^1.5.0",
+        "cypress-delete-downloads-folder": "^0.0.6",
         "cypress-if": "^1.12.4",
         "cypress-split": "^1.24.0",
         "cypress-wait-until": "^3.0.2",
@@ -10051,6 +10052,12 @@
         "cypress": "^10 || ^11 || ^12 || ^13"
       }
     },
+    "node_modules/cypress-delete-downloads-folder": {
+      "version": "0.0.6",
+      "resolved": "https://registry.npmjs.org/cypress-delete-downloads-folder/-/cypress-delete-downloads-folder-0.0.6.tgz",
+      "integrity": "sha512-oKMhBM/O74a3XWCrMdFrv3iWMK/UAOdYX8PU/ZUGLaRjmC0p/XWFrMRJZFAYo7owpip3jvmf+bkm82cV0RJhAw==",
+      "dev": true
+    },
     "node_modules/cypress-if": {
       "version": "1.12.4",
       "resolved": "https://registry.npmjs.org/cypress-if/-/cypress-if-1.12.4.tgz",

package.json

@@ -147,6 +147,7 @@
     "css-loader": "^6.8.1",
     "cypress": "^13.13.1",
     "cypress-axe": "^1.5.0",
+    "cypress-delete-downloads-folder": "^0.0.6",
     "cypress-if": "^1.12.4",
     "cypress-split": "^1.24.0",
     "cypress-wait-until": "^3.0.2",