diff options
-rw-r--r-- | apps/files/src/actions/downloadAction.spec.ts | 4 | ||||
-rw-r--r-- | apps/files/src/actions/downloadAction.ts | 6 | ||||
-rw-r--r-- | apps/files/src/components/FileEntry/FileEntryActions.vue | 41 | ||||
-rw-r--r-- | apps/files/src/components/FileEntry/FileEntryName.vue | 54 | ||||
-rw-r--r-- | apps/files/src/components/FileEntryMixin.ts | 42 | ||||
-rw-r--r-- | apps/files/src/components/FilesListVirtual.vue | 16 | ||||
-rw-r--r-- | cypress.config.ts | 5 | ||||
-rw-r--r-- | cypress/e2e/files/files-download.cy.ts | 145 | ||||
-rw-r--r-- | cypress/e2e/files_sharing/file-request.cy.ts | 6 | ||||
-rw-r--r-- | cypress/e2e/files_sharing/files-shares-view.cy.ts | 4 | ||||
-rw-r--r-- | package-lock.json | 7 | ||||
-rw-r--r-- | package.json | 1 |
12 files changed, 252 insertions, 79 deletions
diff --git a/apps/files/src/actions/downloadAction.spec.ts b/apps/files/src/actions/downloadAction.spec.ts index 8aca20eb4d4..59e533441be 100644 --- a/apps/files/src/actions/downloadAction.spec.ts +++ b/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) }) }) diff --git a/apps/files/src/actions/downloadAction.ts b/apps/files/src/actions/downloadAction.ts index 3ba4e23f0cf..cce1e48579a 100644 --- a/apps/files/src/actions/downloadAction.ts +++ b/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, diff --git a/apps/files/src/components/FileEntry/FileEntryActions.vue b/apps/files/src/components/FileEntry/FileEntryActions.vue index 3df4289b1a0..f886d4be3ec 100644 --- a/apps/files/src/components/FileEntry/FileEntryActions.vue +++ b/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 diff --git a/apps/files/src/components/FileEntry/FileEntryName.vue b/apps/files/src/components/FileEntry/FileEntryName.vue index 7a6ad2a1051..329ec7fdf19 100644 --- a/apps/files/src/components/FileEntry/FileEntryName.vue +++ b/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' @@ -98,8 +99,11 @@ export default defineComponent({ const { currentView } = useNavigation() const renamingStore = useRenamingStore() + const defaultFileAction = inject<FileAction | undefined>('defaultFileAction') + return { currentView, + defaultFileAction, renamingStore, } @@ -139,32 +143,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 +272,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 ? @@ -309,3 +304,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> diff --git a/apps/files/src/components/FileEntryMixin.ts b/apps/files/src/components/FileEntryMixin.ts index d9117053dd8..e18841e159e 100644 --- a/apps/files/src/components/FileEntryMixin.ts +++ b/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) { diff --git a/apps/files/src/components/FilesListVirtual.vue b/apps/files/src/components/FilesListVirtual.vue index 1b9475333ab..37638aeabe6 100644 --- a/apps/files/src/components/FilesListVirtual.vue +++ b/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. diff --git a/cypress.config.ts b/cypress.config.ts index 8c35f354735..e9b09f2012d 100644 --- a/cypress.config.ts +++ b/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') { diff --git a/cypress/e2e/files/files-download.cy.ts b/cypress/e2e/files/files-download.cy.ts new file mode 100644 index 00000000000..5522fb947d6 --- /dev/null +++ b/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>') + }) +}) diff --git a/cypress/e2e/files_sharing/file-request.cy.ts b/cypress/e2e/files_sharing/file-request.cy.ts index 793383e9a77..7c33594e25c 100644 --- a/cypress/e2e/files_sharing/file-request.cy.ts +++ b/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') }) }) diff --git a/cypress/e2e/files_sharing/files-shares-view.cy.ts b/cypress/e2e/files_sharing/files-shares-view.cy.ts index 01083e6dda9..12a67d9ee0f 100644 --- a/cypress/e2e/files_sharing/files-shares-view.cy.ts +++ b/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 diff --git a/package-lock.json b/package-lock.json index 1969bfe7543..af849079c2c 100644 --- a/package-lock.json +++ b/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", diff --git a/package.json b/package.json index 115487c2b94..1a945f2d785 100644 --- a/package.json +++ b/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", |