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: Allow searching when only the note viewer is visible and sync search with editor #10866

Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -841,9 +841,11 @@ packages/editor/CodeMirror/utils/formatting/toggleRegionFormatGlobally.test.js
packages/editor/CodeMirror/utils/formatting/toggleRegionFormatGlobally.js
packages/editor/CodeMirror/utils/formatting/toggleSelectedLinesStartWith.js
packages/editor/CodeMirror/utils/formatting/types.js
packages/editor/CodeMirror/utils/getSearchState.js
packages/editor/CodeMirror/utils/growSelectionToNode.js
packages/editor/CodeMirror/utils/handlePasteEvent.js
packages/editor/CodeMirror/utils/isInSyntaxNode.js
packages/editor/CodeMirror/utils/searchExtension.js
packages/editor/CodeMirror/utils/setupVim.js
packages/editor/SelectionFormatting.js
packages/editor/events.js
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -818,9 +818,11 @@ packages/editor/CodeMirror/utils/formatting/toggleRegionFormatGlobally.test.js
packages/editor/CodeMirror/utils/formatting/toggleRegionFormatGlobally.js
packages/editor/CodeMirror/utils/formatting/toggleSelectedLinesStartWith.js
packages/editor/CodeMirror/utils/formatting/types.js
packages/editor/CodeMirror/utils/getSearchState.js
packages/editor/CodeMirror/utils/growSelectionToNode.js
packages/editor/CodeMirror/utils/handlePasteEvent.js
packages/editor/CodeMirror/utils/isInSyntaxNode.js
packages/editor/CodeMirror/utils/searchExtension.js
packages/editor/CodeMirror/utils/setupVim.js
packages/editor/SelectionFormatting.js
packages/editor/events.js
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useRef, useState } from 'react';
import { useCallback, 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';
Expand All @@ -20,16 +20,15 @@ export default function useEditorSearchExtension(CodeMirror: CodeMirror5Emulatio
const overlayTimeoutRef = useRef(null);
overlayTimeoutRef.current = overlayTimeout;

function clearMarkers() {
const clearMarkers = useCallback(() => {
for (let i = 0; i < markers.length; i++) {
markers[i].clear();
}

setMarkers([]);
}
}, [markers]);

// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
function clearOverlay(cm: any) {
const clearOverlay = useCallback((cm: CodeMirror5Emulation) => {
if (overlay) cm.removeOverlay(overlay);
if (scrollbarMarks) {
try {
Expand All @@ -47,10 +46,10 @@ export default function useEditorSearchExtension(CodeMirror: CodeMirror5Emulatio
setOverlay(null);
setScrollbarMarks(null);
setOverlayTimeout(null);
}
}, [scrollbarMarks, overlay, overlayTimeout]);

// Modified from codemirror/addons/search/search.js
function searchOverlay(query: RegExp) {
const searchOverlay = useCallback((query: RegExp) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
return { token: function(stream: any) {
query.lastIndex = stream.pos;
Expand All @@ -65,13 +64,13 @@ export default function useEditorSearchExtension(CodeMirror: CodeMirror5Emulatio
}
return null;
} };
}
}, []);

// Highlights the currently active found work
// It's possible to get tricky with this functions and just use findNext/findPrev
// but this is fast enough and works more naturally with the current search logic
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
function highlightSearch(cm: any, searchTerm: RegExp, index: number, scrollTo: boolean, withSelection: boolean) {
function highlightSearch(cm: CodeMirror5Emulation, searchTerm: RegExp, index: number, scrollTo: boolean, withSelection: boolean) {
const cursor = cm.getSearchCursor(searchTerm);

// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
Expand Down Expand Up @@ -122,6 +121,12 @@ export default function useEditorSearchExtension(CodeMirror: CodeMirror5Emulatio
options = { selectedIndex: 0, searchTimestamp: 0 };
}

if (options.showEditorMarkers === false) {
clearMarkers();
clearOverlay(this);
return;
}

clearMarkers();

// HIGHLIGHT KEYWORDS
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { RefObject, useEffect } from 'react';
import { RefObject, useEffect, useMemo, useRef } from 'react';
import usePrevious from '../../../../hooks/usePrevious';
import { RenderedBody } from './types';
import { SearchMarkers } from '../../../utils/useSearchMarkers';
const debounce = require('debounce');

interface Props {
Expand All @@ -14,14 +15,31 @@ interface Props {

noteContent: string;
renderedBody: RenderedBody;
showEditorMarkers: boolean;
}

const useEditorSearchHandler = (props: Props) => {
const { webviewRef, editorRef, renderedBody, noteContent, searchMarkers } = props;
const {
webviewRef, editorRef, renderedBody, noteContent, searchMarkers, showEditorMarkers,
} = props;

const previousContent = usePrevious(noteContent);
const previousRenderedBody = usePrevious(renderedBody);
const previousSearchMarkers = usePrevious(searchMarkers);
const showEditorMarkersRef = useRef(showEditorMarkers);
showEditorMarkersRef.current = showEditorMarkers;

// Fixes https://github.com/laurent22/joplin/issues/7565
const debouncedMarkers = useMemo(() => debounce((searchMarkers: SearchMarkers) => {
if (!editorRef.current) return;

if (showEditorMarkersRef.current) {
const matches = editorRef.current.setMarkers(searchMarkers.keywords, searchMarkers.options);
props.setLocalSearchResultCount(matches);
} else {
editorRef.current.setMarkers(searchMarkers.keywords, { ...searchMarkers.options, showEditorMarkers: false });
}
}, 50), [editorRef, props.setLocalSearchResultCount]);

useEffect(() => {
if (!searchMarkers) return () => {};
Expand All @@ -37,19 +55,7 @@ const useEditorSearchHandler = (props: Props) => {

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();
};
}
debouncedMarkers(searchMarkers);
}
return () => {};
}, [
Expand All @@ -62,6 +68,7 @@ const useEditorSearchHandler = (props: Props) => {
previousContent,
previousRenderedBody,
renderedBody,
debouncedMarkers,
]);

};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@ function CodeMirror(props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
editorRef,
noteContent: props.content,
renderedBody,
showEditorMarkers: true,
});

const cellEditorStyle = useMemo(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { MarkupToHtml } from '@joplin/renderer';
const { clipboard } = require('electron');
import { reg } from '@joplin/lib/registry';
import ErrorBoundary from '../../../../ErrorBoundary';
import { EditorKeymap, EditorLanguageType, EditorSettings, UserEventSource } from '@joplin/editor/types';
import { EditorKeymap, EditorLanguageType, EditorSettings, SearchState, UserEventSource } from '@joplin/editor/types';
import useStyles from '../utils/useStyles';
import { EditorEvent, EditorEventType } from '@joplin/editor/events';
import useScrollHandler from '../utils/useScrollHandler';
Expand Down Expand Up @@ -175,6 +175,9 @@ const CodeMirror = (props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
}
},
supportsCommand: (name: string) => {
if (name === 'search' && !props.visiblePanes.includes('editor')) {
return false;
}
return name in commands || editorRef.current.supportsCommand(name);
},
execCommand: async (cmd: EditorCommand) => {
Expand All @@ -197,7 +200,7 @@ const CodeMirror = (props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
return commandOutput;
},
};
}, [props.content, commands, resetScroll, setEditorPercentScroll, setViewerPercentScroll]);
}, [props.content, props.visiblePanes, commands, resetScroll, setEditorPercentScroll, setViewerPercentScroll]);

const webview_domReady = useCallback(() => {
setWebviewReady(true);
Expand Down Expand Up @@ -321,6 +324,7 @@ const CodeMirror = (props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
editorRef,
noteContent: props.content,
renderedBody,
showEditorMarkers: !props.useLocalSearch,
});

useContextMenu({
Expand All @@ -330,15 +334,25 @@ const CodeMirror = (props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
editorClassName: 'cm-editor',
});

const lastSearchState = useRef<SearchState|null>(null);
const onEditorEvent = useCallback((event: EditorEvent) => {
if (event.kind === EditorEventType.Scroll) {
editor_scroll();
} else if (event.kind === EditorEventType.Change) {
codeMirror_change(event.value);
} else if (event.kind === EditorEventType.SelectionRangeChange) {
setSelectionRange({ from: event.from, to: event.to });
} else if (event.kind === EditorEventType.UpdateSearchDialog) {
if (lastSearchState.current?.searchText !== event.searchState.searchText) {
props.setLocalSearch(event.searchState.searchText);
}

if (lastSearchState.current?.dialogVisible !== event.searchState.dialogVisible) {
props.setShowLocalSearch(event.searchState.dialogVisible);
}
lastSearchState.current = event.searchState;
}
}, [editor_scroll, codeMirror_change]);
}, [editor_scroll, codeMirror_change, props.setLocalSearch, props.setShowLocalSearch]);

const editorSettings = useMemo((): EditorSettings => {
const isHTMLNote = props.contentMarkupLanguage === MarkupToHtml.MARKUP_LANGUAGE_HTML;
Expand Down Expand Up @@ -389,6 +403,8 @@ const CodeMirror = (props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
onEvent={onEditorEvent}
onLogMessage={logDebug}
onEditorPaste={onEditorPaste}
externalSearch={props.searchMarkers}
useLocalSearch={props.useLocalSearch}
/>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@ import { dirname } from 'path';
import useKeymap from './utils/useKeymap';
import useEditorSearch from '../utils/useEditorSearchExtension';
import CommandService from '@joplin/lib/services/CommandService';
import { SearchMarkers } from '../../../utils/useSearchMarkers';

interface Props extends EditorProps {
style: React.CSSProperties;
pluginStates: PluginStates;

onEditorPaste: (event: Event)=> void;
externalSearch: SearchMarkers;
useLocalSearch: boolean;
}

const Editor = (props: Props, ref: ForwardedRef<CodeMirrorControl>) => {
Expand Down Expand Up @@ -117,6 +120,25 @@ const Editor = (props: Props, ref: ForwardedRef<CodeMirrorControl>) => {
// eslint-disable-next-line @seiyab/react-hooks/exhaustive-deps -- Should run just once
}, []);

useEffect(() => {
if (!editor) {
return;
}

const searchState = editor.getSearchState();
const externalSearchText = props.externalSearch.keywords.map(k => k.value).join(' ') || searchState.searchText;

if (externalSearchText === searchState.searchText && searchState.dialogVisible === props.useLocalSearch) {
return;
}

editor.setSearchState({
...searchState,
dialogVisible: props.useLocalSearch,
searchText: externalSearchText,
});
}, [editor, props.externalSearch, props.useLocalSearch]);

const theme = props.settings.themeData;
useEffect(() => {
if (!editor) return () => {};
Expand Down
4 changes: 4 additions & 0 deletions packages/app-desktop/gui/NoteEditor/NoteEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,9 @@ function NoteEditor(props: NoteEditorProps) {
noteToolbar: null,
onScroll: onScroll,
setLocalSearchResultCount: setLocalSearchResultCount,
setLocalSearch: localSearch_change,
setShowLocalSearch,
useLocalSearch: showLocalSearch,
searchMarkers: searchMarkers,
visiblePanes: props.noteVisiblePanes || ['editor', 'viewer'],
keyboardMode: Setting.value('editor.keyboardMode'),
Expand Down Expand Up @@ -506,6 +509,7 @@ function NoteEditor(props: NoteEditorProps) {
onPrevious={localSearch_previous}
onClose={localSearch_close}
visiblePanes={props.noteVisiblePanes}
editorType={props.bodyEditor}
/>
);
}
Expand Down
6 changes: 5 additions & 1 deletion packages/app-desktop/gui/NoteEditor/utils/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { RenderResult, RenderResultPluginAsset } from '@joplin/renderer/types';
import { Dispatch } from 'redux';
import { ProcessResultsRow } from '@joplin/lib/services/search/SearchEngine';
import { DropHandler } from './useDropHandler';
import { SearchMarkers } from './useSearchMarkers';

export interface AllAssetsOptions {
contentMaxWidthTarget?: string;
Expand Down Expand Up @@ -119,8 +120,11 @@ export interface NoteBodyEditorProps {
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
noteToolbar: any;
setLocalSearchResultCount(count: number): void;
setLocalSearch(search: string): void;
setShowLocalSearch(show: boolean): void;
useLocalSearch: boolean;
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
searchMarkers: any;
searchMarkers: SearchMarkers;
visiblePanes: string[];
keyboardMode: string;
resourceInfos: ResourceInfos;
Expand Down
17 changes: 11 additions & 6 deletions packages/app-desktop/gui/NoteSearchBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ interface Props {
resultCount: number;
selectedIndex: number;
visiblePanes: string[];
editorType: string;
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
style: any;
}
Expand All @@ -26,6 +27,7 @@ class NoteSearchBar extends React.Component<Props> {

// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
private backgroundColor: any;
private searchInputRef: React.RefObject<HTMLInputElement>;

public constructor(props: Props) {
super(props);
Expand All @@ -40,6 +42,7 @@ class NoteSearchBar extends React.Component<Props> {
this.focus = this.focus.bind(this);

this.backgroundColor = undefined;
this.searchInputRef = React.createRef();
}

public style() {
Expand Down Expand Up @@ -133,10 +136,8 @@ class NoteSearchBar extends React.Component<Props> {
}

public focus() {
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
focus('NoteSearchBar::focus', this.refs.searchInput as any);
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
(this.refs.searchInput as any).select();
focus('NoteSearchBar::focus', this.searchInputRef.current);
this.searchInputRef.current?.select();
}

public render() {
Expand Down Expand Up @@ -173,14 +174,18 @@ class NoteSearchBar extends React.Component<Props> {
</div>
) : null;

const allowScrolling = this.props.visiblePanes.indexOf('editor') >= 0;
const editorVisible = this.props.visiblePanes.includes('editor');
const usesEditorSearch = this.props.editorType === 'CodeMirror6' && editorVisible;
const allowScrolling = editorVisible;

const viewerWarning = (
<div style={textStyle}>
{'Jumping between matches is not available in the viewer, please toggle the editor'}
</div>
);

if (usesEditorSearch) return null;

return (
<div className="note-search-bar" style={this.props.style}>
<div style={{ display: 'flex', flexDirection: 'row', alignItems: 'center' }}>
Expand All @@ -190,7 +195,7 @@ class NoteSearchBar extends React.Component<Props> {
value={query}
onChange={this.searchInput_change}
onKeyDown={this.searchInput_keyDown}
ref="searchInput"
ref={this.searchInputRef}
type="text"
style={{ width: 200, marginRight: 5, backgroundColor: this.backgroundColor, color: theme.color }}
/>
Expand Down
Loading
Loading