From aebfa6e96d3efd29892dd20b6104bf1aa5453fd4 Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Wed, 26 Jul 2023 10:07:00 -0700 Subject: [PATCH] Desktop: Fixes #8535: Fix CodeMirror context menu not containing correct items (#8543) --- packages/app-desktop/bridge.ts | 6 +-- .../NoteBody/CodeMirror/CodeMirror.tsx | 50 ++++++++++++++++--- .../services/electron-context-menu.js | 5 ++ 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/packages/app-desktop/bridge.ts b/packages/app-desktop/bridge.ts index 8f44ba1ca9..4c0a2adf7a 100644 --- a/packages/app-desktop/bridge.ts +++ b/packages/app-desktop/bridge.ts @@ -96,11 +96,7 @@ export class Bridge { electronApp: this.electronApp(), shouldShowMenu: (_event: any, params: any) => { - // params.inputFieldType === 'none' when right-clicking the text - // editor. This is a bit of a hack to detect it because in this - // case we don't want to use the built-in context menu but a - // custom one. - return params.isEditable && params.inputFieldType !== 'none'; + return params.isEditable; }, // menu: (actions: any, props: any) => { diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/CodeMirror.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/CodeMirror.tsx index f9c4b2d669..21b58c0cc1 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/CodeMirror.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/CodeMirror.tsx @@ -40,6 +40,7 @@ import ErrorBoundary from '../../../ErrorBoundary'; import { MarkupToHtmlOptions } from '../../utils/useMarkupToHtml'; import eventManager from '@joplin/lib/eventManager'; import { EditContextMenuFilterObject } from '@joplin/lib/services/plugins/api/JoplinWorkspace'; +import type { ContextMenuEvent, ContextMenuParams } from 'electron'; const menuUtils = new MenuUtils(CommandService.instance()); @@ -782,20 +783,50 @@ function CodeMirror(props: NoteBodyEditorProps, ref: any) { // It might be buggy, refer to the below issue // https://github.com/laurent22/joplin/pull/3974#issuecomment-718936703 useEffect(() => { - function pointerInsideEditor(params: any) { - const x = params.x, y = params.y, isEditable = params.isEditable, inputFieldType = params.inputFieldType; + const isAncestorOfCodeMirrorEditor = (elem: HTMLElement) => { + for (; elem.parentElement; elem = elem.parentElement) { + if (elem.classList.contains('codeMirrorEditor')) { + return true; + } + } + + return false; + }; + + let lastInCodeMirrorContextMenuTimestamp = 0; + + // The browser's contextmenu event provides additional information about the + // target of the event, not provided by the Electron context-menu event. + const onBrowserContextMenu = (event: Event) => { + if (isAncestorOfCodeMirrorEditor(event.target as HTMLElement)) { + lastInCodeMirrorContextMenuTimestamp = Date.now(); + } + }; + + function pointerInsideEditor(params: ContextMenuParams) { + const x = params.x, y = params.y, isEditable = params.isEditable; const elements = document.getElementsByClassName('codeMirrorEditor'); - // inputFieldType: The input field type of CodeMirror is "textarea" so the inputFieldType = "none", - // and any single-line input above codeMirror has inputFieldType value according to the type of input e.g.(text = plainText, password = password, ...). - if (!elements.length || !isEditable || inputFieldType !== 'none') return null; + // Note: We can't check inputFieldType here. When spellcheck is enabled, + // params.inputFieldType is "none". When spellcheck is disabled, + // params.inputFieldType is "plainText". Thus, such a check would be inconsistent. + if (!elements.length || !isEditable) return false; + + const maximumMsSinceBrowserEvent = 100; + if (Date.now() - lastInCodeMirrorContextMenuTimestamp > maximumMsSinceBrowserEvent) { + return false; + } + const rect = convertToScreenCoordinates(Setting.value('windowContentZoomFactor'), elements[0].getBoundingClientRect()); return rect.x < x && rect.y < y && rect.right > x && rect.bottom > y; } - async function onContextMenu(_event: any, params: any) { + async function onContextMenu(event: ContextMenuEvent, params: ContextMenuParams) { if (!pointerInsideEditor(params)) return; + // Don't show the default menu. + event.preventDefault(); + const menu = new Menu(); const hasSelectedText = editorRef.current && !!editorRef.current.getSelection() ; @@ -872,10 +903,15 @@ function CodeMirror(props: NoteBodyEditorProps, ref: any) { menu.popup(); } - bridge().window().webContents.on('context-menu', onContextMenu); + // Prepend the event listener so that it gets called before + // the listener that shows the default menu. + bridge().window().webContents.prependListener('context-menu', onContextMenu); + + window.addEventListener('contextmenu', onBrowserContextMenu); return () => { bridge().window().webContents.off('context-menu', onContextMenu); + window.removeEventListener('contextmenu', onBrowserContextMenu); }; // eslint-disable-next-line @seiyab/react-hooks/exhaustive-deps -- Old code before rule was applied }, [props.plugins]); diff --git a/packages/app-desktop/services/electron-context-menu.js b/packages/app-desktop/services/electron-context-menu.js index 20015a511d..61a1e4fff6 100644 --- a/packages/app-desktop/services/electron-context-menu.js +++ b/packages/app-desktop/services/electron-context-menu.js @@ -42,6 +42,11 @@ const create = (win, options) => { return; } + // If another listener has called .preventDefault, don't show the default context menu. + if (event.defaultPrevented) { + return; + } + const { editFlags } = props; const hasText = props.selectionText.trim().length > 0; const isLink = Boolean(props.linkURL);