From 220f867814b5e43ed9735d748f7c899e55e353b5 Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Sat, 21 Sep 2024 05:05:27 -0700 Subject: [PATCH] Mobile: Resolves #10763: Support permanent note deletion on mobile (#10786) --- .eslintignore | 4 +- .gitignore | 4 +- .../gui/MainScreen/commands/index.ts | 4 -- .../components/screens/Note.test.tsx | 19 +++++++ .../app-mobile/components/screens/Note.tsx | 56 ++++++++----------- .../MainScreen => lib}/commands/deleteNote.ts | 6 +- packages/lib/commands/index.ts | 4 ++ .../commands/permanentlyDeleteNote.ts | 15 +++-- .../components/shared/note-screen-shared.ts | 41 +++++++++----- 9 files changed, 88 insertions(+), 65 deletions(-) rename packages/{app-desktop/gui/MainScreen => lib}/commands/deleteNote.ts (85%) rename packages/{app-desktop/gui/MainScreen => lib}/commands/permanentlyDeleteNote.ts (73%) diff --git a/.eslintignore b/.eslintignore index 1ae03ae0b..f02967c9e 100644 --- a/.eslintignore +++ b/.eslintignore @@ -209,7 +209,6 @@ packages/app-desktop/gui/MainScreen/MainScreen.js packages/app-desktop/gui/MainScreen/commands/addProfile.js packages/app-desktop/gui/MainScreen/commands/commandPalette.js packages/app-desktop/gui/MainScreen/commands/deleteFolder.js -packages/app-desktop/gui/MainScreen/commands/deleteNote.js packages/app-desktop/gui/MainScreen/commands/duplicateNote.js packages/app-desktop/gui/MainScreen/commands/editAlarm.js packages/app-desktop/gui/MainScreen/commands/exportPdf.js @@ -228,7 +227,6 @@ packages/app-desktop/gui/MainScreen/commands/openItem.js packages/app-desktop/gui/MainScreen/commands/openNote.js packages/app-desktop/gui/MainScreen/commands/openPdfViewer.js packages/app-desktop/gui/MainScreen/commands/openTag.js -packages/app-desktop/gui/MainScreen/commands/permanentlyDeleteNote.js packages/app-desktop/gui/MainScreen/commands/print.js packages/app-desktop/gui/MainScreen/commands/renameFolder.js packages/app-desktop/gui/MainScreen/commands/renameTag.js @@ -921,10 +919,12 @@ packages/lib/array.js packages/lib/callbackUrlUtils.test.js packages/lib/callbackUrlUtils.js packages/lib/clipperUtils.js +packages/lib/commands/deleteNote.js packages/lib/commands/historyBackward.js packages/lib/commands/historyForward.js packages/lib/commands/index.js packages/lib/commands/openMasterPasswordDialog.js +packages/lib/commands/permanentlyDeleteNote.js packages/lib/commands/synchronize.js packages/lib/components/EncryptionConfigScreen/utils.test.js packages/lib/components/EncryptionConfigScreen/utils.js diff --git a/.gitignore b/.gitignore index fcef98154..0193755dc 100644 --- a/.gitignore +++ b/.gitignore @@ -186,7 +186,6 @@ packages/app-desktop/gui/MainScreen/MainScreen.js packages/app-desktop/gui/MainScreen/commands/addProfile.js packages/app-desktop/gui/MainScreen/commands/commandPalette.js packages/app-desktop/gui/MainScreen/commands/deleteFolder.js -packages/app-desktop/gui/MainScreen/commands/deleteNote.js packages/app-desktop/gui/MainScreen/commands/duplicateNote.js packages/app-desktop/gui/MainScreen/commands/editAlarm.js packages/app-desktop/gui/MainScreen/commands/exportPdf.js @@ -205,7 +204,6 @@ packages/app-desktop/gui/MainScreen/commands/openItem.js packages/app-desktop/gui/MainScreen/commands/openNote.js packages/app-desktop/gui/MainScreen/commands/openPdfViewer.js packages/app-desktop/gui/MainScreen/commands/openTag.js -packages/app-desktop/gui/MainScreen/commands/permanentlyDeleteNote.js packages/app-desktop/gui/MainScreen/commands/print.js packages/app-desktop/gui/MainScreen/commands/renameFolder.js packages/app-desktop/gui/MainScreen/commands/renameTag.js @@ -898,10 +896,12 @@ packages/lib/array.js packages/lib/callbackUrlUtils.test.js packages/lib/callbackUrlUtils.js packages/lib/clipperUtils.js +packages/lib/commands/deleteNote.js packages/lib/commands/historyBackward.js packages/lib/commands/historyForward.js packages/lib/commands/index.js packages/lib/commands/openMasterPasswordDialog.js +packages/lib/commands/permanentlyDeleteNote.js packages/lib/commands/synchronize.js packages/lib/components/EncryptionConfigScreen/utils.test.js packages/lib/components/EncryptionConfigScreen/utils.js diff --git a/packages/app-desktop/gui/MainScreen/commands/index.ts b/packages/app-desktop/gui/MainScreen/commands/index.ts index b2e503472..f238c44bd 100644 --- a/packages/app-desktop/gui/MainScreen/commands/index.ts +++ b/packages/app-desktop/gui/MainScreen/commands/index.ts @@ -2,7 +2,6 @@ import * as addProfile from './addProfile'; import * as commandPalette from './commandPalette'; import * as deleteFolder from './deleteFolder'; -import * as deleteNote from './deleteNote'; import * as duplicateNote from './duplicateNote'; import * as editAlarm from './editAlarm'; import * as exportPdf from './exportPdf'; @@ -20,7 +19,6 @@ import * as openItem from './openItem'; import * as openNote from './openNote'; import * as openPdfViewer from './openPdfViewer'; import * as openTag from './openTag'; -import * as permanentlyDeleteNote from './permanentlyDeleteNote'; import * as print from './print'; import * as renameFolder from './renameFolder'; import * as renameTag from './renameTag'; @@ -52,7 +50,6 @@ const index: any[] = [ addProfile, commandPalette, deleteFolder, - deleteNote, duplicateNote, editAlarm, exportPdf, @@ -70,7 +67,6 @@ const index: any[] = [ openNote, openPdfViewer, openTag, - permanentlyDeleteNote, print, renameFolder, renameTag, diff --git a/packages/app-mobile/components/screens/Note.test.tsx b/packages/app-mobile/components/screens/Note.test.tsx index f5d5c21ff..8c4ba0798 100644 --- a/packages/app-mobile/components/screens/Note.test.tsx +++ b/packages/app-mobile/components/screens/Note.test.tsx @@ -23,6 +23,7 @@ import ItemChange from '@joplin/lib/models/ItemChange'; import { getDisplayParentId } from '@joplin/lib/services/trash'; import { itemIsReadOnlySync, ItemSlice } from '@joplin/lib/models/utils/readOnly'; import { LayoutChangeEvent } from 'react-native'; +import shim from '@joplin/lib/shim'; interface WrapperProps { } @@ -140,6 +141,24 @@ describe('Note', () => { }); }); + it('pressing "delete permanently" should permanently delete a note', async () => { + const noteId = await openNewNote({ title: 'To be deleted', body: '...', deleted_time: Date.now() }); + render(); + + // Permanently delete note shows a confirmation dialog -- mock it. + const deleteId = 0; + shim.showMessageBox = jest.fn(async () => deleteId); + + await openNoteActionsMenu(); + const deleteButton = await screen.findByText('Permanently delete note'); + fireEvent.press(deleteButton); + + await waitFor(async () => { + expect(await Note.load(noteId)).toBeUndefined(); + }); + expect(shim.showMessageBox).toHaveBeenCalled(); + }); + it('delete should be disabled in a read-only note', async () => { const shareId = 'testShare'; const noteId = await openNewNote({ diff --git a/packages/app-mobile/components/screens/Note.tsx b/packages/app-mobile/components/screens/Note.tsx index dd97b919c..9bb82db71 100644 --- a/packages/app-mobile/components/screens/Note.tsx +++ b/packages/app-mobile/components/screens/Note.tsx @@ -627,21 +627,6 @@ class NoteScreenComponent extends BaseScreenComponent implements B await shared.saveOneProperty(this, name, value); } - private async deleteNote_onPress() { - const note = this.state.note; - if (!note.id) return; - - const folderId = note.parent_id; - - await Note.delete(note.id, { toTrash: true, sourceDescription: 'Delete note button' }); - - this.props.dispatch({ - type: 'NAV_GO', - routeName: 'Notes', - folderId: folderId, - }); - } - private async pickDocuments() { const result = await pickDocument({ multiple: true }); return result; @@ -1283,30 +1268,33 @@ class NoteScreenComponent extends BaseScreenComponent implements B }); } - output.push({ - title: _('Delete'), - onPress: () => { - void this.deleteNote_onPress(); - }, - disabled: readOnly, - }); + const commandService = CommandService.instance(); + const whenContext = commandService.currentWhenClauseContext(); + const addButtonFromCommand = (commandName: string, title?: string) => { + if (commandName === '-') { + output.push({ isDivider: true }); + } else { + output.push({ + title: title ?? commandService.description(commandName), + onPress: async () => { + void commandService.execute(commandName); + }, + disabled: !commandService.isEnabled(commandName, whenContext), + }); + } + }; + + if (whenContext.inTrash) { + addButtonFromCommand('permanentlyDeleteNote'); + } else { + addButtonFromCommand('deleteNote', _('Delete')); + } if (pluginCommands.length) { output.push({ isDivider: true }); - const commandService = CommandService.instance(); for (const commandName of pluginCommands) { - if (commandName === '-') { - output.push({ isDivider: true }); - } else { - output.push({ - title: commandService.description(commandName), - onPress: async () => { - void commandService.execute(commandName); - }, - disabled: !commandService.isEnabled(commandName), - }); - } + addButtonFromCommand(commandName); } } diff --git a/packages/app-desktop/gui/MainScreen/commands/deleteNote.ts b/packages/lib/commands/deleteNote.ts similarity index 85% rename from packages/app-desktop/gui/MainScreen/commands/deleteNote.ts rename to packages/lib/commands/deleteNote.ts index 23735f1cd..9e0e55f93 100644 --- a/packages/app-desktop/gui/MainScreen/commands/deleteNote.ts +++ b/packages/lib/commands/deleteNote.ts @@ -1,6 +1,6 @@ -import { CommandRuntime, CommandDeclaration, CommandContext } from '@joplin/lib/services/CommandService'; -import { _ } from '@joplin/lib/locale'; -import Note from '@joplin/lib/models/Note'; +import { CommandRuntime, CommandDeclaration, CommandContext } from '../services/CommandService'; +import { _ } from '../locale'; +import Note from '../models/Note'; export const declaration: CommandDeclaration = { name: 'deleteNote', diff --git a/packages/lib/commands/index.ts b/packages/lib/commands/index.ts index 049eace48..334f5e100 100644 --- a/packages/lib/commands/index.ts +++ b/packages/lib/commands/index.ts @@ -1,13 +1,17 @@ // AUTO-GENERATED using `gulp buildScriptIndexes` +import * as deleteNote from './deleteNote'; import * as historyBackward from './historyBackward'; import * as historyForward from './historyForward'; import * as openMasterPasswordDialog from './openMasterPasswordDialog'; +import * as permanentlyDeleteNote from './permanentlyDeleteNote'; import * as synchronize from './synchronize'; const index: any[] = [ + deleteNote, historyBackward, historyForward, openMasterPasswordDialog, + permanentlyDeleteNote, synchronize, ]; diff --git a/packages/app-desktop/gui/MainScreen/commands/permanentlyDeleteNote.ts b/packages/lib/commands/permanentlyDeleteNote.ts similarity index 73% rename from packages/app-desktop/gui/MainScreen/commands/permanentlyDeleteNote.ts rename to packages/lib/commands/permanentlyDeleteNote.ts index 67782e69e..d54595590 100644 --- a/packages/app-desktop/gui/MainScreen/commands/permanentlyDeleteNote.ts +++ b/packages/lib/commands/permanentlyDeleteNote.ts @@ -1,7 +1,7 @@ -import { CommandRuntime, CommandDeclaration, CommandContext } from '@joplin/lib/services/CommandService'; -import { _ } from '@joplin/lib/locale'; -import Note from '@joplin/lib/models/Note'; -import bridge from '../../../services/bridge'; +import { CommandRuntime, CommandDeclaration, CommandContext } from '../services/CommandService'; +import { _ } from '../locale'; +import Note from '../models/Note'; +import shim from '../shim'; export const declaration: CommandDeclaration = { name: 'permanentlyDeleteNote', @@ -16,12 +16,15 @@ export const runtime = (): CommandRuntime => { if (!noteIds.length) return; const msg = await Note.permanentlyDeleteMessage(noteIds); - const ok = bridge().showConfirmMessageBox(msg, { + const deleteIndex = 0; + const result = await shim.showMessageBox(msg, { buttons: [_('Delete'), _('Cancel')], defaultId: 1, + cancelId: 1, + type: 'question', }); - if (ok) { + if (result === deleteIndex) { await Note.batchDelete(noteIds, { toTrash: false, sourceDescription: 'permanentlyDeleteNote command' }); } }, diff --git a/packages/lib/components/shared/note-screen-shared.ts b/packages/lib/components/shared/note-screen-shared.ts index b6a1eb130..443d97dd3 100644 --- a/packages/lib/components/shared/note-screen-shared.ts +++ b/packages/lib/components/shared/note-screen-shared.ts @@ -280,19 +280,32 @@ shared.initState = async function(comp: BaseNoteScreenComponent) { comp.scheduleFocusUpdate(); } - const folder = Folder.byId(comp.props.folders, note.parent_id); - - comp.setState({ - lastSavedNote: { ...note }, - note: note, - mode: mode, - folder: folder, - isLoading: false, - fromShare: !!comp.props.sharedData, - noteResources: await shared.attachedResources(note ? note.body : ''), - readOnly: itemIsReadOnlySync(ModelType.Note, ItemChange.SOURCE_UNSPECIFIED, note as ItemSlice, Setting.value('sync.userId'), BaseItem.syncShareCache), - }); - + const fromShare = !!comp.props.sharedData; + if (note) { + const folder = Folder.byId(comp.props.folders, note.parent_id); + comp.setState({ + lastSavedNote: { ...note }, + note: note, + mode: mode, + folder: folder, + isLoading: false, + fromShare: !!comp.props.sharedData, + noteResources: await shared.attachedResources(note ? note.body : ''), + readOnly: itemIsReadOnlySync(ModelType.Note, ItemChange.SOURCE_UNSPECIFIED, note as ItemSlice, Setting.value('sync.userId'), BaseItem.syncShareCache), + }); + } else { + // Handle the case where a non-existent note is loaded. This can happen briefly after deleting a note. + comp.setState({ + lastSavedNote: {}, + note: {}, + mode, + folder: null, + isLoading: true, + fromShare, + noteResources: [], + readOnly: true, + }); + } if (comp.props.sharedData) { if (comp.props.sharedData.title) { @@ -315,7 +328,7 @@ shared.initState = async function(comp: BaseNoteScreenComponent) { } // eslint-disable-next-line require-atomic-updates - comp.lastLoadedNoteId_ = note.id; + comp.lastLoadedNoteId_ = note?.id; }; shared.toggleIsTodo_onPress = function(comp: BaseNoteScreenComponent) {