From 5d78a4ee4ee700b5590a515c90535486967bf621 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Fri, 21 Aug 2026 15:15:37 +0200 Subject: [PATCH 1/4] Extract folder emptiness check into FileFoldersCollection Whether a folder can be deleted is a question about the folder tree, which the collection already answers for parents, children and descendants. Having it here keeps the next caller from spelling the rule out a second time. --- .../collections/FileFoldersCollection-spec.js | 28 +++++++++++++++++++ .../collections/FileFoldersCollection.js | 11 ++++++++ package/src/editor/views/FolderItemView.js | 12 ++------ 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/package/spec/editor/collections/FileFoldersCollection-spec.js b/package/spec/editor/collections/FileFoldersCollection-spec.js index 0ab7bfec87..1e26afb30a 100644 --- a/package/spec/editor/collections/FileFoldersCollection-spec.js +++ b/package/spec/editor/collections/FileFoldersCollection-spec.js @@ -1,3 +1,5 @@ +import Backbone from 'backbone'; + import {FileFoldersCollection} from 'pageflow/editor'; import * as support from '$support'; @@ -159,6 +161,32 @@ describe('FileFoldersCollection', () => { }); }); + describe('#isEmptyFolder', () => { + it('returns true for folder without files or subfolders', () => { + const fileFolders = collection([{perma_id: 1, name: 'Interviews'}]); + const files = new Backbone.Collection([{folder_perma_id: 2}]); + + expect(fileFolders.isEmptyFolder(fileFolders.byPermaId(1), files)).toBe(true); + }); + + it('returns false for folder which holds a file', () => { + const fileFolders = collection([{perma_id: 1, name: 'Interviews'}]); + const files = new Backbone.Collection([{folder_perma_id: 1}]); + + expect(fileFolders.isEmptyFolder(fileFolders.byPermaId(1), files)).toBe(false); + }); + + it('returns false for folder which holds an empty subfolder', () => { + const fileFolders = collection([ + {perma_id: 1, name: 'Interviews'}, + {perma_id: 2, name: 'Raw', parent_folder_perma_id: 1} + ]); + const files = new Backbone.Collection([]); + + expect(fileFolders.isEmptyFolder(fileFolders.byPermaId(1), files)).toBe(false); + }); + }); + describe('persistence', () => { support.useFakeXhr(() => testContext); diff --git a/package/src/editor/collections/FileFoldersCollection.js b/package/src/editor/collections/FileFoldersCollection.js index 56337d0862..82f1af1510 100644 --- a/package/src/editor/collections/FileFoldersCollection.js +++ b/package/src/editor/collections/FileFoldersCollection.js @@ -64,6 +64,17 @@ export const FileFoldersCollection = Backbone.Collection.extend({ }); }, + // The server refuses to delete a folder which still holds files or + // subfolders, empty subfolders included. + isEmptyFolder: function(folder, files) { + var permaId = folder.get('perma_id'); + + return !this.childrenOf(folder).length && + !files.some(function(file) { + return file.get('folder_perma_id') === permaId; + }); + }, + descendantPermaIdsOf: function(folder) { return this.collectDescendantPermaIds(folder, []); }, diff --git a/package/src/editor/views/FolderItemView.js b/package/src/editor/views/FolderItemView.js index 57612c2f8f..7c1e56913e 100644 --- a/package/src/editor/views/FolderItemView.js +++ b/package/src/editor/views/FolderItemView.js @@ -132,16 +132,10 @@ export const FolderItemView = Marionette.ItemView.extend({ this.menuItems.findWhere({name: 'move'}).set('hidden', !movable); }, - // The server refuses to delete a folder which still holds files or - // subfolders. Files hidden by the file type filter cannot make one - // look empty, since such a folder is not listed to begin with. + // Files hidden by the file type filter cannot make a folder look + // empty, since such a folder is not listed to begin with. updateDestroyItem: function() { - var permaId = this.model.get('perma_id'); - - var empty = !this.options.fileFolders.childrenOf(this.model).length && - !this.options.files.some(function(file) { - return file.get('folder_perma_id') === permaId; - }); + var empty = this.options.fileFolders.isEmptyFolder(this.model, this.options.files); this.menuItems.findWhere({name: 'destroy'}).set('hidden', !empty); }, From 3ab16ae93c08c4095ddf4ffacdbd0b1f47cb8612 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Fri, 21 Aug 2026 15:15:44 +0200 Subject: [PATCH 2/4] Delete checked files and folders from the selection bar The selection bar gained a trash button next to the move button, which asks for confirmation and then destroys everything that is checked. Folders which still hold files or subfolders are refused by the server, so a selection containing one disables the button instead of failing halfway through the deletions. --- .../pageflow/editor/filtered_files.scss | 21 +++++- config/locales/de.yml | 4 + config/locales/en.yml | 4 + .../editor/collections/ListSelection-spec.js | 27 +++++++ package/spec/editor/views/FilesView-spec.js | 65 ++++++++++++++++ .../editor/views/FilteredFilesView-spec.js | 75 +++++++++++++++++++ .../src/editor/collections/ListSelection.js | 8 ++ .../src/editor/templates/filteredFiles.jst | 5 +- package/src/editor/views/FilteredFilesView.js | 38 +++++++++- 9 files changed, 240 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/pageflow/editor/filtered_files.scss b/app/assets/stylesheets/pageflow/editor/filtered_files.scss index 90726d2507..e1325b940e 100644 --- a/app/assets/stylesheets/pageflow/editor/filtered_files.scss +++ b/app/assets/stylesheets/pageflow/editor/filtered_files.scss @@ -112,12 +112,17 @@ flex: 1; } - // On a line of its own, so that the number of checked files is not - // squeezed next to it in languages with longer words than English. + // On a line of their own, so that the number of checked files is not + // squeezed next to them in languages with longer words than English. + &-selection_bar_actions { + display: flex; + align-self: flex-end; + gap: space(1); + } + &-selection_bar_action { @include simple-button; - align-self: flex-end; padding: space(1) space(4); &[disabled] { @@ -126,6 +131,16 @@ } } + // The icon only mixin brings its own vertical padding, which would + // make this button taller than the one next to it. + &-selection_bar_destroy { + @include icon-only-button("destructive"); + @include trash-icon; + + padding-top: space(1); + padding-bottom: space(1); + } + &-selection_bar_dismiss { @include cancel-icon; diff --git a/config/locales/de.yml b/config/locales/de.yml index c8c77702ff..d89e462cfb 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -1590,6 +1590,10 @@ de: actions: 'Aktionen für die Dateiliste' any_file_type: 'Datei' cancel_selection: 'Auswahl abbrechen' + confirm_destroy_selection: + one: '1 Element wirklich löschen?' + other: '%{count} Elemente wirklich löschen?' + destroy_selection: 'Auswahl löschen' end_selection: 'Auswahl beenden' move_selection: 'Verschieben...' reset_filter: 'Filter zurücksetzen' diff --git a/config/locales/en.yml b/config/locales/en.yml index 14672e825a..d0e6b6f982 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1582,6 +1582,10 @@ en: actions: 'File list actions' any_file_type: 'a file' cancel_selection: 'Cancel selection' + confirm_destroy_selection: + one: 'Really delete 1 item?' + other: 'Really delete %{count} items?' + destroy_selection: 'Delete selection' end_selection: 'End selection' move_selection: 'Move...' reset_filter: 'Reset filter' diff --git a/package/spec/editor/collections/ListSelection-spec.js b/package/spec/editor/collections/ListSelection-spec.js index 9a97cbecf1..0d3bd5f450 100644 --- a/package/spec/editor/collections/ListSelection-spec.js +++ b/package/spec/editor/collections/ListSelection-spec.js @@ -46,4 +46,31 @@ describe('ListSelection', () => { expect(selection.models).toEqual([file]); }); + + describe('#destroyAll', () => { + it('destroys every selected item', () => { + const file = f.file({id: 1}); + const otherFile = f.file({id: 2}); + jest.spyOn(file, 'destroy').mockImplementation(() => {}); + jest.spyOn(otherFile, 'destroy').mockImplementation(() => {}); + const selection = new ListSelection([file, otherFile]); + + selection.destroyAll(); + + expect(file.destroy).toHaveBeenCalled(); + expect(otherFile.destroy).toHaveBeenCalled(); + }); + + it('destroys items which leave the selection along the way', () => { + const file = f.file({id: 1}); + const otherFile = f.file({id: 2}); + const selection = new ListSelection([file, otherFile]); + jest.spyOn(file, 'destroy').mockImplementation(() => selection.remove(file)); + jest.spyOn(otherFile, 'destroy').mockImplementation(() => selection.remove(otherFile)); + + selection.destroyAll(); + + expect(otherFile.destroy).toHaveBeenCalled(); + }); + }); }); diff --git a/package/spec/editor/views/FilesView-spec.js b/package/spec/editor/views/FilesView-spec.js index a3a3a4a9fb..4e0bcbf2bc 100644 --- a/package/spec/editor/views/FilesView-spec.js +++ b/package/spec/editor/views/FilesView-spec.js @@ -77,6 +77,11 @@ describe('FilesView', () => { 'pageflow.editor.views.filtered_files_view.actions': 'File list actions', 'pageflow.editor.views.filtered_files_view.select_items': 'Select files and folders', 'pageflow.editor.views.filtered_files_view.end_selection': 'End selection', + 'pageflow.editor.views.filtered_files_view.destroy_selection': 'Delete selection', + 'pageflow.editor.views.filtered_files_view.confirm_destroy_selection': { + one: 'Really delete 1 item?', + other: 'Really delete %{count} items?' + }, 'pageflow.editor.views.filtered_files_view.selected_items': { zero: 'No items selected', one: '1 item selected', @@ -1322,6 +1327,10 @@ describe('FilesView', () => { return view.el.querySelector('.filtered_files-selection_bar'); } + function destroyButton(queries) { + return queries.getByRole('button', {name: 'Delete selection'}); + } + it('does not show check boxes before selecting has started', () => { const view = new FilesView({model: entryWithFolders()}); @@ -1447,6 +1456,62 @@ describe('FilesView', () => { expect(queryByRole('checkbox', {name: /^Interviews/})).toBeNull(); }); + + it('deletes checked folders and files at once', async () => { + const entry = entryWithFolders(); + const view = new FilesView({model: entry}); + const user = userEvent.setup(); + window.confirm = jest.fn(() => true); + + const queries = render(view); + await startSelecting(user, queries); + await check(user, queries, /^Landscapes/); + await check(user, queries, 'unfiled.png'); + await user.click(destroyButton(queries)); + + expect(window.confirm).toHaveBeenCalledWith('Really delete 2 items?'); + expect(testContext.requests.map(request => request.url)) + .toEqual(['/editor/entries/1/file_folders/12', + '/editor/entries/1/files/image_files/1']); + }); + + it('is not offered for a folder which still holds files', async () => { + const view = new FilesView({model: entryWithFolders(), folderPermaId: '1'}); + const user = userEvent.setup(); + + const queries = render(view); + await startSelecting(user, queries); + await check(user, queries, /^Raw/); + + expect(destroyButton(queries)).toBeDisabled(); + }); + + it('is not offered for a folder which still holds subfolders', async () => { + const view = new FilesView({model: entryWithFolders()}); + const user = userEvent.setup(); + + const queries = render(view); + await startSelecting(user, queries); + await check(user, queries, /^Interviews/); + + expect(destroyButton(queries)).toBeDisabled(); + }); + + it('is offered again once the non-empty folder is unchecked', async () => { + const view = new FilesView({model: entryWithFolders()}); + const user = userEvent.setup(); + + const queries = render(view); + await startSelecting(user, queries); + await check(user, queries, /^Landscapes/); + await check(user, queries, /^Interviews/); + + expect(destroyButton(queries)).toBeDisabled(); + + await check(user, queries, /^Interviews/); + + expect(destroyButton(queries)).toBeEnabled(); + }); }); describe('filtering by file type', () => { diff --git a/package/spec/editor/views/FilteredFilesView-spec.js b/package/spec/editor/views/FilteredFilesView-spec.js index 0c51f5ee84..d5f0bc828f 100644 --- a/package/spec/editor/views/FilteredFilesView-spec.js +++ b/package/spec/editor/views/FilteredFilesView-spec.js @@ -24,6 +24,11 @@ describe('FilteredFilesView', () => { 'pageflow.editor.views.filtered_files_view.actions': 'File list actions', 'pageflow.editor.views.filtered_files_view.select_items': 'Select files and folders', 'pageflow.editor.views.filtered_files_view.end_selection': 'End selection', + 'pageflow.editor.views.filtered_files_view.destroy_selection': 'Delete selection', + 'pageflow.editor.views.filtered_files_view.confirm_destroy_selection': { + one: 'Really delete 1 item?', + other: 'Really delete %{count} items?' + }, 'pageflow.editor.views.filtered_files_view.selected_items': { zero: 'No items selected', one: '1 item selected', @@ -737,6 +742,76 @@ describe('FilteredFilesView', () => { expect(queries.getByRole('link', {name: 'Alphabetical'})).not.toBeNull(); expect(view.el.querySelector('.drop_down_button_item .label')).toHaveTextContent('Sort'); }); + + describe('deleting the selection', () => { + let testContext; + + beforeEach(() => { + testContext = {}; + }); + + support.useFakeXhr(() => testContext); + + function destroyButton(queries) { + return queries.getByRole('button', {name: 'Delete selection'}); + } + + it('deletes all checked files once the confirmation is accepted', async () => { + const {entry, queries} = setup(); + const user = userEvent.setup(); + window.confirm = jest.fn(() => true); + + await startSelecting(user, queries); + await user.click(checkBoxFor(queries, 'image.png')); + await user.click(checkBoxFor(queries, 'photo.png')); + await user.click(destroyButton(queries)); + + expect(window.confirm).toHaveBeenCalledWith('Really delete 2 items?'); + expect(testContext.requests.map(request => request.method)).toEqual(['DELETE', 'DELETE']); + expect(entry.getFileCollection('image_files').length).toEqual(0); + }); + + it('keeps the files when the confirmation is dismissed', async () => { + const {view, entry, queries} = setup(); + const user = userEvent.setup(); + window.confirm = jest.fn(() => false); + + await startSelecting(user, queries); + await user.click(checkBoxFor(queries, 'image.png')); + await user.click(destroyButton(queries)); + + expect(testContext.requests).toEqual([]); + expect(entry.getFileCollection('image_files').length).toEqual(2); + expect(list(view)).toHaveClass('is_selecting'); + expect(selectionBar(view)).toHaveTextContent('1 item selected'); + }); + + it('stops checking files once they have been deleted', async () => { + const {view, queries} = setup(); + const user = userEvent.setup(); + window.confirm = jest.fn(() => true); + + await startSelecting(user, queries); + await user.click(checkBoxFor(queries, 'image.png')); + await user.click(destroyButton(queries)); + + expect(list(view)).not.toHaveClass('is_selecting'); + expect(selectionBar(view)).toHaveTextContent('No items selected'); + }); + + it('is not offered while nothing is checked', async () => { + const {queries} = setup(); + const user = userEvent.setup(); + + await startSelecting(user, queries); + + expect(destroyButton(queries)).toBeDisabled(); + + await user.click(checkBoxFor(queries, 'image.png')); + + expect(destroyButton(queries)).toBeEnabled(); + }); + }); }); describe('banner', () => { diff --git a/package/src/editor/collections/ListSelection.js b/package/src/editor/collections/ListSelection.js index 035e613736..6434e6cf09 100644 --- a/package/src/editor/collections/ListSelection.js +++ b/package/src/editor/collections/ListSelection.js @@ -34,5 +34,13 @@ export const ListSelection = Backbone.Collection.extend({ includes: function(item) { return !!this.get(item); + }, + + // Iterating a copy since destroying an item takes it out of the list, + // which drops it from the selection and would skip the item behind it. + destroyAll: function() { + this.models.slice().forEach(function(item) { + item.destroy(); + }); } }); diff --git a/package/src/editor/templates/filteredFiles.jst b/package/src/editor/templates/filteredFiles.jst index ff85d1b0e8..725fc6f3e4 100644 --- a/package/src/editor/templates/filteredFiles.jst +++ b/package/src/editor/templates/filteredFiles.jst @@ -14,7 +14,10 @@ - +
+ + +
diff --git a/package/src/editor/views/FilteredFilesView.js b/package/src/editor/views/FilteredFilesView.js index 9526d491bf..97d3110a9c 100644 --- a/package/src/editor/views/FilteredFilesView.js +++ b/package/src/editor/views/FilteredFilesView.js @@ -16,6 +16,7 @@ import {FilesBlankSlateView} from './FilesBlankSlateView'; import {FilesListItemView} from './FilesListItemView'; import {FileTypePillsView} from './FileTypePillsView'; import {FolderBreadcrumbView} from './FolderBreadcrumbView'; +import {FileFolder} from '../models/FileFolder'; import {Search} from '../models/Search'; import {ListHighlight} from '../models/ListHighlight'; import {ListSearchFieldView} from './ListSearchFieldView'; @@ -37,6 +38,7 @@ export const FilteredFilesView = Marionette.ItemView.extend({ selectionBar: '.filtered_files-selection_bar', selectionBarText: '.filtered_files-selection_bar_text', selectionBarAction: '.filtered_files-selection_bar_action', + selectionBarDestroy: '.filtered_files-selection_bar_destroy', selectionBarDismiss: '.filtered_files-selection_bar_dismiss', list: '.filtered_files-list', sort: '.filtered_files-sort', @@ -51,8 +53,8 @@ export const FilteredFilesView = Marionette.ItemView.extend({ return false; }, - // Moving is all a selection is good for so far, so the check boxes - // go away again once the files have been moved. + // Each bulk action is the point of the selection it acts on, so the + // check boxes go away again once one of them has run. 'click .filtered_files-selection_bar_action': function() { MoveToFolderDialogView.open({ models: this.listSelection.models, @@ -63,6 +65,11 @@ export const FilteredFilesView = Marionette.ItemView.extend({ return false; }, + 'click .filtered_files-selection_bar_destroy': function() { + this.destroySelection(); + return false; + }, + 'click .filtered_files-selection_bar_dismiss': function() { this.stopSelecting(); return false; @@ -308,6 +315,18 @@ export const FilteredFilesView = Marionette.ItemView.extend({ this.listSelection.stop(); }, + destroySelection: function() { + var message = this.translation('confirm_destroy_selection', + {count: this.listSelection.length}); + + if (!window.confirm(message)) { + return; + } + + this.listSelection.destroyAll(); + this.stopSelecting(); + }, + updateSelecting: function() { if (!this.listSelection) { return this.ui.selectionBar.remove(); @@ -320,12 +339,14 @@ export const FilteredFilesView = Marionette.ItemView.extend({ this.collectionView.$el.toggleClass('is_selecting', selecting); var dismissLabel = this.translation('end_selection'); + var destroyLabel = this.translation('destroy_selection'); this.ui.selectionBarDismiss.attr({title: dismissLabel, 'aria-label': dismissLabel}); this.ui.selectionBarAction.text(this.translation('move_selection')); + this.ui.selectionBarDestroy.attr({title: destroyLabel, 'aria-label': destroyLabel}); // Files can only be moved into folders, so entries which have none - // offer nothing to do with a selection yet. + // offer nothing to move a selection into. this.ui.selectionBarAction.toggle(!!this.options.fileFolders); this.updateSelectionBar(); @@ -355,6 +376,17 @@ export const FilteredFilesView = Marionette.ItemView.extend({ {count: this.listSelection.length})); this.ui.selectionBarAction.prop('disabled', !this.listSelection.length); + this.ui.selectionBarDestroy.prop('disabled', !this.listSelection.length || + this.selectionContainsNonEmptyFolder()); + }, + + selectionContainsNonEmptyFolder: function() { + var files = this.selectedFiles || this.combinedFiles; + + return this.listSelection.some(function(model) { + return model instanceof FileFolder && + !this.options.fileFolders.isEmptyFolder(model, files); + }, this); }, translation: function(keyName, options) { From a35754770916cd8cd6cfb38900d8c3ae2e942b93 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Fri, 21 Aug 2026 15:26:49 +0200 Subject: [PATCH 3/4] Translate the confirmation for deleting a single file Both file rows asked in a hardcoded German sentence, which also had a word twice. Each view now reads its own confirm_destroy, the way every other delete confirmation in the editor gets its wording. --- config/locales/de.yml | 4 ++ config/locales/en.yml | 4 ++ .../views/ConfirmableFileItemView-spec.js | 38 +++++++++++++++++++ .../spec/editor/views/FileItemView-spec.js | 17 ++++++++- .../editor/views/ConfirmableFileItemView.js | 3 +- package/src/editor/views/FileItemView.js | 2 +- 6 files changed, 65 insertions(+), 3 deletions(-) diff --git a/config/locales/de.yml b/config/locales/de.yml index d89e462cfb..8f75ebff71 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -1535,6 +1535,8 @@ de: chapter_item_view: chapter: Kapitel unnamed: "(Unbenannt)" + confirmable_file_item_view: + confirm_destroy: Datei wirklich löschen? edit_chapter_view: confirm_destroy: |- Kapitel einschließlich ALLER enthaltener Seiten wirklich löschen? @@ -1565,6 +1567,8 @@ de: show_editor: Editorleiste einblenden entry_preview_view: scroll_hint: Scrollen, um weiterzulesen + file_item_view: + confirm_destroy: Datei wirklich löschen? file_meta_data_item_value_view: blank: "-" file_preview_progress_bar_view: diff --git a/config/locales/en.yml b/config/locales/en.yml index d0e6b6f982..510f61afe9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1527,6 +1527,8 @@ en: chapter_item_view: chapter: Chapter unnamed: "(New chapter)" + confirmable_file_item_view: + confirm_destroy: Really delete this file? edit_chapter_view: confirm_destroy: |- Really delete this chapter including ALL its pages? @@ -1557,6 +1559,8 @@ en: show_editor: Show editor panel entry_preview_view: scroll_hint: Scroll down to continue + file_item_view: + confirm_destroy: Really delete this file? file_meta_data_item_value_view: blank: "-" file_preview_progress_bar_view: diff --git a/package/spec/editor/views/ConfirmableFileItemView-spec.js b/package/spec/editor/views/ConfirmableFileItemView-spec.js index b06f8187c6..1086328de4 100644 --- a/package/spec/editor/views/ConfirmableFileItemView-spec.js +++ b/package/spec/editor/views/ConfirmableFileItemView-spec.js @@ -4,6 +4,11 @@ import * as support from '$support'; import {renderBackboneView as render} from 'pageflow/testHelpers'; describe('ConfirmableFileItemView', () => { + support.useFakeTranslations({ + 'pageflow.editor.templates.confirmable_file_item.remove': 'Delete', + 'pageflow.editor.views.confirmable_file_item_view.confirm_destroy': 'Really delete this file?' + }); + it('displays file title', () => { const file = support.factories.file({file_name: 'original.mp4'}); @@ -16,4 +21,37 @@ describe('ConfirmableFileItemView', () => { expect(getByText('original.mp4')).not.toBeNull(); }); + + it('destroys file when delete is clicked', () => { + window.confirm = jest.fn(() => true); + const file = support.factories.file({file_name: 'original.mp4'}); + jest.spyOn(file, 'destroy').mockImplementation(() => {}); + + const view = new ConfirmableFileItemView({ + model: file, + selectedFiles: new Backbone.Collection() + }); + + const {getByTitle} = render(view); + getByTitle('Delete').click(); + + expect(window.confirm).toHaveBeenCalledWith('Really delete this file?'); + expect(file.destroy).toHaveBeenCalled(); + }); + + it('keeps file when the confirmation is dismissed', () => { + window.confirm = jest.fn(() => false); + const file = support.factories.file({file_name: 'original.mp4'}); + jest.spyOn(file, 'destroy').mockImplementation(() => {}); + + const view = new ConfirmableFileItemView({ + model: file, + selectedFiles: new Backbone.Collection() + }); + + const {getByTitle} = render(view); + getByTitle('Delete').click(); + + expect(file.destroy).not.toHaveBeenCalled(); + }); }); diff --git a/package/spec/editor/views/FileItemView-spec.js b/package/spec/editor/views/FileItemView-spec.js index 41b1eb6713..8c69c10238 100644 --- a/package/spec/editor/views/FileItemView-spec.js +++ b/package/spec/editor/views/FileItemView-spec.js @@ -22,7 +22,8 @@ describe('FileItemView', () => { 'pageflow.editor.templates.file_item.move': 'Move...', 'pageflow.editor.templates.file_item.settings': 'Settings', 'pageflow.editor.templates.file_item.select': 'Select', - 'pageflow.editor.templates.files.in_folder': 'In folder:' + 'pageflow.editor.templates.files.in_folder': 'In folder:', + 'pageflow.editor.views.file_item_view.confirm_destroy': 'Really delete this file?' }); it('displays file title', () => { @@ -406,9 +407,23 @@ describe('FileItemView', () => { const {getByRole} = render(view); getByRole('link', {name: 'Delete'}).click(); + expect(window.confirm).toHaveBeenCalledWith('Really delete this file?'); expect(file.destroy).toHaveBeenCalled(); }); + it('keeps file when the confirmation is dismissed', () => { + window.confirm = jest.fn(() => false); + const file = support.factories.file({id: 123}); + jest.spyOn(file, 'destroy').mockImplementation(() => {}); + + const view = new FileItemView({model: file}); + + const {getByRole} = render(view); + getByRole('link', {name: 'Delete'}).click(); + + expect(file.destroy).not.toHaveBeenCalled(); + }); + it('cancels upload when cancel upload is selected', () => { const file = support.factories.file({id: 123, state: 'uploading'}); jest.spyOn(file, 'cancelUpload').mockImplementation(() => {}); diff --git a/package/src/editor/views/ConfirmableFileItemView.js b/package/src/editor/views/ConfirmableFileItemView.js index d39c611426..25cc6aa023 100644 --- a/package/src/editor/views/ConfirmableFileItemView.js +++ b/package/src/editor/views/ConfirmableFileItemView.js @@ -1,4 +1,5 @@ import Marionette from 'backbone.marionette'; +import I18n from 'i18n-js'; import template from '../templates/confirmableFileItem.jst'; @@ -31,7 +32,7 @@ export const ConfirmableFileItemView = Marionette.ItemView.extend({ }, destroy: function() { - if (confirm("Datei wirklich wirklich löschen?")) { + if (window.confirm(I18n.t('pageflow.editor.views.confirmable_file_item_view.confirm_destroy'))) { this.model.destroy(); } }, diff --git a/package/src/editor/views/FileItemView.js b/package/src/editor/views/FileItemView.js index 6253d12904..f367656074 100644 --- a/package/src/editor/views/FileItemView.js +++ b/package/src/editor/views/FileItemView.js @@ -303,7 +303,7 @@ export const FileItemView = Marionette.ItemView.extend({ }, destroy: function() { - if (confirm("Datei wirklich wirklich löschen?")) { + if (window.confirm(I18n.t('pageflow.editor.views.file_item_view.confirm_destroy'))) { this.model.destroy(); } }, From 86284b5d150a971da3373e4623b5a7d92a739787 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 24 Aug 2026 10:08:08 +0200 Subject: [PATCH 4/4] Point DestroyMenuItem at the translations it documents The default prefix was missing the views segment, so the keys it looked up existed nowhere. Every subclass so far sets its own prefix, which is why nothing showed a missing translation yet. --- package/spec/editor/models/DestroyMenuItem-spec.js | 4 ++-- package/src/editor/models/DestroyMenuItem.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package/spec/editor/models/DestroyMenuItem-spec.js b/package/spec/editor/models/DestroyMenuItem-spec.js index 255467a86b..501ec90bfb 100644 --- a/package/spec/editor/models/DestroyMenuItem-spec.js +++ b/package/spec/editor/models/DestroyMenuItem-spec.js @@ -3,8 +3,8 @@ import {useFakeTranslations} from 'pageflow/testHelpers'; describe('DestroyMenuItem', () => { useFakeTranslations({ - 'pageflow.editor.destroy_menu_item.destroy': 'Delete', - 'pageflow.editor.destroy_menu_item.confirm_destroy': 'Really delete?' + 'pageflow.editor.views.destroy_menu_item.destroy': 'Delete', + 'pageflow.editor.views.destroy_menu_item.confirm_destroy': 'Really delete?' }); it('has name destroy by default', () => { diff --git a/package/src/editor/models/DestroyMenuItem.js b/package/src/editor/models/DestroyMenuItem.js index 83256c9fea..64826cf4da 100644 --- a/package/src/editor/models/DestroyMenuItem.js +++ b/package/src/editor/models/DestroyMenuItem.js @@ -18,7 +18,7 @@ import I18n from 'i18n-js'; * @since edge */ export const DestroyMenuItem = Backbone.Model.extend({ - translationKeyPrefix: 'pageflow.editor.destroy_menu_item', + translationKeyPrefix: 'pageflow.editor.views.destroy_menu_item', defaults: { name: 'destroy',