From c4a7749f2a9572098e4548e04ed6485cca4690ac Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Thu, 26 Sep 2024 03:35:32 -0700 Subject: [PATCH] Desktop: Fixes #11105: Plugin API: Save changes made with `editor.setText` (#11117) --- .eslintignore | 1 + .gitignore | 1 + .../app-desktop/gui/NoteEditor/NoteEditor.tsx | 18 +++--- .../utils/useWindowCommandHandler.ts | 18 +++--- .../integration-tests/models/GoToAnything.ts | 12 ++++ .../models/NoteEditorScreen.ts | 34 ++++++++++- .../integration-tests/pluginApi.spec.ts | 32 +++++++++++ .../resources/test-plugins/execCommand.js | 31 ++++++++++ .../integration-tests/richTextEditor.spec.ts | 4 +- .../integration-tests/util/test.ts | 56 +++++++++++++++---- packages/app-desktop/plugins/GotoAnything.tsx | 15 ++++- 11 files changed, 188 insertions(+), 34 deletions(-) create mode 100644 packages/app-desktop/integration-tests/pluginApi.spec.ts create mode 100644 packages/app-desktop/integration-tests/resources/test-plugins/execCommand.js diff --git a/.eslintignore b/.eslintignore index b616e04af..3aac494c7 100644 --- a/.eslintignore +++ b/.eslintignore @@ -475,6 +475,7 @@ packages/app-desktop/integration-tests/models/NoteList.js packages/app-desktop/integration-tests/models/SettingsScreen.js packages/app-desktop/integration-tests/models/Sidebar.js packages/app-desktop/integration-tests/noteList.spec.js +packages/app-desktop/integration-tests/pluginApi.spec.js packages/app-desktop/integration-tests/richTextEditor.spec.js packages/app-desktop/integration-tests/settings.spec.js packages/app-desktop/integration-tests/sidebar.spec.js diff --git a/.gitignore b/.gitignore index e7cbe4d7c..7bb7996a2 100644 --- a/.gitignore +++ b/.gitignore @@ -452,6 +452,7 @@ packages/app-desktop/integration-tests/models/NoteList.js packages/app-desktop/integration-tests/models/SettingsScreen.js packages/app-desktop/integration-tests/models/Sidebar.js packages/app-desktop/integration-tests/noteList.spec.js +packages/app-desktop/integration-tests/pluginApi.spec.js packages/app-desktop/integration-tests/richTextEditor.spec.js packages/app-desktop/integration-tests/settings.spec.js packages/app-desktop/integration-tests/sidebar.spec.js diff --git a/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx b/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx index d832166ba..ac308550c 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx @@ -218,15 +218,6 @@ function NoteEditor(props: NoteEditorProps) { } }, [handleProvisionalFlag, formNote, setFormNote, isNewNote, titleHasBeenManuallyChanged, scheduleNoteListResort, scheduleSaveNote]); - useWindowCommandHandler({ - dispatch: props.dispatch, - setShowLocalSearch, - noteSearchBarRef, - editorRef, - titleInputRef, - setFormNote, - }); - const onDrop = useDropHandler({ editorRef }); const onBodyChange = useCallback((event: OnChangeEvent) => onFieldChange('body', event.content, event.changeId), [onFieldChange]); @@ -234,6 +225,15 @@ function NoteEditor(props: NoteEditorProps) { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied const onTitleChange = useCallback((event: any) => onFieldChange('title', event.target.value), [onFieldChange]); + useWindowCommandHandler({ + dispatch: props.dispatch, + setShowLocalSearch, + noteSearchBarRef, + editorRef, + titleInputRef, + onBodyChange, + }); + // const onTitleKeydown = useCallback((event:any) => { // const keyCode = event.keyCode; diff --git a/packages/app-desktop/gui/NoteEditor/utils/useWindowCommandHandler.ts b/packages/app-desktop/gui/NoteEditor/utils/useWindowCommandHandler.ts index ae739b762..c8aa14e01 100644 --- a/packages/app-desktop/gui/NoteEditor/utils/useWindowCommandHandler.ts +++ b/packages/app-desktop/gui/NoteEditor/utils/useWindowCommandHandler.ts @@ -1,5 +1,5 @@ import { RefObject, useEffect } from 'react'; -import { FormNote, NoteBodyEditorRef, ScrollOptionTypes } from './types'; +import { NoteBodyEditorRef, OnChangeEvent, ScrollOptionTypes } from './types'; import editorCommandDeclarations, { enabledCondition } from '../editorCommandDeclarations'; import CommandService, { CommandDeclaration, CommandRuntime, CommandContext } from '@joplin/lib/services/CommandService'; import time from '@joplin/lib/time'; @@ -12,7 +12,7 @@ const commandsWithDependencies = [ require('../commands/pasteAsText'), ]; -type SetFormNoteCallback = (callback: (prev: FormNote)=> FormNote)=> void; +type OnBodyChange = (event: OnChangeEvent)=> void; interface HookDependencies { // eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied @@ -23,13 +23,13 @@ interface HookDependencies { noteSearchBarRef: any; editorRef: RefObject; titleInputRef: RefObject; - setFormNote: SetFormNoteCallback; + onBodyChange: OnBodyChange; } function editorCommandRuntime( declaration: CommandDeclaration, editorRef: RefObject, - setFormNote: SetFormNoteCallback, + onBodyChange: OnBodyChange, ): CommandRuntime { return { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied @@ -55,9 +55,7 @@ function editorCommandRuntime( value: args[0], }); } else if (declaration.name === 'editor.setText') { - setFormNote((prev: FormNote) => { - return { ...prev, body: args[0] }; - }); + onBodyChange({ content: args[0], changeId: 0 }); } else { return editorRef.current.execCommand({ name: declaration.name, @@ -78,11 +76,11 @@ function editorCommandRuntime( } export default function useWindowCommandHandler(dependencies: HookDependencies) { - const { setShowLocalSearch, noteSearchBarRef, editorRef, titleInputRef, setFormNote } = dependencies; + const { setShowLocalSearch, noteSearchBarRef, editorRef, titleInputRef, onBodyChange } = dependencies; useEffect(() => { for (const declaration of editorCommandDeclarations) { - CommandService.instance().registerRuntime(declaration.name, editorCommandRuntime(declaration, editorRef, setFormNote)); + CommandService.instance().registerRuntime(declaration.name, editorCommandRuntime(declaration, editorRef, onBodyChange)); } const dependencies = { @@ -105,5 +103,5 @@ export default function useWindowCommandHandler(dependencies: HookDependencies) CommandService.instance().unregisterRuntime(command.declaration.name); } }; - }, [editorRef, setShowLocalSearch, noteSearchBarRef, titleInputRef, setFormNote]); + }, [editorRef, setShowLocalSearch, noteSearchBarRef, titleInputRef, onBodyChange]); } diff --git a/packages/app-desktop/integration-tests/models/GoToAnything.ts b/packages/app-desktop/integration-tests/models/GoToAnything.ts index ee141bfb8..86ed9cd57 100644 --- a/packages/app-desktop/integration-tests/models/GoToAnything.ts +++ b/packages/app-desktop/integration-tests/models/GoToAnything.ts @@ -30,4 +30,16 @@ export default class GoToAnything { public async expectToBeOpen() { await expect(this.containerLocator).toBeAttached(); } + + public async runCommand(electronApp: ElectronApplication, command: string) { + if (!command.startsWith(':')) { + command = `:${command}`; + } + + await this.open(electronApp); + await this.inputLocator.fill(command); + await this.containerLocator.locator('.match-highlight').first().waitFor(); + await this.inputLocator.press('Enter'); + await this.expectToBeClosed(); + } } diff --git a/packages/app-desktop/integration-tests/models/NoteEditorScreen.ts b/packages/app-desktop/integration-tests/models/NoteEditorScreen.ts index 3f336509b..0fe5cc88d 100644 --- a/packages/app-desktop/integration-tests/models/NoteEditorScreen.ts +++ b/packages/app-desktop/integration-tests/models/NoteEditorScreen.ts @@ -1,5 +1,6 @@ import { Locator, Page } from '@playwright/test'; +import { expect } from '../util/test'; export default class NoteEditorPage { public readonly codeMirrorEditor: Locator; @@ -31,6 +32,31 @@ export default class NoteEditorPage { return this.containerLocator.getByRole('button', { name: title }); } + public async contentLocator() { + const richTextBody = this.getRichTextFrameLocator().locator('body'); + const markdownEditor = this.codeMirrorEditor; + + // Work around an issue where .or doesn't work with frameLocators. + // See https://github.com/microsoft/playwright/issues/27688#issuecomment-1771403495 + await Promise.race([ + richTextBody.waitFor({ state: 'visible' }).catch(()=>{}), + markdownEditor.waitFor({ state: 'visible' }).catch(()=>{}), + ]); + if (await richTextBody.isVisible()) { + return richTextBody; + } else { + return markdownEditor; + } + } + + public async expectToHaveText(content: string) { + // expect(...).toHaveText can fail in the Rich Text Editor (perhaps due to frame locators). + // Using expect.poll refreshes the locator on each attempt, which seems to prevent flakiness. + await expect.poll( + async () => (await this.contentLocator()).textContent(), + ).toBe(content); + } + public getNoteViewerFrameLocator() { // The note viewer can change content when the note re-renders. As such, // a new locator needs to be created after re-renders (and this can't be a @@ -38,7 +64,7 @@ export default class NoteEditorPage { return this.noteViewerContainer.frameLocator(':scope'); } - public getTinyMCEFrameLocator() { + public getRichTextFrameLocator() { // We use frameLocator(':scope') to convert the richTextEditor Locator into // a FrameLocator. (:scope selects the locator itself). // https://playwright.dev/docs/api/class-framelocator @@ -53,4 +79,10 @@ export default class NoteEditorPage { await this.noteTitleInput.waitFor(); await this.toggleEditorsButton.waitFor(); } + + public async goBack() { + const backButton = this.toolbarButtonLocator('Back'); + await expect(backButton).not.toBeDisabled(); + await backButton.click(); + } } diff --git a/packages/app-desktop/integration-tests/pluginApi.spec.ts b/packages/app-desktop/integration-tests/pluginApi.spec.ts new file mode 100644 index 000000000..e1ba59e14 --- /dev/null +++ b/packages/app-desktop/integration-tests/pluginApi.spec.ts @@ -0,0 +1,32 @@ + +import { test } from './util/test'; +import MainScreen from './models/MainScreen'; + +test.describe('pluginApi', () => { + for (const richTextEditor of [false, true]) { + test(`the editor.setText command should update the current note (use RTE: ${richTextEditor})`, async ({ startAppWithPlugins }) => { + const { app, mainWindow } = await startAppWithPlugins(['resources/test-plugins/execCommand.js']); + const mainScreen = new MainScreen(mainWindow); + await mainScreen.createNewNote('First note'); + const editor = mainScreen.noteEditor; + + await editor.focusCodeMirrorEditor(); + await mainWindow.keyboard.type('This content should be overwritten.'); + + if (richTextEditor) { + await editor.toggleEditorsButton.click(); + await editor.richTextEditor.click(); + } + + await mainScreen.goToAnything.runCommand(app, 'testUpdateEditorText'); + await editor.expectToHaveText('PASS'); + + // Should still have the same text after switching notes: + await mainScreen.createNewNote('Second note'); + await editor.goBack(); + + await editor.expectToHaveText('PASS'); + }); + } +}); + diff --git a/packages/app-desktop/integration-tests/resources/test-plugins/execCommand.js b/packages/app-desktop/integration-tests/resources/test-plugins/execCommand.js new file mode 100644 index 000000000..27f6699ad --- /dev/null +++ b/packages/app-desktop/integration-tests/resources/test-plugins/execCommand.js @@ -0,0 +1,31 @@ +// Allows referencing the Joplin global: +/* eslint-disable no-undef */ + +// Allows the `joplin-manifest` block comment: +/* eslint-disable multiline-comment-style */ + +/* joplin-manifest: +{ + "id": "org.joplinapp.plugins.example.execCommand", + "manifest_version": 1, + "app_min_version": "3.1", + "name": "JS Bundle test", + "description": "JS Bundle Test plugin", + "version": "1.0.0", + "author": "", + "homepage_url": "https://joplinapp.org" +} +*/ + +joplin.plugins.register({ + onStart: async function() { + await joplin.commands.register({ + name: 'testUpdateEditorText', + label: 'Test setting the editor\'s text with editor.setText', + iconName: 'fas fa-drum', + execute: async () => { + await joplin.commands.execute('editor.setText', 'PASS'); + }, + }); + }, +}); diff --git a/packages/app-desktop/integration-tests/richTextEditor.spec.ts b/packages/app-desktop/integration-tests/richTextEditor.spec.ts index 3b40f88d3..60c60f9f6 100644 --- a/packages/app-desktop/integration-tests/richTextEditor.spec.ts +++ b/packages/app-desktop/integration-tests/richTextEditor.spec.ts @@ -38,7 +38,7 @@ test.describe('richTextEditor', () => { await editor.richTextEditor.waitFor(); // Edit the note to cause the original content to update - await editor.getTinyMCEFrameLocator().locator('a').click(); + await editor.getRichTextFrameLocator().locator('a').click(); await mainWindow.keyboard.type('Test...'); await editor.toggleEditorsButton.click(); @@ -70,7 +70,7 @@ test.describe('richTextEditor', () => { // Click on the attached file URL const openPathResult = waitForNextOpenPath(electronApp); - const targetLink = editor.getTinyMCEFrameLocator().getByRole('link', { name: basename(pathToAttach) }); + const targetLink = editor.getRichTextFrameLocator().getByRole('link', { name: basename(pathToAttach) }); if (process.platform === 'darwin') { await targetLink.click({ modifiers: ['Meta'] }); } else { diff --git a/packages/app-desktop/integration-tests/util/test.ts b/packages/app-desktop/integration-tests/util/test.ts index 0efa8f48d..93729bed5 100644 --- a/packages/app-desktop/integration-tests/util/test.ts +++ b/packages/app-desktop/integration-tests/util/test.ts @@ -6,10 +6,12 @@ import createStartupArgs from './createStartupArgs'; import firstNonDevToolsWindow from './firstNonDevToolsWindow'; +type StartWithPluginsResult = { app: ElectronApplication; mainWindow: Page }; type JoplinFixtures = { profileDirectory: string; electronApp: ElectronApplication; + startAppWithPlugins: (pluginPaths: string[])=> Promise; startupPluginsLoaded: Promise; mainWindow: Page; }; @@ -17,6 +19,20 @@ type JoplinFixtures = { // A custom fixture that loads an electron app. See // https://playwright.dev/docs/test-fixtures +const getAndResizeMainWindow = async (electronApp: ElectronApplication) => { + const mainWindow = await firstNonDevToolsWindow(electronApp); + + // Setting the viewport size helps keep test environments consistent. + await mainWindow.setViewportSize({ + width: 1200, + height: 800, + }); + + return mainWindow; +}; + +const testDir = dirname(__dirname); + export const test = base.extend({ // Playwright fails if we don't use the object destructuring // pattern in the first argument. @@ -25,7 +41,7 @@ export const test = base.extend({ // // eslint-disable-next-line no-empty-pattern profileDirectory: async ({ }, use) => { - const profilePath = resolve(join(dirname(__dirname), 'test-profile')); + const profilePath = resolve(join(testDir, 'test-profile')); const profileSubdir = join(profilePath, uuid.createNano()); await mkdirp(profileSubdir); @@ -44,6 +60,34 @@ export const test = base.extend({ await electronApp.close(); }, + startAppWithPlugins: async ({ profileDirectory }, use) => { + const startupArgs = createStartupArgs(profileDirectory); + let electronApp: ElectronApplication; + + await use(async (pluginPaths: string[]) => { + if (electronApp) { + throw new Error('Electron app already created'); + } + electronApp = await electron.launch({ + args: [ + ...startupArgs, + '--dev-plugins', + pluginPaths.map(path => resolve(testDir, path)).join(','), + ], + }); + + return { + app: electronApp, + mainWindow: await getAndResizeMainWindow(electronApp), + }; + }); + + if (electronApp) { + await electronApp.firstWindow(); + await electronApp.close(); + } + }, + startupPluginsLoaded: async ({ electronApp }, use) => { const startupPluginsLoadedPromise = electronApp.evaluate(({ ipcMain }) => { return new Promise(resolve => { @@ -55,15 +99,7 @@ export const test = base.extend({ }, mainWindow: async ({ electronApp }, use) => { - const mainWindow = await firstNonDevToolsWindow(electronApp); - - // Setting the viewport size helps keep test environments consistent. - await mainWindow.setViewportSize({ - width: 1200, - height: 800, - }); - - await use(mainWindow); + await use(await getAndResizeMainWindow(electronApp)); }, }); diff --git a/packages/app-desktop/plugins/GotoAnything.tsx b/packages/app-desktop/plugins/GotoAnything.tsx index 68bf2a842..fdad2636b 100644 --- a/packages/app-desktop/plugins/GotoAnything.tsx +++ b/packages/app-desktop/plugins/GotoAnything.tsx @@ -543,11 +543,22 @@ class DialogComponent extends React.PureComponent { const resultId = getResultId(item); const isSelected = resultId === this.state.selectedItemId; const rowStyle = isSelected ? style.rowSelected : style.row; + + const wrapKeywordMatches = (unescapedContent: string) => { + return surroundKeywords( + this.state.keywords, + unescapedContent, + ``, + '', + { escapeHtml: true }, + ); + }; + const titleHtml = item.fragments ? `${item.title}` - : surroundKeywords(this.state.keywords, item.title, ``, '', { escapeHtml: true }); + : wrapKeywordMatches(item.title); - const fragmentsHtml = !item.fragments ? null : surroundKeywords(this.state.keywords, item.fragments, ``, '', { escapeHtml: true }); + const fragmentsHtml = !item.fragments ? null : wrapKeywordMatches(item.fragments); const folderIcon = ; const pathComp = !item.path ? null :
{folderIcon} {item.path}
;