-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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: Resolves #9927: Beta editor: Fix search results not highlighted #9928
Changes from all commits
509da44
b36ff9e
e162f03
0556a70
a974c35
061dd75
6d70a48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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); | ||
|
@@ -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'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At present, |
||
const overlay = searchOverlay(searchTerm); | ||
this.addOverlay(overlay); | ||
setOverlay(overlay); | ||
|
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) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the CodeMirror documentation,
.scrollTo
takes two arguments, whilescrollIntoView
can take an object with the same type asmatch
.As such, I suspect we were calling
scrollTo
incorrectly.