diff options
author | Ferdinand Thiessen <opensource@fthiessen.de> | 2025-02-22 13:25:18 +0100 |
---|---|---|
committer | Ferdinand Thiessen <opensource@fthiessen.de> | 2025-02-25 02:16:56 +0100 |
commit | e712104e58db906d8d0859d2f920c74b78825089 (patch) | |
tree | 1d6c5ce3cb023081a0e80258b4f29a466d2a75cd /apps/files/src | |
parent | b0653e956c4a2751b1ff7f1660c1369c7d73a747 (diff) | |
download | nextcloud-server-e712104e58db906d8d0859d2f920c74b78825089.tar.gz nextcloud-server-e712104e58db906d8d0859d2f920c74b78825089.zip |
feat(files): allow to ignore warning to change file type
* Missing pieces of https://github.com/nextcloud/server/issues/46528
* Add checkbox to not show this dialog again
* Add user config as suggested by designers in files settings to
reenable or diable this behavior.
* Fix behavior of dialog: It says "keep .ext" but it does not keep the
extension but cancels the operation. From the button label the user
expects that the operation is continued but with the old extension.
* Added more test coverage by adding component tests.
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Diffstat (limited to 'apps/files/src')
-rw-r--r-- | apps/files/src/components/FileEntry/FileEntryName.vue | 13 | ||||
-rw-r--r-- | apps/files/src/eventbus.d.ts | 1 | ||||
-rw-r--r-- | apps/files/src/store/renaming.ts | 307 | ||||
-rw-r--r-- | apps/files/src/store/userconfig.ts | 2 | ||||
-rw-r--r-- | apps/files/src/types.ts | 1 | ||||
-rw-r--r-- | apps/files/src/views/DialogConfirmFileExtension.cy.ts | 161 | ||||
-rw-r--r-- | apps/files/src/views/DialogConfirmFileExtension.vue | 92 | ||||
-rw-r--r-- | apps/files/src/views/Settings.vue | 9 |
8 files changed, 417 insertions, 169 deletions
diff --git a/apps/files/src/components/FileEntry/FileEntryName.vue b/apps/files/src/components/FileEntry/FileEntryName.vue index bc6b61ad54c..2fec9e5d556 100644 --- a/apps/files/src/components/FileEntry/FileEntryName.vue +++ b/apps/files/src/components/FileEntry/FileEntryName.vue @@ -23,7 +23,6 @@ <component :is="linkTo.is" v-else ref="basename" - :aria-hidden="isRenaming" class="files-list__row-name-link" data-cy-files-list-row-name-link v-bind="linkTo.params"> @@ -117,11 +116,11 @@ export default defineComponent({ return this.isRenaming && this.filesListWidth < 512 }, newName: { - get() { - return this.renamingStore.newName + get(): string { + return this.renamingStore.newNodeName }, - set(newName) { - this.renamingStore.newName = newName + set(newName: string) { + this.renamingStore.newNodeName = newName }, }, @@ -249,7 +248,9 @@ export default defineComponent({ try { const status = await this.renamingStore.rename() if (status) { - showSuccess(t('files', 'Renamed "{oldName}" to "{newName}"', { oldName, newName })) + showSuccess( + t('files', 'Renamed "{oldName}" to "{newName}"', { oldName, newName: this.source.basename }), + ) this.$nextTick(() => { const nameContainer = this.$refs.basename as HTMLElement | undefined nameContainer?.focus() diff --git a/apps/files/src/eventbus.d.ts b/apps/files/src/eventbus.d.ts index ce0611580fe..49ac67fe4db 100644 --- a/apps/files/src/eventbus.d.ts +++ b/apps/files/src/eventbus.d.ts @@ -16,6 +16,7 @@ declare module '@nextcloud/event-bus' { 'files:node:created': Node 'files:node:deleted': Node 'files:node:updated': Node + 'files:node:rename': Node 'files:node:renamed': Node 'files:node:moved': { node: Node, oldSource: string } diff --git a/apps/files/src/store/renaming.ts b/apps/files/src/store/renaming.ts index 8ed455a182a..2ac9e06ba16 100644 --- a/apps/files/src/store/renaming.ts +++ b/apps/files/src/store/renaming.ts @@ -3,184 +3,165 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ import type { Node } from '@nextcloud/files' -import type { RenamingStore } from '../types' import axios, { isAxiosError } from '@nextcloud/axios' import { emit, subscribe } from '@nextcloud/event-bus' import { FileType, NodeStatus } from '@nextcloud/files' -import { DialogBuilder } from '@nextcloud/dialogs' import { t } from '@nextcloud/l10n' +import { spawnDialog } from '@nextcloud/vue/functions/dialog' import { basename, dirname, extname } from 'path' import { defineStore } from 'pinia' import logger from '../logger' -import Vue from 'vue' -import IconCancel from '@mdi/svg/svg/cancel.svg?raw' -import IconCheck from '@mdi/svg/svg/check.svg?raw' - -let isDialogVisible = false - -const showWarningDialog = (oldExtension: string, newExtension: string): Promise<boolean> => { - if (isDialogVisible) { - return Promise.resolve(false) - } - - isDialogVisible = true - - let message - - if (!oldExtension && newExtension) { - message = t( - 'files', - 'Adding the file extension "{new}" may render the file unreadable.', - { new: newExtension }, - ) - } else if (!newExtension) { - message = t( - 'files', - 'Removing the file extension "{old}" may render the file unreadable.', - { old: oldExtension }, - ) - } else { - message = t( - 'files', - 'Changing the file extension from "{old}" to "{new}" may render the file unreadable.', - { old: oldExtension, new: newExtension }, - ) - } - - return new Promise((resolve) => { - const dialog = new DialogBuilder() - .setName(t('files', 'Change file extension')) - .setText(message) - .setButtons([ - { - label: t('files', 'Keep {oldextension}', { oldextension: oldExtension }), - icon: IconCancel, - type: 'secondary', - callback: () => { - isDialogVisible = false - resolve(false) - }, +import Vue, { defineAsyncComponent, ref } from 'vue' +import { useUserConfigStore } from './userconfig' + +export const useRenamingStore = defineStore('renaming', () => { + /** + * The currently renamed node + */ + const renamingNode = ref<Node>() + /** + * The new name of the currently renamed node + */ + const newNodeName = ref('') + + /** + * Internal flag to only allow calling `rename` once. + */ + const isRenaming = ref(false) + + /** + * Execute the renaming. + * This will rename the node set as `renamingNode` to the configured new name `newName`. + * + * @return true if success, false if skipped (e.g. new and old name are the same) + * @throws Error if renaming fails, details are set in the error message + */ + async function rename(): Promise<boolean> { + if (renamingNode.value === undefined) { + throw new Error('No node is currently being renamed') + } + + // Only rename once so we use this as some kind of mutex + if (isRenaming.value) { + return false + } + isRenaming.value = true + + const node = renamingNode.value + Vue.set(node, 'status', NodeStatus.LOADING) + + const userConfig = useUserConfigStore() + + let newName = newNodeName.value.trim() + const oldName = node.basename + const oldExtension = extname(oldName) + const newExtension = extname(newName) + // Check for extension change for files + if (node.type === FileType.File + && oldExtension !== newExtension + && userConfig.userConfig.show_dialog_file_extension + && !(await showFileExtensionDialog(oldExtension, newExtension)) + ) { + // user selected to use the old extension + newName = basename(newName, newExtension) + oldExtension + } + + const oldEncodedSource = node.encodedSource + try { + if (oldName === newName) { + return false + } + + // rename the node + node.rename(newName) + logger.debug('Moving file to', { destination: node.encodedSource, oldEncodedSource }) + // create MOVE request + await axios({ + method: 'MOVE', + url: oldEncodedSource, + headers: { + Destination: node.encodedSource, + Overwrite: 'F', }, - { - label: newExtension.length ? t('files', 'Use {newextension}', { newextension: newExtension }) : t('files', 'Remove extension'), - icon: IconCheck, - type: 'primary', - callback: () => { - isDialogVisible = false - resolve(true) - }, - }, - ]) - .build() - - dialog.show().then(() => { - dialog.hide() - }) - }) -} - -export const useRenamingStore = function(...args) { - const store = defineStore('renaming', { - state: () => ({ - renamingNode: undefined, - newName: '', - } as RenamingStore), - - actions: { - /** - * Execute the renaming. - * This will rename the node set as `renamingNode` to the configured new name `newName`. - * @return true if success, false if skipped (e.g. new and old name are the same) - * @throws Error if renaming fails, details are set in the error message - */ - async rename(): Promise<boolean> { - if (this.renamingNode === undefined) { - throw new Error('No node is currently being renamed') - } - - const newName = this.newName.trim?.() || '' - const oldName = this.renamingNode.basename - const oldEncodedSource = this.renamingNode.encodedSource - - // Check for extension change for files - const oldExtension = extname(oldName) - const newExtension = extname(newName) - if (oldExtension !== newExtension && this.renamingNode.type === FileType.File) { - const proceed = await showWarningDialog(oldExtension, newExtension) - if (!proceed) { - return false - } + }) + + // Success 🎉 + emit('files:node:updated', node) + emit('files:node:renamed', node) + emit('files:node:moved', { + node, + oldSource: `${dirname(node.source)}/${oldName}`, + }) + + // Reset the state not changed + if (renamingNode.value === node) { + $reset() + } + + return true + } catch (error) { + logger.error('Error while renaming file', { error }) + // Rename back as it failed + node.rename(oldName) + if (isAxiosError(error)) { + // TODO: 409 means current folder does not exist, redirect ? + if (error?.response?.status === 404) { + throw new Error(t('files', 'Could not rename "{oldName}", it does not exist any more', { oldName })) + } else if (error?.response?.status === 412) { + throw new Error(t( + 'files', + 'The name "{newName}" is already used in the folder "{dir}". Please choose a different name.', + { + newName, + dir: basename(renamingNode.value!.dirname), + }, + )) } + } + // Unknown error + throw new Error(t('files', 'Could not rename "{oldName}"', { oldName })) + } finally { + Vue.set(node, 'status', undefined) + isRenaming.value = false + } + } - if (oldName === newName) { - return false - } + /** + * Reset the store state + */ + function $reset(): void { + newNodeName.value = '' + renamingNode.value = undefined + } - const node = this.renamingNode - Vue.set(node, 'status', NodeStatus.LOADING) - - try { - // rename the node - this.renamingNode.rename(newName) - logger.debug('Moving file to', { destination: this.renamingNode.encodedSource, oldEncodedSource }) - // create MOVE request - await axios({ - method: 'MOVE', - url: oldEncodedSource, - headers: { - Destination: this.renamingNode.encodedSource, - Overwrite: 'F', - }, - }) - - // Success 🎉 - emit('files:node:updated', this.renamingNode as Node) - emit('files:node:renamed', this.renamingNode as Node) - emit('files:node:moved', { - node: this.renamingNode as Node, - oldSource: `${dirname(this.renamingNode.source)}/${oldName}`, - }) - this.$reset() - return true - } catch (error) { - logger.error('Error while renaming file', { error }) - // Rename back as it failed - this.renamingNode.rename(oldName) - if (isAxiosError(error)) { - // TODO: 409 means current folder does not exist, redirect ? - if (error?.response?.status === 404) { - throw new Error(t('files', 'Could not rename "{oldName}", it does not exist any more', { oldName })) - } else if (error?.response?.status === 412) { - throw new Error(t( - 'files', - 'The name "{newName}" is already used in the folder "{dir}". Please choose a different name.', - { - newName, - dir: basename(this.renamingNode.dirname), - }, - )) - } - } - // Unknown error - throw new Error(t('files', 'Could not rename "{oldName}"', { oldName })) - } finally { - Vue.set(node, 'status', undefined) - } - }, - }, + // Make sure we only register the listeners once + subscribe('files:node:rename', (node: Node) => { + renamingNode.value = node + newNodeName.value = node.basename }) - const renamingStore = store(...args) + return { + $reset, - // Make sure we only register the listeners once - if (!renamingStore._initialized) { - subscribe('files:node:rename', function(node: Node) { - renamingStore.renamingNode = node - renamingStore.newName = node.basename - }) - renamingStore._initialized = true + newNodeName, + rename, + renamingNode, } +}) - return renamingStore +/** + * Show a dialog asking user for confirmation about changing the file extension. + * + * @param oldExtension the old file name extension + * @param newExtension the new file name extension + */ +async function showFileExtensionDialog(oldExtension: string, newExtension: string): Promise<boolean> { + const { promise, resolve } = Promise.withResolvers<boolean>() + spawnDialog( + defineAsyncComponent(() => import('../views/DialogConfirmFileExtension.vue')), + { oldExtension, newExtension }, + (useNewExtension: unknown) => resolve(Boolean(useNewExtension)), + ) + return await promise } diff --git a/apps/files/src/store/userconfig.ts b/apps/files/src/store/userconfig.ts index 95829c849e8..d84a5e0d935 100644 --- a/apps/files/src/store/userconfig.ts +++ b/apps/files/src/store/userconfig.ts @@ -17,6 +17,8 @@ const initialUserConfig = loadState<UserConfig>('files', 'config', { sort_favorites_first: true, sort_folders_first: true, grid_view: false, + + show_dialog_file_extension: true, }) export const useUserConfigStore = defineStore('userconfig', () => { diff --git a/apps/files/src/types.ts b/apps/files/src/types.ts index 62ba53b89d2..4bf8a557f49 100644 --- a/apps/files/src/types.ts +++ b/apps/files/src/types.ts @@ -52,6 +52,7 @@ export interface PathOptions { export interface UserConfig { [key: string]: boolean|undefined + show_dialog_file_extension: boolean, show_hidden: boolean crop_image_previews: boolean sort_favorites_first: boolean diff --git a/apps/files/src/views/DialogConfirmFileExtension.cy.ts b/apps/files/src/views/DialogConfirmFileExtension.cy.ts new file mode 100644 index 00000000000..460497dd91f --- /dev/null +++ b/apps/files/src/views/DialogConfirmFileExtension.cy.ts @@ -0,0 +1,161 @@ +/*! + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import { createTestingPinia } from '@pinia/testing' +import DialogConfirmFileExtension from './DialogConfirmFileExtension.vue' +import { useUserConfigStore } from '../store/userconfig' + +describe('DialogConfirmFileExtension', () => { + it('renders with both extensions', () => { + cy.mount(DialogConfirmFileExtension, { + propsData: { + oldExtension: '.old', + newExtension: '.new', + }, + global: { + plugins: [createTestingPinia({ + createSpy: cy.spy, + })], + }, + }) + + cy.findByRole('dialog') + .as('dialog') + .should('be.visible') + cy.get('@dialog') + .findByRole('heading') + .should('contain.text', 'Change file extension') + cy.get('@dialog') + .findByRole('checkbox', { name: /Do not show this dialog again/i }) + .should('exist') + .and('not.be.checked') + cy.get('@dialog') + .findByRole('button', { name: 'Keep .old' }) + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Use .new' }) + .should('be.visible') + }) + + it('renders without old extension', () => { + cy.mount(DialogConfirmFileExtension, { + propsData: { + newExtension: '.new', + }, + global: { + plugins: [createTestingPinia({ + createSpy: cy.spy, + })], + }, + }) + + cy.findByRole('dialog') + .as('dialog') + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Keep without extension' }) + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Use .new' }) + .should('be.visible') + }) + + it('renders without new extension', () => { + cy.mount(DialogConfirmFileExtension, { + propsData: { + oldExtension: '.old', + }, + global: { + plugins: [createTestingPinia({ + createSpy: cy.spy, + })], + }, + }) + + cy.findByRole('dialog') + .as('dialog') + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Keep .old' }) + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Remove extension' }) + .should('be.visible') + }) + + it('emits correct value on keep old', () => { + cy.mount(DialogConfirmFileExtension, { + propsData: { + oldExtension: '.old', + newExtension: '.new', + }, + global: { + plugins: [createTestingPinia({ + createSpy: cy.spy, + })], + }, + }).as('component') + + cy.findByRole('dialog') + .as('dialog') + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Keep .old' }) + .click() + cy.get('@component') + .its('wrapper') + .should((wrapper) => expect(wrapper.emitted('close')).to.eql([[false]])) + }) + + it('emits correct value on use new', () => { + cy.mount(DialogConfirmFileExtension, { + propsData: { + oldExtension: '.old', + newExtension: '.new', + }, + global: { + plugins: [createTestingPinia({ + createSpy: cy.spy, + })], + }, + }).as('component') + + cy.findByRole('dialog') + .as('dialog') + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Use .new' }) + .click() + cy.get('@component') + .its('wrapper') + .should((wrapper) => expect(wrapper.emitted('close')).to.eql([[true]])) + }) + + it('updates user config when checking the checkbox', () => { + const pinia = createTestingPinia({ + createSpy: cy.spy, + }) + + cy.mount(DialogConfirmFileExtension, { + propsData: { + oldExtension: '.old', + newExtension: '.new', + }, + global: { + plugins: [pinia], + }, + }).as('component') + + cy.findByRole('dialog') + .as('dialog') + .should('be.visible') + cy.get('@dialog') + .findByRole('checkbox', { name: /Do not show this dialog again/i }) + .check({ force: true }) + + cy.wrap(useUserConfigStore()) + .its('update') + .should('have.been.calledWith', 'show_dialog_file_extension', false) + }) +}) diff --git a/apps/files/src/views/DialogConfirmFileExtension.vue b/apps/files/src/views/DialogConfirmFileExtension.vue new file mode 100644 index 00000000000..cc1ee363f98 --- /dev/null +++ b/apps/files/src/views/DialogConfirmFileExtension.vue @@ -0,0 +1,92 @@ +<!-- + - SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + - SPDX-License-Identifier: AGPL-3.0-or-later +--> + +<script setup lang="ts"> +import type { IDialogButton } from '@nextcloud/dialogs' +import { t } from '@nextcloud/l10n' +import { computed, ref } from 'vue' +import { useUserConfigStore } from '../store/userconfig.ts' + +import NcCheckboxRadioSwitch from '@nextcloud/vue/components/NcCheckboxRadioSwitch' +import NcDialog from '@nextcloud/vue/components/NcDialog' +import svgIconCancel from '@mdi/svg/svg/cancel.svg?raw' +import svgIconCheck from '@mdi/svg/svg/check.svg?raw' + +const props = defineProps<{ + oldExtension?: string + newExtension?: string +}>() + +const emit = defineEmits<{ + (e: 'close', v: boolean): void +}>() + +const userConfigStore = useUserConfigStore() +const dontShowAgain = computed({ + get: () => !userConfigStore.userConfig.show_dialog_file_extension, + set: (value: boolean) => userConfigStore.update('show_dialog_file_extension', !value), +}) + +const buttons = computed<IDialogButton[]>(() => [ + { + label: props.oldExtension + ? t('files', 'Keep {old}', { old: props.oldExtension }) + : t('files', 'Keep without extension'), + icon: svgIconCancel, + type: 'secondary', + callback: () => closeDialog(false), + }, + { + label: props.newExtension + ? t('files', 'Use {new}', { new: props.newExtension }) + : t('files', 'Remove extension'), + icon: svgIconCheck, + type: 'primary', + callback: () => closeDialog(true), + }, +]) + +/** Open state of the dialog */ +const open = ref(true) + +/** + * Close the dialog and emit the response + * @param value User selected response + */ +function closeDialog(value: boolean) { + emit('close', value) + open.value = false +} +</script> + +<template> + <NcDialog :buttons="buttons" + :open="open" + :can-close="false" + :name="t('files', 'Change file extension')" + size="small"> + <p v-if="newExtension && oldExtension"> + {{ t('files', 'Changing the file extension from "{old}" to "{new}" may render the file unreadable.', { old: oldExtension, new: newExtension }) }} + </p> + <p v-else-if="oldExtension"> + {{ t('files', 'Removing the file extension "{old}" may render the file unreadable.', { old: oldExtension }) }} + </p> + <p v-else-if="newExtension"> + {{ t('files', 'Adding the file extension "{new}" may render the file unreadable.', { new: newExtension }) }} + </p> + + <NcCheckboxRadioSwitch v-model="dontShowAgain" + class="dialog-confirm-file-extension__checkbox" + type="checkbox"> + {{ t('files', 'Do not show this dialog again.') }} + </NcCheckboxRadioSwitch> + </NcDialog> +</template> + +<style scoped> +.dialog-confirm-file-extension__checkbox { + margin-top: 1rem; +} +</style> diff --git a/apps/files/src/views/Settings.vue b/apps/files/src/views/Settings.vue index 1def0e41c15..872b8a8e6d3 100644 --- a/apps/files/src/views/Settings.vue +++ b/apps/files/src/views/Settings.vue @@ -83,6 +83,15 @@ </em> </NcAppSettingsSection> + <NcAppSettingsSection id="warning" :name="t('files', 'Warnings')"> + <em>{{ t('files', 'Prevent warning dialogs from open or reenable them.') }}</em> + <NcCheckboxRadioSwitch type="switch" + :checked="userConfig.show_dialog_file_extension" + @update:checked="setConfig('show_dialog_file_extension', $event)"> + {{ t('files', 'Show a warning dialog when changing a file extension.') }} + </NcCheckboxRadioSwitch> + </NcAppSettingsSection> + <NcAppSettingsSection id="shortcuts" :name="t('files', 'Keyboard shortcuts')"> <em>{{ t('files', 'Speed up your Files experience with these quick shortcuts.') }}</em> |