diff --git a/apps/files/src/actions/downloadAction.spec.ts b/apps/files/src/actions/downloadAction.spec.ts index bc9c87c0718..56ad3882d21 100644 --- a/apps/files/src/actions/downloadAction.spec.ts +++ b/apps/files/src/actions/downloadAction.spec.ts @@ -21,7 +21,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', @@ -34,7 +34,7 @@ describe('Download action conditions tests', () => { expect(action.id).toBe('download') expect(action.displayName([], view)).toBe('Download') expect(action.iconSvgInline([], view)).toBe('SvgMock') - 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 de2fa081166..03686cd4243 100644 --- a/apps/files/src/actions/downloadAction.ts +++ b/apps/files/src/actions/downloadAction.ts @@ -20,7 +20,7 @@ * */ import { generateUrl } from '@nextcloud/router' -import { FileAction, Permission, Node, FileType, View } from '@nextcloud/files' +import { FileAction, Permission, Node, FileType, View, DefaultType } from '@nextcloud/files' import { translate as t } from '@nextcloud/l10n' import ArrowDownSvg from '@mdi/svg/svg/arrow-down.svg?raw' @@ -60,6 +60,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 c6ee7b9aac7..597fbc5a082 100644 --- a/apps/files/src/components/FileEntry/FileEntryActions.vue +++ b/apps/files/src/components/FileEntry/FileEntryActions.vue @@ -97,9 +97,9 @@ import type { PropType, ShallowRef } from 'vue' import type { FileAction, Node, View } from '@nextcloud/files' import { showError, showSuccess } from '@nextcloud/dialogs' -import { DefaultType, NodeStatus, getFileActions } from '@nextcloud/files' +import { DefaultType, NodeStatus } from '@nextcloud/files' import { translate as t } from '@nextcloud/l10n' -import { defineComponent } from 'vue' +import { defineComponent, inject } from 'vue' import ArrowLeftIcon from 'vue-material-design-icons/ArrowLeft.vue' import NcActionButton from '@nextcloud/vue/dist/Components/NcActionButton.js' @@ -112,9 +112,6 @@ import { useNavigation } from '../../composables/useNavigation' import CustomElementRender from '../CustomElementRender.vue' import logger from '../../logger.js' -// The registered actions list -const actions = getFileActions() - export default defineComponent({ name: 'FileEntryActions', @@ -153,10 +150,12 @@ export default defineComponent({ setup() { const { currentView } = useNavigation() + const enabledFileActions = inject('enabledFileActions', []) return { // The file list is guaranteed to be only shown with active view currentView: currentView as ShallowRef, + enabledFileActions, } }, @@ -175,23 +174,12 @@ export default defineComponent({ return this.source.status === NodeStatus.LOADING }, - // Sorted actions that are enabled for this node - enabledActions() { - if (this.source.attributes.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 @@ -199,12 +187,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 @@ -219,7 +202,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) @@ -233,7 +216,7 @@ export default defineComponent({ }, enabledSubmenuActions() { - return this.enabledActions + return this.enabledFileActions .filter(action => action.parent) .reduce((arr, action) => { if (!arr[action.parent!]) { @@ -322,14 +305,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 5543b05436a..1a66b0a8e39 100644 --- a/apps/files/src/components/FileEntry/FileEntryName.vue +++ b/apps/files/src/components/FileEntry/FileEntryName.vue @@ -54,16 +54,17 @@ + + diff --git a/apps/files/src/components/FileEntryMixin.ts b/apps/files/src/components/FileEntryMixin.ts index 95740efe185..42a29455680 100644 --- a/apps/files/src/components/FileEntryMixin.ts +++ b/apps/files/src/components/FileEntryMixin.ts @@ -20,11 +20,11 @@ * */ -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, View } 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' @@ -36,10 +36,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: { @@ -56,6 +57,13 @@ export default defineComponent({ }, }, + provide() { + return { + defaultFileAction: this.defaultFileAction, + enabledFileActions: this.enabledFileActions, + } + }, + data() { return { loading: '', @@ -173,6 +181,23 @@ export default defineComponent({ isRenaming() { return this.renamingStore.renamingNode === this.source }, + + /** + * 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: { @@ -254,8 +279,15 @@ export default defineComponent({ return false } - const actions = this.$refs.actions as ComponentPublicInstance - 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 c11d33f207a..65c88df2184 100644 --- a/apps/files/src/components/FilesListVirtual.vue +++ b/apps/files/src/components/FilesListVirtual.vue @@ -596,24 +596,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; @@ -623,7 +625,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; @@ -764,12 +766,6 @@ tbody.files-list__tbody.files-list__tbody--grid { padding-top: var(--half-clickable-area); } - a.files-list__row-name-link { - // Minus action menu - width: calc(100% - var(--clickable-area)); - height: var(--clickable-area); - } - .files-list__row-name-text { margin: 0; padding-right: 0; diff --git a/cypress.config.ts b/cypress.config.ts index 718d1d74507..efad0e14f05 100644 --- a/cypress.config.ts +++ b/cypress.config.ts @@ -1,3 +1,8 @@ +/** + * SPDX-FileCopyrightText: 2022 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import type { Configuration } from 'webpack' import { applyChangesToNextcloud, configureNextcloud, @@ -6,9 +11,9 @@ import { waitOnNextcloud, } from './cypress/dockerNode' import { defineConfig } from 'cypress' +import { removeDirectory } from 'cypress-delete-downloads-folder' import cypressSplit from 'cypress-split' import webpackPreprocessor from '@cypress/webpack-preprocessor' -import type { Configuration } from 'webpack' import webpackConfig from './webpack.config.js' @@ -55,6 +60,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(['']), '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', '') + }) + + /** + * Regression test of https://github.com/nextcloud/server/issues/44855 + */ + it('can download file with hash name', () => { + cy.uploadContent(user, new Blob(['']), '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', '') + }) + + /** + * 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(['']), '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', '') + }) +}) + +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(['']), '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', '') + }) + + /** + * Regression test of https://github.com/nextcloud/server/issues/44855 + */ + it('can download file with hash name', () => { + cy.uploadContent(user, new Blob(['']), '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', '') + }) + + /** + * 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(['']), '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', '') + }) +}) 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.json b/package.json index b39ea7fa1ca..9b8c1c25f60 100644 --- a/package.json +++ b/package.json @@ -147,9 +147,10 @@ "css-loader": "^6.8.1", "cypress": "^13.13.2", "cypress-axe": "^1.5.0", - "cypress-if": "^1.10.5", - "cypress-split": "^1.21.0", - "cypress-wait-until": "^2.0.1", + "cypress-delete-downloads-folder": "^0.0.6", + "cypress-if": "^1.12.5", + "cypress-split": "^1.24.0", + "cypress-wait-until": "^3.0.2", "dockerode": "^4.0.2", "eslint-plugin-cypress": "^2.15.2", "eslint-plugin-es": "^4.1.0",