-
Notifications
You must be signed in to change notification settings - Fork 4
[ENG-1224 & ENG-343] Replaced fuzzy with minisearch in Discourse Summoning Menu #694
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
base: main
Are you sure you want to change the base?
Changes from all commits
4ce3a80
bf327a8
0def4e0
61cc9bd
3084113
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,3 +1,4 @@ | ||
| /* eslint-disable @typescript-eslint/no-redundant-type-constituents */ | ||
|
Contributor
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. Why do we need this? |
||
| import React, { | ||
| useCallback, | ||
| useEffect, | ||
|
|
@@ -25,7 +26,7 @@ import getDiscourseNodes, { DiscourseNode } from "~/utils/getDiscourseNodes"; | |
| import getDiscourseNodeFormatExpression from "~/utils/getDiscourseNodeFormatExpression"; | ||
| import { Result } from "~/utils/types"; | ||
| import { getSetting } from "~/utils/extensionSettings"; | ||
| import fuzzy from "fuzzy"; | ||
| import MiniSearch from "minisearch"; | ||
|
|
||
| type Props = { | ||
| textarea: HTMLTextAreaElement; | ||
|
|
@@ -34,6 +35,13 @@ type Props = { | |
| triggerText: string; | ||
| }; | ||
|
|
||
| type MinisearchResult = Result & { | ||
| type: string; | ||
| }; | ||
|
|
||
| const MIN_SEARCH_SCORE = 0.1; | ||
| const MAX_ITEMS_PER_TYPE = 10; | ||
|
|
||
| const waitForBlock = ({ | ||
| uid, | ||
| text, | ||
|
|
@@ -76,7 +84,6 @@ const NodeSearchMenu = ({ | |
| const [discourseTypes, setDiscourseTypes] = useState<DiscourseNode[]>([]); | ||
| const [checkedTypes, setCheckedTypes] = useState<Record<string, boolean>>({}); | ||
| const [isLoading, setIsLoading] = useState(true); | ||
| const [allNodes, setAllNodes] = useState<Record<string, Result[]>>({}); | ||
| const [searchResults, setSearchResults] = useState<Record<string, Result[]>>( | ||
| {}, | ||
| ); | ||
|
|
@@ -91,6 +98,7 @@ const NodeSearchMenu = ({ | |
| ); | ||
| const scrollContainerRef = useRef<HTMLDivElement | null>(null); | ||
| const searchTimeoutRef = useRef<NodeJS.Timeout | null>(null); | ||
| const miniSearchRef = useRef<MiniSearch<MinisearchResult> | null>(null); | ||
| const POPOVER_TOP_OFFSET = 30; | ||
|
|
||
| const debouncedSearchTerm = useCallback((term: string) => { | ||
|
|
@@ -135,16 +143,78 @@ const NodeSearchMenu = ({ | |
| } | ||
| }; | ||
|
|
||
| const filterNodesLocally = useCallback( | ||
| (nodes: Result[], searchTerm: string): Result[] => { | ||
| if (!searchTerm.trim()) return nodes; | ||
| const searchWithMiniSearch = useCallback( | ||
| (searchTerm: string, typeFilter?: string[]): Record<string, Result[]> => { | ||
| if (!miniSearchRef.current) { | ||
| return {}; | ||
| } | ||
|
|
||
| return fuzzy | ||
| .filter(searchTerm, nodes, { | ||
| extract: (node) => node.text, | ||
| }) | ||
| .map((result) => result.original) | ||
| .filter((node): node is Result => !!node); | ||
| const search = miniSearchRef.current; | ||
|
|
||
| if (!searchTerm.trim()) { | ||
| if (!typeFilter) { | ||
| return {}; | ||
| } | ||
|
|
||
| const allResults: Record<string, Result[]> = {}; | ||
| typeFilter.forEach((type) => { | ||
| const results = ( | ||
| search.search(MiniSearch.wildcard, { | ||
| filter: (result) => | ||
| (result as unknown as MinisearchResult).type === type, | ||
| }) as unknown as MinisearchResult[] | ||
| ).slice(0, MAX_ITEMS_PER_TYPE).map((r) => ({ | ||
|
Contributor
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. format with prettier |
||
| text: r.text, | ||
| uid: r.uid, | ||
| })); | ||
| allResults[type] = results; | ||
| }); | ||
|
|
||
| return allResults; | ||
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
|
Contributor
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. Why do we need this? |
||
| const rawSearchResults = search.search(searchTerm, { | ||
| fields: ["text"], | ||
| fuzzy: 0.2, | ||
| prefix: true, | ||
| combineWith: "AND", | ||
| filter: typeFilter | ||
| ? (result) => | ||
| typeFilter.includes((result as unknown as MinisearchResult).type) | ||
| : undefined, | ||
| }); | ||
|
|
||
| const filteredResults = rawSearchResults.filter( | ||
| (r) => r.score > MIN_SEARCH_SCORE, | ||
| ); | ||
|
|
||
| const searchResults = ( | ||
| filteredResults as unknown as MinisearchResult[] | ||
| ).map((r) => ({ | ||
| text: r.text, | ||
| uid: r.uid, | ||
| type: r.type, | ||
| })); | ||
|
|
||
| const results = searchResults.reduce( | ||
| (acc, result) => { | ||
| if (!acc[result.type]) { | ||
| acc[result.type] = []; | ||
| } | ||
| if (acc[result.type].length < MAX_ITEMS_PER_TYPE) { | ||
|
Collaborator
Author
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. |
||
| acc[result.type].push({ | ||
| id: result.uid, | ||
| text: result.text, | ||
| uid: result.uid, | ||
| }); | ||
| } | ||
| return acc; | ||
| }, | ||
| {} as Record<string, Result[]>, | ||
| ); | ||
|
|
||
| return results; | ||
| }, | ||
| [], | ||
| ); | ||
|
|
@@ -169,7 +239,30 @@ const NodeSearchMenu = ({ | |
| allNodeTypes.forEach((type) => { | ||
| allNodesCache[type.type] = searchNodesForType(type); | ||
| }); | ||
| setAllNodes(allNodesCache); | ||
|
|
||
| const miniSearch = new MiniSearch<MinisearchResult>({ | ||
| fields: ["text"], | ||
| storeFields: ["text", "uid", "type"], | ||
| idField: "uid", | ||
| }); | ||
|
|
||
| const documentsToIndex: MinisearchResult[] = []; | ||
| const seenUids = new Set<string>(); | ||
|
|
||
| allNodeTypes.forEach((type) => { | ||
| const nodes = allNodesCache[type.type] || []; | ||
| nodes.forEach((node) => { | ||
| if (seenUids.has(node.uid)) return; | ||
| seenUids.add(node.uid); | ||
| documentsToIndex.push({ | ||
| ...node, | ||
| type: type.type, | ||
| }); | ||
| }); | ||
| }); | ||
trangdoan982 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| miniSearch.addAll(documentsToIndex); | ||
| miniSearchRef.current = miniSearch; | ||
|
|
||
| const initialSearchResults = Object.fromEntries( | ||
| allNodeTypes.map((type) => [type.type, []]), | ||
|
|
@@ -183,17 +276,21 @@ const NodeSearchMenu = ({ | |
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| if (isLoading || Object.keys(allNodes).length === 0) return; | ||
| if (isLoading || !miniSearchRef.current) return; | ||
|
|
||
| const newResults: Record<string, Result[]> = {}; | ||
|
|
||
| discourseTypes.forEach((type) => { | ||
| const cachedNodes = allNodes[type.type] || []; | ||
| newResults[type.type] = filterNodesLocally(cachedNodes, searchTerm); | ||
| }); | ||
| const selectedTypes = discourseTypes | ||
| .filter((type) => checkedTypes[type.type]) | ||
| .map((type) => type.type); | ||
|
|
||
| const newResults = searchWithMiniSearch(searchTerm, selectedTypes); | ||
| setSearchResults(newResults); | ||
| }, [searchTerm, isLoading, allNodes, discourseTypes, filterNodesLocally]); | ||
| }, [ | ||
| searchTerm, | ||
| isLoading, | ||
| discourseTypes, | ||
| checkedTypes, | ||
| searchWithMiniSearch, | ||
| ]); | ||
|
|
||
| const menuRef = useRef<HTMLUListElement>(null); | ||
| const { ["block-uid"]: blockUid, ["window-id"]: windowId } = useMemo( | ||
|
|
@@ -232,61 +329,61 @@ const NodeSearchMenu = ({ | |
|
|
||
| const onSelect = useCallback( | ||
| (item: Result) => { | ||
| if (!blockUid) { | ||
| onClose(); | ||
| return; | ||
| } | ||
| void waitForBlock({ uid: blockUid, text: textarea.value }) | ||
| .then(() => { | ||
| onClose(); | ||
|
|
||
| setTimeout(() => { | ||
| const originalText = getTextByBlockUid(blockUid); | ||
|
|
||
| const prefix = originalText.substring(0, triggerPosition); | ||
| const suffix = originalText.substring(textarea.selectionStart); | ||
| const pageRef = `[[${item.text}]]`; | ||
|
|
||
| const newText = `${prefix}${pageRef}${suffix}`; | ||
| void updateBlock({ uid: blockUid, text: newText }).then(() => { | ||
| const newCursorPosition = triggerPosition + pageRef.length; | ||
|
|
||
| if (window.roamAlphaAPI.ui.setBlockFocusAndSelection) { | ||
| void window.roamAlphaAPI.ui.setBlockFocusAndSelection({ | ||
| location: { | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| "block-uid": blockUid, | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| "window-id": windowId, | ||
| }, | ||
| selection: { start: newCursorPosition }, | ||
| }); | ||
| } else { | ||
| setTimeout(() => { | ||
| const textareaElements = | ||
| document.querySelectorAll("textarea"); | ||
| for (const el of textareaElements) { | ||
| if (getUids(el).blockUid === blockUid) { | ||
| el.focus(); | ||
| el.setSelectionRange( | ||
| newCursorPosition, | ||
| newCursorPosition, | ||
| ); | ||
| break; | ||
| } | ||
| } | ||
| }, 50); | ||
| } | ||
| }); | ||
| posthog.capture("Discourse Node: Selected from Search Menu", { | ||
| id: item.id, | ||
| text: item.text, | ||
| }); | ||
| }, 10); | ||
| }) | ||
| .catch((error) => { | ||
| console.error("Error waiting for block:", error); | ||
| }); | ||
| if (!blockUid) { | ||
|
Contributor
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. is this block just formatting changes? |
||
| onClose(); | ||
| return; | ||
| } | ||
| void waitForBlock({ uid: blockUid, text: textarea.value }) | ||
| .then(() => { | ||
| onClose(); | ||
|
|
||
| setTimeout(() => { | ||
| const originalText = getTextByBlockUid(blockUid); | ||
|
|
||
| const prefix = originalText.substring(0, triggerPosition); | ||
| const suffix = originalText.substring(textarea.selectionStart); | ||
| const pageRef = `[[${item.text}]]`; | ||
|
|
||
| const newText = `${prefix}${pageRef}${suffix}`; | ||
| void updateBlock({ uid: blockUid, text: newText }).then(() => { | ||
| const newCursorPosition = triggerPosition + pageRef.length; | ||
|
|
||
| if (window.roamAlphaAPI.ui.setBlockFocusAndSelection) { | ||
| void window.roamAlphaAPI.ui.setBlockFocusAndSelection({ | ||
| location: { | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| "block-uid": blockUid, | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| "window-id": windowId, | ||
| }, | ||
| selection: { start: newCursorPosition }, | ||
| }); | ||
| } else { | ||
| setTimeout(() => { | ||
| const textareaElements = | ||
| document.querySelectorAll("textarea"); | ||
| for (const el of textareaElements) { | ||
| if (getUids(el).blockUid === blockUid) { | ||
| el.focus(); | ||
| el.setSelectionRange( | ||
| newCursorPosition, | ||
| newCursorPosition, | ||
| ); | ||
| break; | ||
| } | ||
| } | ||
| }, 50); | ||
| } | ||
| }); | ||
| posthog.capture("Discourse Node: Selected from Search Menu", { | ||
| id: item.id, | ||
| text: item.text, | ||
| }); | ||
trangdoan982 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }, 10); | ||
| }) | ||
| .catch((error) => { | ||
| console.error("Error waiting for block:", error); | ||
| }); | ||
| }, | ||
| [blockUid, onClose, textarea, triggerPosition, windowId], | ||
| ); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.