Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Desktop: Improve beta editor support for the Rich Markdown plugin #9935

Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,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
Expand Down Expand Up @@ -574,6 +575,7 @@ packages/default-plugins/utils/getCurrentCommitHash.js
packages/default-plugins/utils/getPathToPatchFileFor.js
packages/default-plugins/utils/readRepositoryJson.js
packages/default-plugins/utils/waitForCliInput.js
packages/editor/CodeMirror/CodeMirror5Emulation/CodeMirror5BuiltInOptions.js
packages/editor/CodeMirror/CodeMirror5Emulation/CodeMirror5Emulation.test.js
packages/editor/CodeMirror/CodeMirror5Emulation/CodeMirror5Emulation.js
packages/editor/CodeMirror/CodeMirror5Emulation/Decorator.js
Expand Down
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,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
Expand Down Expand Up @@ -554,6 +555,7 @@ packages/default-plugins/utils/getCurrentCommitHash.js
packages/default-plugins/utils/getPathToPatchFileFor.js
packages/default-plugins/utils/readRepositoryJson.js
packages/default-plugins/utils/waitForCliInput.js
packages/editor/CodeMirror/CodeMirror5Emulation/CodeMirror5BuiltInOptions.js
packages/editor/CodeMirror/CodeMirror5Emulation/CodeMirror5Emulation.test.js
packages/editor/CodeMirror/CodeMirror5Emulation/CodeMirror5Emulation.js
packages/editor/CodeMirror/CodeMirror5Emulation/Decorator.js
Expand Down
4 changes: 3 additions & 1 deletion packages/app-desktop/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,10 @@ class Application extends BaseApplication {

// The '*' and '!important' parts are necessary to make sure Russian text is displayed properly
// https://github.com/laurent22/joplin/issues/155
//
// Note: Be careful about the specifity here. Incorrect specificity can break monospaced fonts in tables.

const css = `.CodeMirror *, .cm-editor .cm-content { font-family: ${fontFamilies.join(', ')} !important; }`;
const css = `.CodeMirror5 *, .cm-editor .cm-content { font-family: ${fontFamilies.join(', ')} !important; }`;
const styleTag = document.createElement('style');
styleTag.type = 'text/css';
styleTag.appendChild(document.createTextNode(css));
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -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' });
Expand All @@ -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 };
}
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 };
Expand All @@ -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);
Expand Down Expand Up @@ -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 };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -283,7 +283,7 @@ function Editor(props: EditorProps, ref: any) {
}
}, [pluginOptions, editor]);

return <div className='codeMirrorEditor' style={props.style} ref={editorParent} />;
return <div className='codeMirrorEditor CodeMirror5' style={props.style} ref={editorParent} />;
}

export default forwardRef(Editor);
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -338,6 +339,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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 () => {};
Expand Down Expand Up @@ -95,6 +98,12 @@ const Editor = (props: Props, ref: ForwardedRef<CodeMirrorControl>) => {
const editor = createEditor(editorContainerRef.current, editorProps);
editor.addStyles({
'.cm-scroller': { overflow: 'auto' },
'&.CodeMirror': {
height: 'unset',
background: 'unset',
overflow: 'unset',
direction: 'unset',
},
Comment on lines +101 to +106
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the .CodeMirror class to the editor makes it easier to migrate styles originally intended for CodeMirror 5. It does, however, mean that several CodeMirror 5 styles need to be overridden.

});
setEditor(editor);

Expand All @@ -104,6 +113,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]);
Expand Down
Loading
Loading