Desktop: Resolves #9927: Beta editor: Fix search results not highligh…

…ted (#9928)
laurent22 · Mar 6, 2024 · 20f8bb7 · 20f8bb7
1 parent 5e4c35a
commit 20f8bb7
Show file tree

Hide file tree

Showing 12 changed files with 295 additions and 67 deletions.
diff --git a/.eslintignore b/.eslintignore
@@ -258,7 +258,8 @@ packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/types.js
 packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useContextMenu.js
 packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useCursorUtils.test.js
 packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useCursorUtils.js
-packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useEditorSearch.js
+packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useEditorSearchExtension.js
+packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useEditorSearchHandler.js
 packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useExternalPlugins.js
 packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useJoplinCommands.js
 packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useJoplinMode.js

diff --git a/.gitignore b/.gitignore
@@ -238,7 +238,8 @@ packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/types.js
 packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useContextMenu.js
 packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useCursorUtils.test.js
 packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useCursorUtils.js
-packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useEditorSearch.js
+packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useEditorSearchExtension.js
+packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useEditorSearchHandler.js
 packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useExternalPlugins.js
 packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useJoplinCommands.js
 packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useJoplinMode.js

diff --git a/...eBody/CodeMirror/utils/useEditorSearch.ts → ...eMirror/utils/useEditorSearchExtension.ts b/...eBody/CodeMirror/utils/useEditorSearch.ts → ...eMirror/utils/useEditorSearchExtension.ts
@@ -1,10 +1,14 @@
 import { useEffect, useRef, useState } from 'react';
 import shim from '@joplin/lib/shim';
 import Logger from '@joplin/utils/Logger';
+import CodeMirror5Emulation from '@joplin/editor/CodeMirror/CodeMirror5Emulation/CodeMirror5Emulation';
 
 const logger = Logger.create('useEditorSearch');
 
-export default function useEditorSearch(CodeMirror: any) {
+// Registers a helper CodeMirror extension to be used with
+// useEditorSearchHandler.
+
+export default function useEditorSearchExtension(CodeMirror: CodeMirror5Emulation) {
 
 	const [markers, setMarkers] = useState([]);
 	const [overlay, setOverlay] = useState(null);
@@ -73,15 +77,15 @@ export default function useEditorSearch(CodeMirror: any) {
 				// If we run out of matches then just highlight the final match
 				break;
 			}
-			match = cursor.pos;
+			match = { from: cursor.from(), to: cursor.to() };
 		}
 
 		if (match) {
 			if (scrollTo) {
 				if (withSelection) {
 					cm.setSelection(match.from, match.to);
 				} else {
-					cm.scrollTo(match);
+					cm.scrollIntoView(match);
 				}
 			}
 			return cm.markText(match.from, match.to, { className: 'cm-search-marker-selected' });
@@ -107,7 +111,7 @@ export default function useEditorSearch(CodeMirror: any) {
 		};
 	}, []);
 
-	CodeMirror.defineExtension('setMarkers', function(keywords: any, options: any) {
+	CodeMirror?.defineExtension('setMarkers', function(keywords: any, options: any) {
 		if (!options) {
 			options = { selectedIndex: 0, searchTimestamp: 0 };
 		}
@@ -172,7 +176,7 @@ export default function useEditorSearch(CodeMirror: any) {
 		// These operations are pretty slow, so we won't add use them until the user
 		// has finished typing, 500ms is probably enough time
 		const timeout = shim.setTimeout(() => {
-			const scrollMarks = this.showMatchesOnScrollbar(searchTerm, true, 'cm-search-marker-scrollbar');
+			const scrollMarks = this.showMatchesOnScrollbar?.(searchTerm, true, 'cm-search-marker-scrollbar');
 			const overlay = searchOverlay(searchTerm);
 			this.addOverlay(overlay);
 			setOverlay(overlay);

diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useEditorSearchHandler.ts b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useEditorSearchHandler.ts
@@ -0,0 +1,66 @@
+import { RefObject, useEffect } from 'react';
+import usePrevious from '../../../../hooks/usePrevious';
+import { RenderedBody } from './types';
+const debounce = require('debounce');
+
+interface Props {
+	setLocalSearchResultCount(count: number): void;
+	searchMarkers: any;
+	webviewRef: RefObject<any>;
+	editorRef: RefObject<any>;
+
+	noteContent: string;
+	renderedBody: RenderedBody;
+}
+
+const useEditorSearchHandler = (props: Props) => {
+	const { webviewRef, editorRef, renderedBody, noteContent, searchMarkers } = props;
+
+	const previousContent = usePrevious(noteContent);
+	const previousRenderedBody = usePrevious(renderedBody);
+	const previousSearchMarkers = usePrevious(searchMarkers);
+
+	useEffect(() => {
+		if (!searchMarkers) return () => {};
+
+		// If there is a currently active search, it's important to re-search the text as the user
+		// types. However this is slow for performance so we ONLY want it to happen when there is
+		// a search
+
+		// Note that since the CodeMirror component also needs to handle the viewer pane, we need
+		// to check if the rendered body has changed too (it will be changed with a delay after
+		// props.content has been updated).
+		const textChanged = searchMarkers.keywords.length > 0 && (noteContent !== previousContent || renderedBody !== previousRenderedBody);
+
+		if (webviewRef.current && (searchMarkers !== previousSearchMarkers || textChanged)) {
+			webviewRef.current.send('setMarkers', searchMarkers.keywords, searchMarkers.options);
+
+			if (editorRef.current) {
+				// Fixes https://github.com/laurent22/joplin/issues/7565
+				const debouncedMarkers = debounce(() => {
+					const matches = editorRef.current.setMarkers(searchMarkers.keywords, searchMarkers.options);
+
+					props.setLocalSearchResultCount(matches);
+				}, 50);
+				debouncedMarkers();
+				return () => {
+					debouncedMarkers.clear();
+				};
+			}
+		}
+		return () => {};
+	}, [
+		editorRef,
+		webviewRef,
+		searchMarkers,
+		previousSearchMarkers,
+		props.setLocalSearchResultCount,
+		noteContent,
+		previousContent,
+		previousRenderedBody,
+		renderedBody,
+	]);
+
+};
+
+export default useEditorSearchHandler;
diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/CodeMirror.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/CodeMirror.tsx
@@ -6,7 +6,7 @@ import { EditorCommand, MarkupToHtmlOptions, NoteBodyEditorProps, NoteBodyEditor
 import { commandAttachFileToBody, getResourcesFromPasteEvent } from '../../../utils/resourceHandling';
 import { ScrollOptions, ScrollOptionTypes } from '../../../utils/types';
 import { CommandValue } from '../../../utils/types';
-import { usePrevious, cursorPositionToTextOffset } from '../utils';
+import { cursorPositionToTextOffset } from '../utils';
 import useScrollHandler from '../utils/useScrollHandler';
 import useElementSize from '@joplin/lib/hooks/useElementSize';
 import Toolbar from '../Toolbar';
@@ -25,13 +25,13 @@ import { ThemeAppearance } from '@joplin/lib/themes/type';
 import dialogs from '../../../../dialogs';
 import { MarkupToHtml } from '@joplin/renderer';
 const { clipboard } = require('electron');
-const debounce = require('debounce');
 
 import { reg } from '@joplin/lib/registry';
 import ErrorBoundary from '../../../../ErrorBoundary';
 import useStyles from '../utils/useStyles';
 import useContextMenu from '../utils/useContextMenu';
 import useWebviewIpcMessage from '../utils/useWebviewIpcMessage';
+import useEditorSearchHandler from '../utils/useEditorSearchHandler';
 
 function markupRenderOptions(override: MarkupToHtmlOptions = null): MarkupToHtmlOptions {
 	return { ...override };
@@ -45,10 +45,6 @@ function CodeMirror(props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
 
 	const [webviewReady, setWebviewReady] = useState(false);
 
-	const previousContent = usePrevious(props.content);
-	const previousRenderedBody = usePrevious(renderedBody);
-	const previousSearchMarkers = usePrevious(props.searchMarkers);
-
 	const editorRef = useRef(null);
 	const rootRef = useRef(null);
 	const webviewRef = useRef(null);
@@ -675,37 +671,14 @@ function CodeMirror(props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
 		// eslint-disable-next-line @seiyab/react-hooks/exhaustive-deps -- Old code before rule was applied
 	}, [renderedBody, webviewReady]);
 
-	useEffect(() => {
-		if (!props.searchMarkers) return () => {};
-
-		// If there is a currently active search, it's important to re-search the text as the user
-		// types. However this is slow for performance so we ONLY want it to happen when there is
-		// a search
-
-		// Note that since the CodeMirror component also needs to handle the viewer pane, we need
-		// to check if the rendered body has changed too (it will be changed with a delay after
-		// props.content has been updated).
-		const textChanged = props.searchMarkers.keywords.length > 0 && (props.content !== previousContent || renderedBody !== previousRenderedBody);
-
-		if (webviewRef.current && (props.searchMarkers !== previousSearchMarkers || textChanged)) {
-			webviewRef.current.send('setMarkers', props.searchMarkers.keywords, props.searchMarkers.options);
-
-			if (editorRef.current) {
-				// Fixes https://github.com/laurent22/joplin/issues/7565
-				const debouncedMarkers = debounce(() => {
-					const matches = editorRef.current.setMarkers(props.searchMarkers.keywords, props.searchMarkers.options);
-
-					props.setLocalSearchResultCount(matches);
-				}, 50);
-				debouncedMarkers();
-				return () => {
-					debouncedMarkers.clear();
-				};
-			}
-		}
-		return () => {};
-		// eslint-disable-next-line @seiyab/react-hooks/exhaustive-deps -- Old code before rule was applied
-	}, [props.searchMarkers, previousSearchMarkers, props.setLocalSearchResultCount, props.content, previousContent, renderedBody, previousRenderedBody, renderedBody]);
+	useEditorSearchHandler({
+		setLocalSearchResultCount: props.setLocalSearchResultCount,
+		searchMarkers: props.searchMarkers,
+		webviewRef,
+		editorRef,
+		noteContent: props.content,
+		renderedBody,
+	});
 
 	const cellEditorStyle = useMemo(() => {
 		const output = { ...styles.cellEditor };

diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/Editor.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/Editor.tsx
@@ -16,7 +16,7 @@ import useListIdent from '../utils/useListIdent';
 import useScrollUtils from '../utils/useScrollUtils';
 import useCursorUtils from '../utils/useCursorUtils';
 import useLineSorting from '../utils/useLineSorting';
-import useEditorSearch from '../utils/useEditorSearch';
+import useEditorSearch from '../utils/useEditorSearchExtension';
 import useJoplinMode from '../utils/useJoplinMode';
 import useKeymap from '../utils/useKeymap';
 import useExternalPlugins from '../utils/useExternalPlugins';

diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/CodeMirror.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/CodeMirror.tsx
@@ -26,6 +26,7 @@ import CodeMirrorControl from '@joplin/editor/CodeMirror/CodeMirrorControl';
 import useContextMenu from '../utils/useContextMenu';
 import useWebviewIpcMessage from '../utils/useWebviewIpcMessage';
 import Toolbar from '../Toolbar';
+import useEditorSearchHandler from '../utils/useEditorSearchHandler';
 
 const logger = Logger.create('CodeMirror6');
 const logDebug = (message: string) => logger.debug(message);
@@ -334,6 +335,15 @@ const CodeMirror = (props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
 	// 	}
 	// }, [editorPaneVisible]);
 
+	useEditorSearchHandler({
+		setLocalSearchResultCount: props.setLocalSearchResultCount,
+		searchMarkers: props.searchMarkers,
+		webviewRef,
+		editorRef,
+		noteContent: props.content,
+		renderedBody,
+	});
+
 	useContextMenu({
 		plugins: props.plugins,
 		editorCutText, editorCopyText, editorPaste,

diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/Editor.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/Editor.tsx
@@ -10,6 +10,7 @@ import shim from '@joplin/lib/shim';
 import PluginService from '@joplin/lib/services/plugins/PluginService';
 import setupVim from '@joplin/editor/CodeMirror/util/setupVim';
 import { dirname } from 'path';
+import useEditorSearch from '../utils/useEditorSearchExtension';
 
 interface Props extends EditorProps {
 	style: React.CSSProperties;
@@ -32,6 +33,8 @@ const Editor = (props: Props, ref: ForwardedRef<CodeMirrorControl>) => {
 		onLogMessageRef.current = props.onLogMessage;
 	}, [props.onEvent, props.onLogMessage]);
 
+	useEditorSearch(editor);
+
 	useEffect(() => {
 		if (!editor) {
 			return () => {};
@@ -104,6 +107,26 @@ const Editor = (props: Props, ref: ForwardedRef<CodeMirrorControl>) => {
 	// eslint-disable-next-line @seiyab/react-hooks/exhaustive-deps -- Should run just once
 	}, []);
 
+	const theme = props.settings.themeData;
+	useEffect(() => {
+		if (!editor) return () => {};
+
+		const styles = editor.addStyles({
+			'& .cm-search-marker *, & .cm-search-marker': {
+				color: theme.searchMarkerColor,
+				backgroundColor: theme.searchMarkerBackgroundColor,
+			},
+			'& .cm-search-marker-selected *, & .cm-search-marker-selected': {
+				background: `${theme.selectedColor2} !important`,
+				color: `${theme.color2} !important`,
+			},
+		});
+
+		return () => {
+			styles.remove();
+		};
+	}, [editor, theme]);
+
 	useEffect(() => {
 		editor?.updateSettings(props.settings);
 	}, [props.settings, editor]);

diff --git a/packages/editor/CodeMirror/CodeMirror5Emulation/CodeMirror5Emulation.test.ts b/packages/editor/CodeMirror/CodeMirror5Emulation/CodeMirror5Emulation.test.ts
@@ -109,6 +109,41 @@ describe('CodeMirror5Emulation', () => {
 		expect(onOtherOptionUpdate).toHaveBeenCalledTimes(1);
 	});
 
+	it('markText decorations should be removable', () => {
+		const codeMirror = makeCodeMirrorEmulation('Test 1\nTest 2');
+
+		const markDecoration = codeMirror.markText(
+			{ line: 0, ch: 0 },
+			{ line: 0, ch: 6 },
+			{ className: 'test-mark-decoration' },
+		);
+
+		const markDecoration2 = codeMirror.markText(
+			{ line: 1, ch: 0 },
+			{ line: 1, ch: 1 },
+			{ className: 'test-decoration-2' },
+		);
+
+		const editorDom = codeMirror.cm6.dom;
+		expect(editorDom.querySelectorAll('.test-mark-decoration')).toHaveLength(1);
+		expect(editorDom.querySelectorAll('.test-decoration-2')).toHaveLength(1);
+
+		codeMirror.setCursor(0, 2);
+		codeMirror.replaceSelection('!Test!');
+
+		// Editing the document shouldn't remove the mark
+		expect(codeMirror.editor.state.doc.toString()).toBe('Te!Test!st 1\nTest 2');
+		expect(editorDom.querySelectorAll('.test-mark-decoration')).toHaveLength(1);
+
+		// Clearing should remove only the decoration that was cleared.
+		markDecoration.clear();
+		expect(editorDom.querySelectorAll('.test-mark-decoration')).toHaveLength(0);
+		expect(editorDom.querySelectorAll('.test-decoration-2')).toHaveLength(1);
+
+		markDecoration2.clear();
+		expect(editorDom.querySelectorAll('.test-decoration-2')).toHaveLength(0);
+	});
+
 	it('defineExtension should override previous extensions with the same name', () => {
 		const codeMirror = makeCodeMirrorEmulation('Test...');
 		const testExtensionFn1 = jest.fn();