From 4ebea3db3a6e3ec72502b283f1dfbe842d75dbf0 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Fri, 26 Jul 2024 01:42:31 +0200 Subject: [PATCH] fix(files): Provide file actions from list entry to make it reactive This fixes non reactive default action text of the name component. Also use download action as default action so that only one place is needed to define how to download a file. Signed-off-by: Ferdinand Thiessen --- apps/files/src/actions/downloadAction.spec.ts | 4 +- apps/files/src/actions/downloadAction.ts | 4 +- .../components/FileEntry/FileEntryActions.vue | 41 +---- .../components/FileEntry/FileEntryName.vue | 62 ++++---- apps/files/src/components/FileEntryMixin.ts | 42 ++++- .../files/src/components/FilesListVirtual.vue | 18 +-- cypress.config.ts | 9 +- cypress/e2e/files/files-download.cy.ts | 145 ++++++++++++++++++ .../e2e/files_sharing/files-shares-view.cy.ts | 4 +- package.json | 7 +- 10 files changed, 251 insertions(+), 85 deletions(-) create mode 100644 cypress/e2e/files/files-download.cy.ts 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",