From c803f07685dfb490c09bb053461f44d61c6b23fd Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Fri, 7 Aug 2026 19:11:35 -0400 Subject: [PATCH 01/29] ENG-2109 Create node search modal with ranked results and preview Add the discourse node search surface: a Modal hosting a React root, a result list ranked by the QueryEngine functions from ENG-2108, and a Markdown preview of the active result. Register it as "Open node search" with no default hotkey, so users bind their own and we avoid colliding with core or community bindings. Highlight matched substrings with Obsidian's renderResults, passing the same string that was scored. Using the platform renderer rather than hand-rolled markup means highlights inherit theme styling, which is the code path that produced the equivalent Roam bug. Open with every node listed in title order rather than an empty prompt, so the modal doubles as a node browser. Model candidate loading as a discriminated union covering loading, ready, empty and error; the fetch is synchronous today, but semantic search will make it a network call and threading those states through later costs far more than carrying them now. Co-Authored-By: Claude Opus 5 --- .../src/components/NodeSearchModal.tsx | 353 ++++++++++++++++++ apps/obsidian/src/styles/style.css | 16 + apps/obsidian/src/utils/registerCommands.ts | 12 + 3 files changed, 381 insertions(+) create mode 100644 apps/obsidian/src/components/NodeSearchModal.tsx diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx new file mode 100644 index 000000000..b46325857 --- /dev/null +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -0,0 +1,353 @@ +import { + App, + Component, + MarkdownRenderer, + Modal, + Notice, + renderResults, + TFile, + type SearchResult, +} from "obsidian"; +import { + StrictMode, + useEffect, + useMemo, + useRef, + useState, + type KeyboardEvent, + type ReactElement, +} from "react"; +import { createRoot, Root } from "react-dom/client"; +import type DiscourseGraphPlugin from "~/index"; +import { + QueryEngine, + rankDiscourseNodesByTitle, + type DiscourseNodeCandidate, + type RankedDiscourseNode, +} from "~/services/QueryEngine"; + +const MAX_VISIBLE_RESULTS = 50; +const SEARCH_DEBOUNCE_MS = 250; + +/** + * Loading and error are unreachable today, since `getDiscourseNodeCandidates` is + * synchronous and swallows Datacore failures. They exist because semantic search + * (F12) queries Supabase over the network, and threading those states through + * every render branch later costs far more than carrying them now. + */ +type CandidateState = + | { status: "loading" } + | { status: "ready"; candidates: DiscourseNodeCandidate[] } + | { status: "error"; message: string }; + +type SearchResultRow = RankedDiscourseNode & { + nodeTypeName: string; + authorName: string; +}; + +/** + * A local note is authored by whoever is using the vault; only imported nodes + * carry an `authorId`, and resolving that to a display name is deferred to v1+. + */ +const resolveAuthorName = (app: App, file: TFile): string => { + const frontmatter = app.metadataCache.getFileCache(file)?.frontmatter as + | Record + | undefined; + return frontmatter?.authorId === undefined ? "You" : "Unknown"; +}; + +const formatTimestamp = (epochMs: number): string => + new Date(epochMs).toLocaleString(undefined, { + dateStyle: "medium", + timeStyle: "short", + }); + +const PreviewPane = ({ + app, + result, +}: { + app: App; + result: SearchResultRow | undefined; +}): ReactElement => { + const containerRef = useRef(null); + const [content, setContent] = useState(null); + + const file = result?.file; + + useEffect(() => { + if (!file) { + setContent(null); + return; + } + let cancelled = false; + void app.vault.cachedRead(file).then((text) => { + if (!cancelled) setContent(text); + }); + return () => { + cancelled = true; + }; + }, [app, file]); + + useEffect(() => { + const container = containerRef.current; + if (!container || !file || content === null) return; + + container.empty(); + const component = new Component(); + void MarkdownRenderer.render( + app, + content.trim() || "This note is empty.", + container, + file.path, + component, + ); + + return () => { + component.unload(); + container.empty(); + }; + }, [app, file, content]); + + if (!result || !file) { + return ( +
+ Select a result to preview it. +
+ ); + } + + return ( +
+
+
{result.title}
+
+ {`Created ${formatTimestamp(file.stat.ctime)} · Modified ${formatTimestamp( + file.stat.mtime, + )} · ${result.authorName}`} +
+
+
+
+ ); +}; + +/** + * `renderResults` slices `title` using the offsets in `match`, so it must be + * handed the exact string that was scored. It also applies the theme's own + * highlight styling, which is why matches are not marked up by hand. + */ +const HighlightedTitle = ({ + title, + match, +}: { + title: string; + match: SearchResult; +}): ReactElement => { + const titleRef = useRef(null); + + useEffect(() => { + const container = titleRef.current; + if (!container) return; + container.empty(); + renderResults(container, title, match); + return () => container.empty(); + }, [title, match]); + + return
; +}; + +const ResultList = ({ + results, + activeIndex, + onActivate, +}: { + results: SearchResultRow[]; + activeIndex: number; + onActivate: (index: number) => void; +}): ReactElement => { + const listRef = useRef(null); + + useEffect(() => { + const active = listRef.current?.children[activeIndex]; + active?.scrollIntoView({ block: "nearest" }); + }, [activeIndex]); + + return ( +
+ {results.map((result, index) => ( +
onActivate(index)} + className={`border-modifier-border cursor-pointer border-b px-3 py-2 ${ + index === activeIndex ? "bg-modifier-hover" : "" + }`} + > + +
{result.nodeTypeName}
+
+ ))} +
+ ); +}; + +const NodeSearch = ({ + plugin, +}: { + plugin: DiscourseGraphPlugin; +}): ReactElement => { + const { app } = plugin; + const [candidateState, setCandidateState] = useState({ + status: "loading", + }); + const [query, setQuery] = useState(""); + const [debouncedQuery, setDebouncedQuery] = useState(""); + const [activeIndex, setActiveIndex] = useState(0); + const inputRef = useRef(null); + + const nodeTypeNames = useMemo(() => { + const names = new Map(); + for (const nodeType of plugin.settings.nodeTypes) { + names.set(nodeType.id, nodeType.name); + } + return names; + }, [plugin.settings.nodeTypes]); + + useEffect(() => { + inputRef.current?.focus(); + }, []); + + // The fetch is synchronous today, so there is nothing to await or cancel yet. + // Effects run after paint, so the loading state still renders for a frame; when + // F12 makes this a network call, only this body changes. + useEffect(() => { + try { + const candidates = new QueryEngine(app).getDiscourseNodeCandidates(); + setCandidateState({ status: "ready", candidates }); + } catch (error) { + const message = + error instanceof Error ? error.message : "Unexpected error"; + new Notice(`Could not load discourse nodes: ${message}`); + setCandidateState({ status: "error", message }); + } + }, [app]); + + useEffect(() => { + const timeout = window.setTimeout( + () => setDebouncedQuery(query), + SEARCH_DEBOUNCE_MS, + ); + return () => window.clearTimeout(timeout); + }, [query]); + + const results = useMemo(() => { + if (candidateState.status !== "ready") return []; + return rankDiscourseNodesByTitle({ + candidates: candidateState.candidates, + query: debouncedQuery, + }) + .slice(0, MAX_VISIBLE_RESULTS) + .map((result) => ({ + ...result, + nodeTypeName: nodeTypeNames.get(result.nodeTypeId) ?? "Unknown type", + authorName: resolveAuthorName(app, result.file), + })); + }, [app, candidateState, debouncedQuery, nodeTypeNames]); + + useEffect(() => { + setActiveIndex(0); + }, [results]); + + const moveActiveIndex = (delta: number) => { + if (!results.length) return; + setActiveIndex((current) => { + const next = current + delta; + if (next < 0) return 0; + if (next > results.length - 1) return results.length - 1; + return next; + }); + }; + + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key !== "ArrowDown" && event.key !== "ArrowUp") return; + // Otherwise the caret jumps to the start or end of the query. + event.preventDefault(); + moveActiveIndex(event.key === "ArrowDown" ? 1 : -1); + }; + + return ( +
+ setQuery(event.target.value)} + onKeyDown={handleKeyDown} + className="w-full" + /> +
+
+ {candidateState.status === "loading" && ( +
Loading discourse nodes…
+ )} + {candidateState.status === "error" && ( +
+ Could not load discourse nodes. {candidateState.message} +
+ )} + {candidateState.status === "ready" && results.length === 0 && ( +
No results
+ )} + {candidateState.status === "ready" && results.length > 0 && ( + + )} +
+ +
+
+ ); +}; + +export class NodeSearchModal extends Modal { + private plugin: DiscourseGraphPlugin; + private root: Root | null = null; + + constructor(app: App, plugin: DiscourseGraphPlugin) { + super(app); + this.plugin = plugin; + } + + onOpen() { + const { contentEl, modalEl } = this; + modalEl.addClass("dg-node-search-modal"); + contentEl.empty(); + this.root = createRoot(contentEl); + this.root.render( + + + , + ); + } + + onClose() { + if (this.root) { + this.root.unmount(); + this.root = null; + } + this.contentEl.empty(); + } +} diff --git a/apps/obsidian/src/styles/style.css b/apps/obsidian/src/styles/style.css index 66e243fe6..489d82d81 100644 --- a/apps/obsidian/src/styles/style.css +++ b/apps/obsidian/src/styles/style.css @@ -3898,3 +3898,19 @@ kbd.tlui-kbd { background-color: var(--background-secondary); } } + +/* The default modal is too narrow for a result list beside a preview pane. + Responsive layout is an explicit non-goal, so this is a desktop-only size. */ +.dg-node-search-modal { + width: 900px; + max-width: 90vw; + height: 600px; + max-height: 80vh; +} + +.dg-node-search-modal .modal-content { + display: flex; + flex-direction: column; + height: 100%; + overflow: hidden; +} diff --git a/apps/obsidian/src/utils/registerCommands.ts b/apps/obsidian/src/utils/registerCommands.ts index f72544360..256caec00 100644 --- a/apps/obsidian/src/utils/registerCommands.ts +++ b/apps/obsidian/src/utils/registerCommands.ts @@ -3,6 +3,7 @@ import type DiscourseGraphPlugin from "~/index"; import { NodeTypeModal } from "~/components/NodeTypeModal"; import ModifyNodeModal from "~/components/ModifyNodeModal"; import { BulkIdentifyDiscourseNodesModal } from "~/components/BulkIdentifyDiscourseNodesModal"; +import { NodeSearchModal } from "~/components/NodeSearchModal"; import { ImportNodesModal } from "~/components/ImportNodesModal"; import { FeedbackModal } from "~/components/FeedbackModal"; import { convertPageToDiscourseNode, createDiscourseNode } from "./createNode"; @@ -137,6 +138,17 @@ export const registerCommands = (plugin: DiscourseGraphPlugin) => { }, }); + plugin.addCommand({ + id: "open-node-search", + name: "Open node search", + // No default hotkey: users bind their own, and we avoid colliding with core + // or community bindings. + hotkeys: [], + callback: () => { + new NodeSearchModal(plugin.app, plugin).open(); + }, + }); + plugin.addCommand({ id: "import-nodes-from-another-space", name: "Import nodes from another space", From b321c4a1f5c27932624c9f5097026b09383901c1 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Fri, 7 Aug 2026 20:05:54 -0400 Subject: [PATCH 02/29] ENG-2109 Use the search highlight colour for matched substrings renderResults applies Obsidian's suggestion highlight, which is styled for the quick switcher rather than for search. Point it at --text-highlight-bg instead, the variable behind the yellow in Obsidian's own search view, so matches read the same way there, here, and in the Roam implementation. Target the span element rather than Obsidian's internal class name: renderResults wraps matched ranges in spans and leaves unmatched text as bare text nodes, so every span inside the title is a match, and the rule survives a class rename. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/components/NodeSearchModal.tsx | 7 ++++++- apps/obsidian/src/styles/style.css | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index b46325857..22518dd5d 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -156,7 +156,12 @@ const HighlightedTitle = ({ return () => container.empty(); }, [title, match]); - return
; + return ( +
+ ); }; const ResultList = ({ diff --git a/apps/obsidian/src/styles/style.css b/apps/obsidian/src/styles/style.css index 489d82d81..2079f9e0f 100644 --- a/apps/obsidian/src/styles/style.css +++ b/apps/obsidian/src/styles/style.css @@ -3914,3 +3914,17 @@ kbd.tlui-kbd { height: 100%; overflow: hidden; } + +/* renderResults wraps matched ranges in spans and leaves unmatched text as bare + text nodes, so every span in here is a match. Targeting the element rather + than Obsidian's internal class keeps this working if that class is renamed. + + Obsidian styles these with the suggestion highlight, which is not the yellow + used by its own search view; --text-highlight-bg is that yellow, and stays + theme-aware rather than hardcoding a colour. */ +.dg-node-search-modal .dg-search-result-title span { + background-color: var(--text-highlight-bg); + color: inherit; + border-radius: var(--radius-s); + padding: 0 1px; +} From d4b8fc8d5c411d7bb784080ea32c053c7b478a46 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sat, 8 Aug 2026 13:30:14 -0400 Subject: [PATCH 03/29] Keep preview text paired with its file to avoid stale render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The preview pane read the newly selected note asynchronously while `content` still held the previous note's text, so the render effect fired once with the new file's path and the old file's body — the header showed one note while the pane rendered another. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/components/NodeSearchModal.tsx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 22518dd5d..4d57b8d79 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -70,18 +70,22 @@ const PreviewPane = ({ result: SearchResultRow | undefined; }): ReactElement => { const containerRef = useRef(null); - const [content, setContent] = useState(null); + // The text is kept with the file it came from so the pane never renders one + // note's body under another note's title while the next read is in flight. + const [loaded, setLoaded] = useState<{ file: TFile; text: string } | null>( + null, + ); const file = result?.file; useEffect(() => { if (!file) { - setContent(null); + setLoaded(null); return; } let cancelled = false; void app.vault.cachedRead(file).then((text) => { - if (!cancelled) setContent(text); + if (!cancelled) setLoaded({ file, text }); }); return () => { cancelled = true; @@ -90,13 +94,13 @@ const PreviewPane = ({ useEffect(() => { const container = containerRef.current; - if (!container || !file || content === null) return; + if (!container || !file || loaded?.file !== file) return; container.empty(); const component = new Component(); void MarkdownRenderer.render( app, - content.trim() || "This note is empty.", + loaded.text.trim() || "This note is empty.", container, file.path, component, @@ -106,7 +110,7 @@ const PreviewPane = ({ component.unload(); container.empty(); }; - }, [app, file, content]); + }, [app, file, loaded]); if (!result || !file) { return ( From dafa539383bb9d8c1063f8a908d6078441ca2463 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sat, 8 Aug 2026 13:41:31 -0400 Subject: [PATCH 04/29] Show node type as a badge and resolve author names from the shared cache Follows the Roam result row: the node type is a rounded badge of the first three letters, inline before the title, reusing the colors the editor already paints discourse tags with so a type reads the same in both places. Author names now resolve through `plugin.settings.userNames`, which `fetchUserNames` fills with one query for every person in the vault's spaces. The modal refreshes it at most once per open, and only when an imported node is actually missing a name, so nothing queries per result. Resolution also moved to the selected result, which is the only one whose author is displayed. Co-Authored-By: Claude Opus 5 --- .../src/components/NodeSearchModal.tsx | 150 +++++++++++++++--- apps/obsidian/src/utils/nodeTypeBadge.ts | 43 +++++ apps/obsidian/src/utils/typeUtils.ts | 9 +- 3 files changed, 176 insertions(+), 26 deletions(-) create mode 100644 apps/obsidian/src/utils/nodeTypeBadge.ts diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 4d57b8d79..b3cc00a71 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -25,6 +25,14 @@ import { type DiscourseNodeCandidate, type RankedDiscourseNode, } from "~/services/QueryEngine"; +import { + getNodeTypeBadge, + UNKNOWN_NODE_TYPE_BADGE, + type NodeTypeBadge, +} from "~/utils/nodeTypeBadge"; +import { fetchUserNames } from "~/utils/importNodes"; +import { getLoggedInClient } from "~/utils/supabaseContext"; +import { formatUserName } from "~/utils/typeUtils"; const MAX_VISIBLE_RESULTS = 50; const SEARCH_DEBOUNCE_MS = 250; @@ -40,20 +48,89 @@ type CandidateState = | { status: "ready"; candidates: DiscourseNodeCandidate[] } | { status: "error"; message: string }; +type NodeTypeDisplay = { + name: string; + badge: NodeTypeBadge; +}; + +const UNKNOWN_NODE_TYPE: NodeTypeDisplay = { + name: "Unknown type", + badge: UNKNOWN_NODE_TYPE_BADGE, +}; + type SearchResultRow = RankedDiscourseNode & { - nodeTypeName: string; - authorName: string; + nodeType: NodeTypeDisplay; }; -/** - * A local note is authored by whoever is using the vault; only imported nodes - * carry an `authorId`, and resolving that to a display name is deferred to v1+. - */ -const resolveAuthorName = (app: App, file: TFile): string => { +const getFrontmatterAuthorId = (app: App, file: TFile): number | undefined => { const frontmatter = app.metadataCache.getFileCache(file)?.frontmatter as | Record | undefined; - return frontmatter?.authorId === undefined ? "You" : "Unknown"; + const authorId = frontmatter?.authorId; + return typeof authorId === "number" ? authorId : undefined; +}; + +/** + * A local note is authored by whoever is using the vault; only imported nodes + * carry an `authorId`. The lookup is synchronous because `useAuthorNames` has + * already fetched every name; an id with no cached name degrades to `user ` + * rather than blocking the preview on a request. + */ +const resolveAuthorName = ({ + app, + file, + userNames, +}: { + app: App; + file: TFile; + userNames: Record; +}): string => { + const authorId = getFrontmatterAuthorId(app, file); + if (authorId === undefined) return "You"; + return formatUserName(userNames, authorId); +}; + +/** + * `fetchUserNames` returns every person in the vault's spaces in a single query + * and persists them, so names resolve during render with a map lookup. It runs + * at most once per modal open, and only when an imported node is actually + * missing a name — resolving per result or per selection would fire a request + * per author for data this one request already covers. + */ +const useAuthorNames = ({ + app, + plugin, + candidateState, +}: { + app: App; + plugin: DiscourseGraphPlugin; + candidateState: CandidateState; +}): Record => { + const [userNames, setUserNames] = useState(plugin.settings.userNames ?? {}); + + useEffect(() => { + if (candidateState.status !== "ready") return; + if (!plugin.settings.syncModeEnabled) return; + + const isMissingName = (candidate: DiscourseNodeCandidate): boolean => { + const authorId = getFrontmatterAuthorId(app, candidate.file); + return authorId !== undefined && !plugin.settings.userNames?.[authorId]; + }; + if (!candidateState.candidates.some(isMissingName)) return; + + let cancelled = false; + void (async () => { + const client = await getLoggedInClient(plugin); + if (!client || cancelled) return; + await fetchUserNames(plugin, client); + if (!cancelled) setUserNames(plugin.settings.userNames ?? {}); + })(); + return () => { + cancelled = true; + }; + }, [app, plugin, candidateState]); + + return userNames; }; const formatTimestamp = (epochMs: number): string => @@ -65,9 +142,11 @@ const formatTimestamp = (epochMs: number): string => const PreviewPane = ({ app, result, + authorName, }: { app: App; result: SearchResultRow | undefined; + authorName: string; }): ReactElement => { const containerRef = useRef(null); // The text is kept with the file it came from so the pane never renders one @@ -127,7 +206,7 @@ const PreviewPane = ({
{`Created ${formatTimestamp(file.stat.ctime)} · Modified ${formatTimestamp( file.stat.mtime, - )} · ${result.authorName}`} + )} · ${authorName}`}
); }; @@ -197,12 +276,22 @@ const ResultList = ({ role="option" aria-selected={index === activeIndex} onClick={() => onActivate(index)} - className={`border-modifier-border cursor-pointer border-b px-3 py-2 ${ + className={`border-modifier-border flex cursor-pointer items-center gap-2 border-b px-3 py-2 ${ index === activeIndex ? "bg-modifier-hover" : "" }`} > + + {result.nodeType.badge.text} + -
{result.nodeTypeName}
))}
@@ -222,13 +311,17 @@ const NodeSearch = ({ const [debouncedQuery, setDebouncedQuery] = useState(""); const [activeIndex, setActiveIndex] = useState(0); const inputRef = useRef(null); - - const nodeTypeNames = useMemo(() => { - const names = new Map(); - for (const nodeType of plugin.settings.nodeTypes) { - names.set(nodeType.id, nodeType.name); - } - return names; + const userNames = useAuthorNames({ app, plugin, candidateState }); + + const nodeTypesById = useMemo(() => { + const byId = new Map(); + plugin.settings.nodeTypes.forEach((nodeType, nodeIndex) => { + byId.set(nodeType.id, { + name: nodeType.name, + badge: getNodeTypeBadge({ nodeType, nodeIndex }), + }); + }); + return byId; }, [plugin.settings.nodeTypes]); useEffect(() => { @@ -267,10 +360,21 @@ const NodeSearch = ({ .slice(0, MAX_VISIBLE_RESULTS) .map((result) => ({ ...result, - nodeTypeName: nodeTypeNames.get(result.nodeTypeId) ?? "Unknown type", - authorName: resolveAuthorName(app, result.file), + nodeType: nodeTypesById.get(result.nodeTypeId) ?? UNKNOWN_NODE_TYPE, })); - }, [app, candidateState, debouncedQuery, nodeTypeNames]); + }, [candidateState, debouncedQuery, nodeTypesById]); + + const activeResult = results[activeIndex]; + + // Only the preview shows an author, so resolving the active result costs one + // lookup per selection instead of one per row on every keystroke. + const authorName = useMemo( + () => + activeResult + ? resolveAuthorName({ app, file: activeResult.file, userNames }) + : "", + [app, activeResult, userNames], + ); useEffect(() => { setActiveIndex(0); @@ -325,7 +429,7 @@ const NodeSearch = ({ /> )}
- +
); diff --git a/apps/obsidian/src/utils/nodeTypeBadge.ts b/apps/obsidian/src/utils/nodeTypeBadge.ts new file mode 100644 index 000000000..74f240064 --- /dev/null +++ b/apps/obsidian/src/utils/nodeTypeBadge.ts @@ -0,0 +1,43 @@ +import { DiscourseNode } from "~/types"; +import { getNodeTagColors } from "./colorUtils"; + +const BADGE_TEXT_LENGTH = 3; + +export type NodeTypeBadge = { + text: string; + backgroundColor: string; + textColor: string; +}; + +/** + * Mirrors Roam's `formatBadgeText` so a node type abbreviates to the same three + * letters in both apps. The tag wins over the name because it is the string + * users already see on the node itself. + */ +export const formatNodeTypeBadgeText = (source: string): string => + source.replace(/^#+/, "").trim().slice(0, BADGE_TEXT_LENGTH).toUpperCase(); + +/** + * Reuses the colors the editor already paints discourse tags with, so the same + * node type reads identically in a tag and in a search result. + */ +export const getNodeTypeBadge = ({ + nodeType, + nodeIndex, +}: { + nodeType: DiscourseNode; + nodeIndex: number; +}): NodeTypeBadge => ({ + text: formatNodeTypeBadgeText(nodeType.tag?.trim() || nodeType.name), + ...getNodeTagColors(nodeType, nodeIndex), +}); + +/** + * `nodeTypeId` comes from file frontmatter, so it can outlive the node type it + * names — deleted types and notes imported from another vault both land here. + */ +export const UNKNOWN_NODE_TYPE_BADGE: NodeTypeBadge = { + text: "?", + backgroundColor: "var(--background-modifier-hover)", + textColor: "var(--text-muted)", +}; diff --git a/apps/obsidian/src/utils/typeUtils.ts b/apps/obsidian/src/utils/typeUtils.ts index 9540ea81f..19808ed81 100644 --- a/apps/obsidian/src/utils/typeUtils.ts +++ b/apps/obsidian/src/utils/typeUtils.ts @@ -89,9 +89,12 @@ export const getAndFormatImportSource = ( return formatImportSource(importInfo.spaceUri || "", spaceNames); }; +export const formatUserName = ( + userNames: Record | undefined, + id: number, +): string => (userNames || {})[id] || `user ${id}`; + export const getUserNameById = ( plugin: DiscourseGraphPlugin, id: number, -): string => { - return (plugin.settings.userNames || {})[id] || `user ${id}`; -}; +): string => formatUserName(plugin.settings.userNames, id); From 39da641bc19e9b6c01ff672dc189d9c7966ce80e Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 9 Aug 2026 13:19:26 -0400 Subject: [PATCH 05/29] Tighten comments on the node type badge and author name cache Co-Authored-By: Claude Opus 5 --- .../src/components/NodeSearchModal.tsx | 21 +++++++------------ apps/obsidian/src/utils/nodeTypeBadge.ts | 15 ++++--------- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index b3cc00a71..48e71d2ef 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -71,10 +71,8 @@ const getFrontmatterAuthorId = (app: App, file: TFile): number | undefined => { }; /** - * A local note is authored by whoever is using the vault; only imported nodes - * carry an `authorId`. The lookup is synchronous because `useAuthorNames` has - * already fetched every name; an id with no cached name degrades to `user ` - * rather than blocking the preview on a request. + * Only imported nodes carry an `authorId`; a local note is the vault owner's. + * `useAuthorNames` has already cached the names, so this stays synchronous. */ const resolveAuthorName = ({ app, @@ -91,11 +89,9 @@ const resolveAuthorName = ({ }; /** - * `fetchUserNames` returns every person in the vault's spaces in a single query - * and persists them, so names resolve during render with a map lookup. It runs - * at most once per modal open, and only when an imported node is actually - * missing a name — resolving per result or per selection would fire a request - * per author for data this one request already covers. + * `fetchUserNames` returns every person in the vault's spaces in one query, so + * this refreshes once per open when a name is missing rather than querying per + * author. */ const useAuthorNames = ({ app, @@ -149,8 +145,8 @@ const PreviewPane = ({ authorName: string; }): ReactElement => { const containerRef = useRef(null); - // The text is kept with the file it came from so the pane never renders one - // note's body under another note's title while the next read is in flight. + // Paired with its file so an in-flight read can't put one note's body under + // another note's title. const [loaded, setLoaded] = useState<{ file: TFile; text: string } | null>( null, ); @@ -366,8 +362,7 @@ const NodeSearch = ({ const activeResult = results[activeIndex]; - // Only the preview shows an author, so resolving the active result costs one - // lookup per selection instead of one per row on every keystroke. + // Only the preview shows an author, so resolve the selection, not all 50 rows. const authorName = useMemo( () => activeResult diff --git a/apps/obsidian/src/utils/nodeTypeBadge.ts b/apps/obsidian/src/utils/nodeTypeBadge.ts index 74f240064..b07d6dff1 100644 --- a/apps/obsidian/src/utils/nodeTypeBadge.ts +++ b/apps/obsidian/src/utils/nodeTypeBadge.ts @@ -10,17 +10,13 @@ export type NodeTypeBadge = { }; /** - * Mirrors Roam's `formatBadgeText` so a node type abbreviates to the same three - * letters in both apps. The tag wins over the name because it is the string - * users already see on the node itself. + * Mirrors Roam's `formatBadgeText`. The tag wins over the name because it is + * the string users already see on the node. */ export const formatNodeTypeBadgeText = (source: string): string => source.replace(/^#+/, "").trim().slice(0, BADGE_TEXT_LENGTH).toUpperCase(); -/** - * Reuses the colors the editor already paints discourse tags with, so the same - * node type reads identically in a tag and in a search result. - */ +/** Reuses the editor's tag colors so a node type reads the same everywhere. */ export const getNodeTypeBadge = ({ nodeType, nodeIndex, @@ -32,10 +28,7 @@ export const getNodeTypeBadge = ({ ...getNodeTagColors(nodeType, nodeIndex), }); -/** - * `nodeTypeId` comes from file frontmatter, so it can outlive the node type it - * names — deleted types and notes imported from another vault both land here. - */ +/** `nodeTypeId` comes from frontmatter, so it can outlive the type it names. */ export const UNKNOWN_NODE_TYPE_BADGE: NodeTypeBadge = { text: "?", backgroundColor: "var(--background-modifier-hover)", From 2ea117e3993b511e821d4e0802298a9edcfa37e1 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 9 Aug 2026 13:38:34 -0400 Subject: [PATCH 06/29] Cycle the node type palette and correct author fallbacks getNodeTagColors clamped any index past the twelfth node type to 0, so every type beyond the palette length shared one color. Cycling spreads them instead. This also changes existing tag colors for vaults with more than twelve types. Author resolution now distinguishes the two cases the scope doc separates: no authorId means the note is local ("You"), while an authorId that cannot be resolved from settings or Supabase stays "Unknown" rather than claiming local authorship. A non-numeric authorId counts as present-but-unresolvable. Co-Authored-By: Claude Opus 5 --- .../src/components/NodeSearchModal.tsx | 25 ++++++++++++------- apps/obsidian/src/utils/colorUtils.ts | 6 ++--- apps/obsidian/src/utils/typeUtils.ts | 9 +++---- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 48e71d2ef..788aed794 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -32,7 +32,6 @@ import { } from "~/utils/nodeTypeBadge"; import { fetchUserNames } from "~/utils/importNodes"; import { getLoggedInClient } from "~/utils/supabaseContext"; -import { formatUserName } from "~/utils/typeUtils"; const MAX_VISIBLE_RESULTS = 50; const SEARCH_DEBOUNCE_MS = 250; @@ -62,17 +61,22 @@ type SearchResultRow = RankedDiscourseNode & { nodeType: NodeTypeDisplay; }; -const getFrontmatterAuthorId = (app: App, file: TFile): number | undefined => { +const LOCAL_AUTHOR_NAME = "You"; +const UNRESOLVED_AUTHOR_NAME = "Unknown"; + +/** Frontmatter is untyped, so the raw value is narrowed by each caller. */ +const getFrontmatterAuthorId = (app: App, file: TFile): unknown => { const frontmatter = app.metadataCache.getFileCache(file)?.frontmatter as | Record | undefined; - const authorId = frontmatter?.authorId; - return typeof authorId === "number" ? authorId : undefined; + return frontmatter?.authorId; }; /** - * Only imported nodes carry an `authorId`; a local note is the vault owner's. - * `useAuthorNames` has already cached the names, so this stays synchronous. + * "You" belongs only to a note with no `authorId` at all — every note in an + * unsynced vault. An id that is present but unresolvable stays "Unknown" rather + * than claiming local authorship. `useAuthorNames` has already cached the + * names, so this stays synchronous. */ const resolveAuthorName = ({ app, @@ -84,8 +88,9 @@ const resolveAuthorName = ({ userNames: Record; }): string => { const authorId = getFrontmatterAuthorId(app, file); - if (authorId === undefined) return "You"; - return formatUserName(userNames, authorId); + if (authorId === undefined || authorId === null) return LOCAL_AUTHOR_NAME; + if (typeof authorId !== "number") return UNRESOLVED_AUTHOR_NAME; + return userNames[authorId] ?? UNRESOLVED_AUTHOR_NAME; }; /** @@ -110,7 +115,9 @@ const useAuthorNames = ({ const isMissingName = (candidate: DiscourseNodeCandidate): boolean => { const authorId = getFrontmatterAuthorId(app, candidate.file); - return authorId !== undefined && !plugin.settings.userNames?.[authorId]; + return ( + typeof authorId === "number" && !plugin.settings.userNames?.[authorId] + ); }; if (!candidateState.candidates.some(isMissingName)) return; diff --git a/apps/obsidian/src/utils/colorUtils.ts b/apps/obsidian/src/utils/colorUtils.ts index 091667fa9..68757c389 100644 --- a/apps/obsidian/src/utils/colorUtils.ts +++ b/apps/obsidian/src/utils/colorUtils.ts @@ -42,8 +42,9 @@ export const getNodeTagColors = ( ): { backgroundColor: string; textColor: string } => { const customColor = nodeType.color || ""; - const safeIndex = - nodeIndex >= 0 && nodeIndex < COLOR_ARRAY.length ? nodeIndex : 0; + // Cycling keeps the 13th node type onwards spread across the palette; clamping + // them to index 0 made every type past the twelfth share one color. + const safeIndex = nodeIndex >= 0 ? nodeIndex % COLOR_ARRAY.length : 0; const paletteColorKey = COLOR_ARRAY[safeIndex]; const paletteColor = paletteColorKey ? COLOR_PALETTE[paletteColorKey] @@ -55,7 +56,6 @@ export const getNodeTagColors = ( return { backgroundColor, textColor }; }; - export const getAllDiscourseNodeColors = ( nodeTypes: DiscourseNode[], ): Array<{ diff --git a/apps/obsidian/src/utils/typeUtils.ts b/apps/obsidian/src/utils/typeUtils.ts index 19808ed81..9540ea81f 100644 --- a/apps/obsidian/src/utils/typeUtils.ts +++ b/apps/obsidian/src/utils/typeUtils.ts @@ -89,12 +89,9 @@ export const getAndFormatImportSource = ( return formatImportSource(importInfo.spaceUri || "", spaceNames); }; -export const formatUserName = ( - userNames: Record | undefined, - id: number, -): string => (userNames || {})[id] || `user ${id}`; - export const getUserNameById = ( plugin: DiscourseGraphPlugin, id: number, -): string => formatUserName(plugin.settings.userNames, id); +): string => { + return (plugin.settings.userNames || {})[id] || `user ${id}`; +}; From abe1dfcdefc182fe747d9bdc6f928d86cc8a7742 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 9 Aug 2026 13:43:57 -0400 Subject: [PATCH 07/29] ENG-2109 Navigate results by keyboard from anywhere in the modal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the arrow-key handler from the search input to the modal container, so navigation keeps working when focus moves elsewhere inside the modal, and so result actions have one place to live when they arrive. Mirrors the Roam dialog, which binds its handler at the same level. Activate rows on hover as well as click, again matching Roam. Suppress the mouseenter that fires when scrolling drags a row under a stationary cursor — that is the list moving, not the user choosing, and honouring it makes arrow keys jump back a row. Prevent the default on mousedown so clicking a result never pulls focus out of the input. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/components/NodeSearchModal.tsx | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 788aed794..0cd552fb5 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -260,10 +260,14 @@ const ResultList = ({ onActivate: (index: number) => void; }): ReactElement => { const listRef = useRef(null); + const pointerMovedRef = useRef(false); useEffect(() => { const active = listRef.current?.children[activeIndex]; active?.scrollIntoView({ block: "nearest" }); + // Scrolling drags rows under a stationary cursor, and the mouseenter that + // fires is not a choice. Ignore hover until the pointer actually moves. + pointerMovedRef.current = false; }, [activeIndex]); return ( @@ -271,6 +275,7 @@ const ResultList = ({ ref={listRef} role="listbox" aria-label="Discourse node search results" + onMouseMove={() => (pointerMovedRef.current = true)} className="flex-1 overflow-y-auto" > {results.map((result, index) => ( @@ -278,7 +283,11 @@ const ResultList = ({ key={result.file.path} role="option" aria-selected={index === activeIndex} + onMouseEnter={() => pointerMovedRef.current && onActivate(index)} onClick={() => onActivate(index)} + // Keeps focus in the search input, so the keyboard path stays live + // after a click. + onMouseDown={(event) => event.preventDefault()} className={`border-modifier-border flex cursor-pointer items-center gap-2 border-b px-3 py-2 ${ index === activeIndex ? "bg-modifier-hover" : "" }`} @@ -392,7 +401,7 @@ const NodeSearch = ({ }); }; - const handleKeyDown = (event: KeyboardEvent) => { + const handleKeyDown = (event: KeyboardEvent) => { if (event.key !== "ArrowDown" && event.key !== "ArrowUp") return; // Otherwise the caret jumps to the start or end of the query. event.preventDefault(); @@ -400,14 +409,15 @@ const NodeSearch = ({ }; return ( -
+ // Bound here rather than on the input so navigation survives focus moving + // elsewhere in the modal, and so result actions have one place to live. +
setQuery(event.target.value)} - onKeyDown={handleKeyDown} className="w-full" />
From 08d3da9fd3f5956f7dcfc936ee231bf656b0633b Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 9 Aug 2026 13:49:31 -0400 Subject: [PATCH 08/29] ENG-2109 Derive the badge from the title when the node type is gone A "?" chip told the reader nothing except that something was wrong. Roam handles the same case by storing the type's label on each result at index time and falling back to that; we have no stored label, but node formats are `PREFIX - {content}`, so the title still carries the prefix the badge would have shown. A note whose type was deleted, or imported from a differently configured vault, now reads QUE or CLM instead of ?. Omit the chip entirely when the title has no prefix either. Abbreviating the note's own words would produce a confident-looking label that says nothing about its type, which is worse than no label. Co-Authored-By: Claude Opus 5 --- .../src/components/NodeSearchModal.tsx | 39 ++++++++++--------- apps/obsidian/src/utils/nodeTypeBadge.ts | 29 +++++++++++--- 2 files changed, 44 insertions(+), 24 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 0cd552fb5..d3a8dd5e0 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -27,7 +27,7 @@ import { } from "~/services/QueryEngine"; import { getNodeTypeBadge, - UNKNOWN_NODE_TYPE_BADGE, + getFallbackNodeTypeBadge, type NodeTypeBadge, } from "~/utils/nodeTypeBadge"; import { fetchUserNames } from "~/utils/importNodes"; @@ -49,12 +49,8 @@ type CandidateState = type NodeTypeDisplay = { name: string; - badge: NodeTypeBadge; -}; - -const UNKNOWN_NODE_TYPE: NodeTypeDisplay = { - name: "Unknown type", - badge: UNKNOWN_NODE_TYPE_BADGE, + /** Null when neither the config nor the title says what type this is. */ + badge: NodeTypeBadge | null; }; type SearchResultRow = RankedDiscourseNode & { @@ -292,17 +288,19 @@ const ResultList = ({ index === activeIndex ? "bg-modifier-hover" : "" }`} > - - {result.nodeType.badge.text} - + {result.nodeType.badge && ( + + {result.nodeType.badge.text} + + )}
))} @@ -372,7 +370,10 @@ const NodeSearch = ({ .slice(0, MAX_VISIBLE_RESULTS) .map((result) => ({ ...result, - nodeType: nodeTypesById.get(result.nodeTypeId) ?? UNKNOWN_NODE_TYPE, + nodeType: nodeTypesById.get(result.nodeTypeId) ?? { + name: "Unknown type", + badge: getFallbackNodeTypeBadge(result.title), + }, })); }, [candidateState, debouncedQuery, nodeTypesById]); diff --git a/apps/obsidian/src/utils/nodeTypeBadge.ts b/apps/obsidian/src/utils/nodeTypeBadge.ts index b07d6dff1..8f36e0a24 100644 --- a/apps/obsidian/src/utils/nodeTypeBadge.ts +++ b/apps/obsidian/src/utils/nodeTypeBadge.ts @@ -28,9 +28,28 @@ export const getNodeTypeBadge = ({ ...getNodeTagColors(nodeType, nodeIndex), }); -/** `nodeTypeId` comes from frontmatter, so it can outlive the type it names. */ -export const UNKNOWN_NODE_TYPE_BADGE: NodeTypeBadge = { - text: "?", - backgroundColor: "var(--background-modifier-hover)", - textColor: "var(--text-muted)", +/** + * `nodeTypeId` comes from frontmatter, so it can outlive the type it names — + * a deleted type, or a note imported from a vault configured differently. + * + * Roam covers this by storing the type's label on the result when it indexes, and + * falling back to that. We have no such label, but node formats are + * `PREFIX - {content}`, so the title still carries the prefix the badge would have + * shown. Returns null when the title has no prefix either: an abbreviation of the + * note's own words would say nothing about its type. + */ +export const getFallbackNodeTypeBadge = ( + title: string, +): NodeTypeBadge | null => { + const [prefix, ...rest] = title.split(" - "); + if (!rest.length || !prefix) return null; + + const text = formatNodeTypeBadgeText(prefix); + if (!text) return null; + + return { + text, + backgroundColor: "var(--background-modifier-hover)", + textColor: "var(--text-muted)", + }; }; From 437623891b15645ca506471e7a353388222a069d Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 9 Aug 2026 13:53:00 -0400 Subject: [PATCH 09/29] ENG-2109 Keep colorUtils out of this change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The palette-cycling fix is a real one — past the twelfth node type every type collapsed to a single colour — but it is a behaviour change to a util shared with the editor, and nothing in the search modal needs it: this vault has nine node types, so clamping and cycling agree. Reverted here so the search PR stays to the search surface; worth its own change. Also drop the badge comments that restated their code, keeping the one that explains what Roam does differently. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/utils/colorUtils.ts | 5 ++--- apps/obsidian/src/utils/nodeTypeBadge.ts | 18 +++++------------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/apps/obsidian/src/utils/colorUtils.ts b/apps/obsidian/src/utils/colorUtils.ts index 68757c389..a1ed9503c 100644 --- a/apps/obsidian/src/utils/colorUtils.ts +++ b/apps/obsidian/src/utils/colorUtils.ts @@ -42,9 +42,8 @@ export const getNodeTagColors = ( ): { backgroundColor: string; textColor: string } => { const customColor = nodeType.color || ""; - // Cycling keeps the 13th node type onwards spread across the palette; clamping - // them to index 0 made every type past the twelfth share one color. - const safeIndex = nodeIndex >= 0 ? nodeIndex % COLOR_ARRAY.length : 0; + const safeIndex = + nodeIndex >= 0 && nodeIndex < COLOR_ARRAY.length ? nodeIndex : 0; const paletteColorKey = COLOR_ARRAY[safeIndex]; const paletteColor = paletteColorKey ? COLOR_PALETTE[paletteColorKey] diff --git a/apps/obsidian/src/utils/nodeTypeBadge.ts b/apps/obsidian/src/utils/nodeTypeBadge.ts index 8f36e0a24..570d81f02 100644 --- a/apps/obsidian/src/utils/nodeTypeBadge.ts +++ b/apps/obsidian/src/utils/nodeTypeBadge.ts @@ -9,14 +9,9 @@ export type NodeTypeBadge = { textColor: string; }; -/** - * Mirrors Roam's `formatBadgeText`. The tag wins over the name because it is - * the string users already see on the node. - */ export const formatNodeTypeBadgeText = (source: string): string => source.replace(/^#+/, "").trim().slice(0, BADGE_TEXT_LENGTH).toUpperCase(); -/** Reuses the editor's tag colors so a node type reads the same everywhere. */ export const getNodeTypeBadge = ({ nodeType, nodeIndex, @@ -29,14 +24,11 @@ export const getNodeTypeBadge = ({ }); /** - * `nodeTypeId` comes from frontmatter, so it can outlive the type it names — - * a deleted type, or a note imported from a vault configured differently. - * - * Roam covers this by storing the type's label on the result when it indexes, and - * falling back to that. We have no such label, but node formats are - * `PREFIX - {content}`, so the title still carries the prefix the badge would have - * shown. Returns null when the title has no prefix either: an abbreviation of the - * note's own words would say nothing about its type. + * Mirrors Roam, which stores each node type's label on the search result at index + * time and falls back to it when the type can no longer be resolved. We have no + * stored label, but node formats are `PREFIX - {content}`, so the title still + * carries the prefix the badge would have shown. Null when it does not: an + * abbreviation of the note's own words would say nothing about its type. */ export const getFallbackNodeTypeBadge = ( title: string, From 2d6141845467c3829502f3f34ed7797b00c7fbcb Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 9 Aug 2026 13:53:54 -0400 Subject: [PATCH 10/29] ENG-2109 Restore colorUtils byte-for-byte The revert left a whitespace-only diff: the pre-commit formatter collapsed a double blank line the file already had. Committing without it so colorUtils drops out of this PR entirely rather than appearing as a one-line change. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/utils/colorUtils.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/obsidian/src/utils/colorUtils.ts b/apps/obsidian/src/utils/colorUtils.ts index a1ed9503c..091667fa9 100644 --- a/apps/obsidian/src/utils/colorUtils.ts +++ b/apps/obsidian/src/utils/colorUtils.ts @@ -55,6 +55,7 @@ export const getNodeTagColors = ( return { backgroundColor, textColor }; }; + export const getAllDiscourseNodeColors = ( nodeTypes: DiscourseNode[], ): Array<{ From 447dd271c13abfba9fb3d961a4863220a8ba6b22 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 9 Aug 2026 14:00:54 -0400 Subject: [PATCH 11/29] ENG-2109 Drop the remaining explanatory comments Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/components/NodeSearchModal.tsx | 11 ----------- apps/obsidian/src/styles/style.css | 7 ------- apps/obsidian/src/utils/nodeTypeBadge.ts | 7 ------- apps/obsidian/src/utils/registerCommands.ts | 2 -- 4 files changed, 27 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index d3a8dd5e0..048f5c3c0 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -36,12 +36,6 @@ import { getLoggedInClient } from "~/utils/supabaseContext"; const MAX_VISIBLE_RESULTS = 50; const SEARCH_DEBOUNCE_MS = 250; -/** - * Loading and error are unreachable today, since `getDiscourseNodeCandidates` is - * synchronous and swallows Datacore failures. They exist because semantic search - * (F12) queries Supabase over the network, and threading those states through - * every render branch later costs far more than carrying them now. - */ type CandidateState = | { status: "loading" } | { status: "ready"; candidates: DiscourseNodeCandidate[] } @@ -216,11 +210,6 @@ const PreviewPane = ({ ); }; -/** - * `renderResults` slices `title` using the offsets in `match`, so it must be - * handed the exact string that was scored. It also applies the theme's own - * highlight styling, which is why matches are not marked up by hand. - */ const HighlightedTitle = ({ title, match, diff --git a/apps/obsidian/src/styles/style.css b/apps/obsidian/src/styles/style.css index 2079f9e0f..63949fec5 100644 --- a/apps/obsidian/src/styles/style.css +++ b/apps/obsidian/src/styles/style.css @@ -3915,13 +3915,6 @@ kbd.tlui-kbd { overflow: hidden; } -/* renderResults wraps matched ranges in spans and leaves unmatched text as bare - text nodes, so every span in here is a match. Targeting the element rather - than Obsidian's internal class keeps this working if that class is renamed. - - Obsidian styles these with the suggestion highlight, which is not the yellow - used by its own search view; --text-highlight-bg is that yellow, and stays - theme-aware rather than hardcoding a colour. */ .dg-node-search-modal .dg-search-result-title span { background-color: var(--text-highlight-bg); color: inherit; diff --git a/apps/obsidian/src/utils/nodeTypeBadge.ts b/apps/obsidian/src/utils/nodeTypeBadge.ts index 570d81f02..a4ad3f258 100644 --- a/apps/obsidian/src/utils/nodeTypeBadge.ts +++ b/apps/obsidian/src/utils/nodeTypeBadge.ts @@ -23,13 +23,6 @@ export const getNodeTypeBadge = ({ ...getNodeTagColors(nodeType, nodeIndex), }); -/** - * Mirrors Roam, which stores each node type's label on the search result at index - * time and falls back to it when the type can no longer be resolved. We have no - * stored label, but node formats are `PREFIX - {content}`, so the title still - * carries the prefix the badge would have shown. Null when it does not: an - * abbreviation of the note's own words would say nothing about its type. - */ export const getFallbackNodeTypeBadge = ( title: string, ): NodeTypeBadge | null => { diff --git a/apps/obsidian/src/utils/registerCommands.ts b/apps/obsidian/src/utils/registerCommands.ts index 256caec00..de3e2eae6 100644 --- a/apps/obsidian/src/utils/registerCommands.ts +++ b/apps/obsidian/src/utils/registerCommands.ts @@ -141,8 +141,6 @@ export const registerCommands = (plugin: DiscourseGraphPlugin) => { plugin.addCommand({ id: "open-node-search", name: "Open node search", - // No default hotkey: users bind their own, and we avoid colliding with core - // or community bindings. hotkeys: [], callback: () => { new NodeSearchModal(plugin.app, plugin).open(); From f3200e16ab82b46c2f313a6c98545f7b0ec457f1 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 10 Aug 2026 11:19:19 -0400 Subject: [PATCH 12/29] Clamp the active index while results are being replaced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A narrowing query rebuilds `results` before the reset effect runs, so the old index could point past the new list for one render — blanking the preview and leaving no row highlighted. Clamping at render covers that frame; the effect still resets the state so arrow keys continue from the top. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/components/NodeSearchModal.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 048f5c3c0..4735ea142 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -366,7 +366,11 @@ const NodeSearch = ({ })); }, [candidateState, debouncedQuery, nodeTypesById]); - const activeResult = results[activeIndex]; + // A narrowing query rebuilds `results` before the effect below can reset the + // state, so the old index can point past the new list for one render. Clamping + // here keeps the preview and the highlighted row from blanking for that frame. + const activeIndexInRange = activeIndex < results.length ? activeIndex : 0; + const activeResult = results[activeIndexInRange]; // Only the preview shows an author, so resolve the selection, not all 50 rows. const authorName = useMemo( @@ -426,7 +430,7 @@ const NodeSearch = ({ {candidateState.status === "ready" && results.length > 0 && ( )} From 24a36254b3c4926a0758b5ac3d40c4f79af54dce Mon Sep 17 00:00:00 2001 From: Trang Doan <44855874+trangdoan982@users.noreply.github.com> Date: Mon, 17 Aug 2026 21:59:37 -0400 Subject: [PATCH 13/29] ENG-2113 Add footer action bar with open in new tab and split (#1292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ENG-2113 Add platform-aware keyboard hint symbols Obsidian renders modifiers as glyphs on macOS and as words on Windows and Linux. Roam's search footer hardcoded the macOS glyphs at each call site and showed the wrong hint on Windows (ENG-2000); routing every hint through one map is what keeps that from repeating. `formatHintKeys` takes `isMacOS` so the non-mac branch can be exercised without that platform. Co-Authored-By: Claude Opus 5 * ENG-2113 Add footer action bar with open in active pane and split Enter opens the active result in the current pane, Shift+Enter in a split, and both close the modal. Mod+Enter and Alt+Enter deliberately fall through, so the insert action (ENG-2114) can claim Mod+Enter as it does in Roam. The footer reuses Obsidian's own `prompt-instruction` markup, the classes `SuggestModal.setInstructions()` emits, so it matches the native quick switcher. This modal extends plain `Modal`, so that API is unavailable. Its actions are left-aligned rather than centred because they sit under a full-width result list. The Enter branch lives in the existing wrapper `onKeyDown`, which ENG-2109 moved off the input so result actions would have one place to live. Co-Authored-By: Claude Opus 5 * ENG-2113 Open the active result in a new tab rather than the current one Replacing the page the user was already reading loses their place, which is the opposite of what a lookup surface should do. `getLeaf("tab")` adds a tab to the main panel instead, so the previous note stays open behind it. This reuses the existing `openFileInNewTab`, so the `openFileInActivePane` helper added earlier in this branch is no longer needed. The label now reads "open in new tab" to match. Diverges from the ticket's stated Solution, which specified `getLeaf(false)`. Co-Authored-By: Claude Opus 5 * ENG-2113 Style footer keys as caps, matching Roam Obsidian's `prompt-instruction-command` is bold with no border, which made the lone `esc` hint read as emphasis rather than as a key. Roam's search footer draws every key as a bordered cap instead, so `esc` sits with the rest of the set. Keeps the `prompt-instructions` container for its native type and spacing, and takes the cap's border, radius, and background from Obsidian's CSS variables so it still follows the active theme. Co-Authored-By: Claude Opus 5 * ENG-2113 Drop the results list tooltip Obsidian renders `aria-label` as a hover tooltip, so labelling the listbox meant a tooltip covered the results as soon as the pointer entered the list. Co-Authored-By: Claude Opus 5 * ENG-2113 Make close clickable and drop the duplicate badge tooltip The close hint was the only footer item that ignored a click, which read as broken next to two working actions. It now goes through the same `FooterAction` as the others and calls the modal's own close. The badge carried both `title` and `aria-label` with the same text, so hovering one stacked a native tooltip on top of Obsidian's. Keeping `aria-label`, since Obsidian's is the themed one. Co-Authored-By: Claude Opus 5 * ENG-2113 Let focused footer buttons handle their own Enter A footer button reached by Tab had its bubbling Enter intercepted by the modal's keydown handler, whose preventDefault suppressed the button's native click. So Enter on close opened a new tab, and Enter on split opened a new tab too. Also moves the footer's layout onto Tailwind utilities, leaving only the four properties Obsidian defends with `button:not(.clickable-icon)` and `button:hover` — both (0,1,1), which outrank a single utility class — plus `font-size`, which has no inherit utility. Trims comments that explained history rather than the code. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- .../src/components/NodeSearchFooter.tsx | 74 +++++++++++++++++++ .../src/components/NodeSearchModal.tsx | 58 +++++++++++++-- apps/obsidian/src/styles/style.css | 42 +++++++++++ apps/obsidian/src/utils/keyboardHints.ts | 33 +++++++++ 4 files changed, 201 insertions(+), 6 deletions(-) create mode 100644 apps/obsidian/src/components/NodeSearchFooter.tsx create mode 100644 apps/obsidian/src/utils/keyboardHints.ts diff --git a/apps/obsidian/src/components/NodeSearchFooter.tsx b/apps/obsidian/src/components/NodeSearchFooter.tsx new file mode 100644 index 000000000..2778c840f --- /dev/null +++ b/apps/obsidian/src/components/NodeSearchFooter.tsx @@ -0,0 +1,74 @@ +import { type ReactElement } from "react"; +import { getHintKeys, type HintKey } from "~/utils/keyboardHints"; + +type NodeSearchFooterProps = { + canAct: boolean; + onClose: () => void; + onOpenInNewTab: () => void; + onOpenInSplit: () => void; +}; + +type FooterActionProps = { + disabled?: boolean; + keys: HintKey[]; + label: string; + onClick: () => void; +}; + +const KeyHints = ({ keys }: { keys: HintKey[] }): ReactElement => ( + <> + {getHintKeys(keys).map((symbol) => ( + + {symbol} + + ))} + +); + +const FooterAction = ({ + disabled = false, + keys, + label, + onClick, +}: FooterActionProps): ReactElement => ( + +); + +// Sits in Obsidian's `prompt-instructions` container for its type and spacing. +// Obsidian centres that row for the narrow quick switcher; this footer spans a +// full-width result list, so the actions start at its left edge instead. +export const NodeSearchFooter = ({ + canAct, + onClose, + onOpenInNewTab, + onOpenInSplit, +}: NodeSearchFooterProps): ReactElement => ( +
+ + + {/* The Escape key itself is handled by Obsidian's modal scope; this button + is the pointer equivalent, so every footer item responds to a click. */} + +
+); diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 4735ea142..6bbf4dfa4 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -19,6 +19,11 @@ import { } from "react"; import { createRoot, Root } from "react-dom/client"; import type DiscourseGraphPlugin from "~/index"; +import { NodeSearchFooter } from "~/components/NodeSearchFooter"; +import { + openFileInNewLeaf, + openFileInNewTab, +} from "~/components/canvas/utils/openFileUtils"; import { QueryEngine, rankDiscourseNodesByTitle, @@ -256,10 +261,11 @@ const ResultList = ({ }, [activeIndex]); return ( + // No `aria-label` here: Obsidian renders one as a hover tooltip, which + // covers the results the moment the pointer enters the list.
(pointerMovedRef.current = true)} className="flex-1 overflow-y-auto" > @@ -279,7 +285,6 @@ const ResultList = ({ > {result.nodeType.badge && ( void; }): ReactElement => { const { app } = plugin; const [candidateState, setCandidateState] = useState({ @@ -395,11 +402,44 @@ const NodeSearch = ({ }); }; + // Closes before opening: `close()` unmounts this React root, so the file and + // app are read first and nothing touches state afterwards. + const openActiveResult = ( + open: (app: App, file: TFile) => Promise, + ): void => { + if (!activeResult) return; + const { file } = activeResult; + onClose(); + void open(app, file).catch((error: unknown) => { + const message = error instanceof Error ? error.message : String(error); + new Notice(`Could not open ${file.basename}: ${message}`); + }); + }; + const handleKeyDown = (event: KeyboardEvent) => { - if (event.key !== "ArrowDown" && event.key !== "ArrowUp") return; - // Otherwise the caret jumps to the start or end of the query. + if (event.key === "ArrowDown" || event.key === "ArrowUp") { + // Otherwise the caret jumps to the start or end of the query. + event.preventDefault(); + moveActiveIndex(event.key === "ArrowDown" ? 1 : -1); + return; + } + + if (event.key !== "Enter") return; + // Enter also commits an IME candidate, which must not open a file. + if (event.nativeEvent.isComposing) return; + // Mod+Enter and Alt+Enter are left alone for the insert and dock actions. + if (event.metaKey || event.ctrlKey || event.altKey) return; + // A footer button reached by Tab runs its own action on Enter. Preventing the + // default here would suppress that click and open a new tab instead. + if ( + event.target instanceof HTMLElement && + event.target.closest("button") !== null + ) { + return; + } + event.preventDefault(); - moveActiveIndex(event.key === "ArrowDown" ? 1 : -1); + openActiveResult(event.shiftKey ? openFileInNewLeaf : openFileInNewTab); }; return ( @@ -437,6 +477,12 @@ const NodeSearch = ({
+ openActiveResult(openFileInNewTab)} + onOpenInSplit={() => openActiveResult(openFileInNewLeaf)} + />
); }; @@ -457,7 +503,7 @@ export class NodeSearchModal extends Modal { this.root = createRoot(contentEl); this.root.render( - + this.close()} /> , ); } diff --git a/apps/obsidian/src/styles/style.css b/apps/obsidian/src/styles/style.css index 63949fec5..f5be6226e 100644 --- a/apps/obsidian/src/styles/style.css +++ b/apps/obsidian/src/styles/style.css @@ -3921,3 +3921,45 @@ kbd.tlui-kbd { border-radius: var(--radius-s); padding: 0 1px; } + +/* Only the properties Tailwind utilities cannot win here; the rest of this + footer's layout is utilities on the elements themselves. Obsidian sets + `color`, `background-color`, and `box-shadow` in `button:not(.clickable-icon)` + and `button:hover` — both (0,1,1), which outrank a single utility class — so a + utility would leave the label in `--text-normal` on an interactive-grey pill. + `font-size` has no inherit utility, and without it the button takes + `--font-ui-small` rather than the smaller type of the row it sits in. */ +.dg-node-search-modal .dg-search-footer-action, +.dg-node-search-modal .dg-search-footer-action:hover { + color: inherit; + background-color: transparent; + box-shadow: none; + font-size: inherit; +} + +/* Each key is a bordered cap, so `esc` reads as one of the set rather than as + emphasised text. Kept in CSS for the inherited font and em-based sizing. */ +.dg-node-search-modal .dg-search-footer-key { + display: inline-flex; + align-items: center; + justify-content: center; + min-width: 1.5em; + padding: 0 var(--size-2-1); + border: 1px solid var(--background-modifier-border); + border-radius: var(--radius-s); + background-color: var(--background-primary); + color: var(--text-muted); + font-family: inherit; + font-size: inherit; + font-weight: inherit; + line-height: 1.6; +} + +.dg-node-search-modal .dg-search-footer-action:hover:not(:disabled) { + color: var(--text-normal); +} + +.dg-node-search-modal .dg-search-footer-action:disabled { + cursor: not-allowed; + opacity: 0.5; +} diff --git a/apps/obsidian/src/utils/keyboardHints.ts b/apps/obsidian/src/utils/keyboardHints.ts new file mode 100644 index 000000000..a14586ab3 --- /dev/null +++ b/apps/obsidian/src/utils/keyboardHints.ts @@ -0,0 +1,33 @@ +import { Platform } from "obsidian"; + +export type HintKey = "Mod" | "Alt" | "Shift" | "Enter" | "Escape"; + +// Obsidian shows glyphs on macOS and spelled-out words everywhere else. +const MAC_SYMBOLS: Record = { + Mod: "⌘", + Alt: "⌥", + Shift: "⇧", + Enter: "↵", + Escape: "esc", +}; + +const NON_MAC_SYMBOLS: Record = { + Mod: "Ctrl", + Alt: "Alt", + Shift: "Shift", + Enter: "Enter", + Escape: "Esc", +}; + +/** Takes `isMacOS` so the non-mac branch can be checked without that platform. */ +export const formatHintKeys = ({ + keys, + isMacOS, +}: { + keys: HintKey[]; + isMacOS: boolean; +}): string[] => + keys.map((key) => (isMacOS ? MAC_SYMBOLS : NON_MAC_SYMBOLS)[key]); + +export const getHintKeys = (keys: HintKey[]): string[] => + formatHintKeys({ keys, isMacOS: Platform.isMacOS }); From 470df4ff254f4d8b52052c7be644d9cdd97cbbd2 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 17 Aug 2026 22:40:09 -0400 Subject: [PATCH 14/29] ENG-2109 Address review: pointer guard, modal sizing utilities Co-Authored-By: Claude Opus 5 --- .../src/components/NodeSearchModal.tsx | 40 ++++++++++++++----- apps/obsidian/src/styles/style.css | 16 -------- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 6bbf4dfa4..e9d527e29 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -15,6 +15,7 @@ import { useRef, useState, type KeyboardEvent, + type MouseEvent, type ReactElement, } from "react"; import { createRoot, Root } from "react-dom/client"; @@ -250,23 +251,35 @@ const ResultList = ({ onActivate: (index: number) => void; }): ReactElement => { const listRef = useRef(null); - const pointerMovedRef = useRef(false); + const pointerPositionRef = useRef<{ x: number; y: number } | null>(null); useEffect(() => { const active = listRef.current?.children[activeIndex]; active?.scrollIntoView({ block: "nearest" }); - // Scrolling drags rows under a stationary cursor, and the mouseenter that - // fires is not a choice. Ignore hover until the pointer actually moves. - pointerMovedRef.current = false; }, [activeIndex]); + // Scrolling drags rows under a stationary cursor, and the mouseenter that + // fires is not a choice. Compare coordinates rather than resetting a flag on + // every activation, so hovering from row to row still counts as a choice. + const hasPointerMoved = (event: MouseEvent): boolean => { + const previous = pointerPositionRef.current; + pointerPositionRef.current = { x: event.clientX, y: event.clientY }; + return ( + previous === null || + previous.x !== event.clientX || + previous.y !== event.clientY + ); + }; + return ( // No `aria-label` here: Obsidian renders one as a hover tooltip, which // covers the results the moment the pointer enters the list.
(pointerMovedRef.current = true)} + onMouseMove={(event) => { + hasPointerMoved(event); + }} className="flex-1 overflow-y-auto" > {results.map((result, index) => ( @@ -274,7 +287,7 @@ const ResultList = ({ key={result.file.path} role="option" aria-selected={index === activeIndex} - onMouseEnter={() => pointerMovedRef.current && onActivate(index)} + onMouseEnter={(event) => hasPointerMoved(event) && onActivate(index)} onClick={() => onActivate(index)} // Keeps focus in the search input, so the keyboard path stays live // after a click. @@ -335,8 +348,8 @@ const NodeSearch = ({ }, []); // The fetch is synchronous today, so there is nothing to await or cancel yet. - // Effects run after paint, so the loading state still renders for a frame; when - // F12 makes this a network call, only this body changes. + // Effects run after paint, so the loading state still renders for a frame; if + // this ever becomes a network call, only this body changes. useEffect(() => { try { const candidates = new QueryEngine(app).getDiscourseNodeCandidates(); @@ -498,7 +511,16 @@ export class NodeSearchModal extends Modal { onOpen() { const { contentEl, modalEl } = this; - modalEl.addClass("dg-node-search-modal"); + // The default modal is too narrow for a result list beside a preview pane. + // Responsive layout is an explicit non-goal, so this is a desktop-only size. + modalEl.addClasses([ + "dg-node-search-modal", + "h-[600px]", + "max-h-[80vh]", + "w-[900px]", + "max-w-[90vw]", + ]); + contentEl.addClasses(["flex", "h-full", "flex-col", "overflow-hidden"]); contentEl.empty(); this.root = createRoot(contentEl); this.root.render( diff --git a/apps/obsidian/src/styles/style.css b/apps/obsidian/src/styles/style.css index f5be6226e..3cf9c64ce 100644 --- a/apps/obsidian/src/styles/style.css +++ b/apps/obsidian/src/styles/style.css @@ -3899,22 +3899,6 @@ kbd.tlui-kbd { } } -/* The default modal is too narrow for a result list beside a preview pane. - Responsive layout is an explicit non-goal, so this is a desktop-only size. */ -.dg-node-search-modal { - width: 900px; - max-width: 90vw; - height: 600px; - max-height: 80vh; -} - -.dg-node-search-modal .modal-content { - display: flex; - flex-direction: column; - height: 100%; - overflow: hidden; -} - .dg-node-search-modal .dg-search-result-title span { background-color: var(--text-highlight-bg); color: inherit; From e3dafe3ffd75ffabde8a89c06da284263863ddee Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 19 Aug 2026 13:29:32 -0400 Subject: [PATCH 15/29] ENG-2110 Add node type filter dropdown menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a type filter beside the node search input. Roam's advanced search is the design and behaviour reference; its pure filter semantics are ported, its BlueprintJS structure is not. The search seam already supported this — rankDiscourseNodesByTitle takes nodeTypeIds and filters before scoring — so this is UI and state only. - discourseNodeTypeFilter.ts ports Roam's semantics, including the canonicalisation that makes "none selected" and "all selected" both mean no filter, matching filterCandidatesByNodeTypeIds. - NodeTypeFilterMenu renders an Obsidian-native trigger (clickable-icon + setIcon) with a count badge, over a panel with checkbox rows, colour dots, per-row Only, Select all with indeterminate state, and a type search past 7 types. - selectedNodeTypeIds lives in NodeSearch as the single source of truth so ENG-2111's chips can share it. Co-Authored-By: Claude Opus 5 --- .../src/components/NodeSearchModal.tsx | 58 +++- .../src/components/NodeTypeFilterMenu.tsx | 292 ++++++++++++++++++ .../src/utils/discourseNodeTypeFilter.ts | 78 +++++ 3 files changed, 419 insertions(+), 9 deletions(-) create mode 100644 apps/obsidian/src/components/NodeTypeFilterMenu.tsx create mode 100644 apps/obsidian/src/utils/discourseNodeTypeFilter.ts diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index e9d527e29..592084afc 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -21,6 +21,7 @@ import { import { createRoot, Root } from "react-dom/client"; import type DiscourseGraphPlugin from "~/index"; import { NodeSearchFooter } from "~/components/NodeSearchFooter"; +import { NodeTypeFilterMenu } from "~/components/NodeTypeFilterMenu"; import { openFileInNewLeaf, openFileInNewTab, @@ -329,6 +330,10 @@ const NodeSearch = ({ const [query, setQuery] = useState(""); const [debouncedQuery, setDebouncedQuery] = useState(""); const [activeIndex, setActiveIndex] = useState(0); + // The single source of truth for active type filters: F6's chips will read and + // write this same state, so either surface can manage them. + const [selectedNodeTypeIds, setSelectedNodeTypeIds] = useState([]); + const [isTypeFilterOpen, setIsTypeFilterOpen] = useState(false); const inputRef = useRef(null); const userNames = useAuthorNames({ app, plugin, candidateState }); @@ -375,6 +380,7 @@ const NodeSearch = ({ return rankDiscourseNodesByTitle({ candidates: candidateState.candidates, query: debouncedQuery, + nodeTypeIds: selectedNodeTypeIds, }) .slice(0, MAX_VISIBLE_RESULTS) .map((result) => ({ @@ -384,7 +390,7 @@ const NodeSearch = ({ badge: getFallbackNodeTypeBadge(result.title), }, })); - }, [candidateState, debouncedQuery, nodeTypesById]); + }, [candidateState, debouncedQuery, nodeTypesById, selectedNodeTypeIds]); // A narrowing query rebuilds `results` before the effect below can reset the // state, so the old index can point past the new list for one render. Clamping @@ -429,7 +435,30 @@ const NodeSearch = ({ }); }; + // Focus stays in the search input while the panel is open, so Escape arrives + // here rather than at the panel. Obsidian's Modal closes on Escape from its own + // keymap scope, so the native event has to stop or the modal goes too. + const closeTypeFilterOnEscape = ( + event: KeyboardEvent, + ): void => { + event.preventDefault(); + event.nativeEvent.stopImmediatePropagation(); + setIsTypeFilterOpen(false); + inputRef.current?.focus(); + }; + + const handleTypeFilterOpenChange = (nextOpen: boolean): void => { + setIsTypeFilterOpen(nextOpen); + // Returns the keyboard path to the results the moment the panel closes. + if (!nextOpen) inputRef.current?.focus(); + }; + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === "Escape" && isTypeFilterOpen) { + closeTypeFilterOnEscape(event); + return; + } + if (event.key === "ArrowDown" || event.key === "ArrowUp") { // Otherwise the caret jumps to the start or end of the query. event.preventDefault(); @@ -459,14 +488,25 @@ const NodeSearch = ({ // Bound here rather than on the input so navigation survives focus moving // elsewhere in the modal, and so result actions have one place to live.
- setQuery(event.target.value)} - className="w-full" - /> + {/* Padded so the filter trigger's count badge, which sits outside the + button box, is not clipped by the modal's overflow-hidden content. */} +
+ setQuery(event.target.value)} + className="min-w-0 flex-1" + /> + +
{candidateState.status === "loading" && ( diff --git a/apps/obsidian/src/components/NodeTypeFilterMenu.tsx b/apps/obsidian/src/components/NodeTypeFilterMenu.tsx new file mode 100644 index 000000000..a0171537e --- /dev/null +++ b/apps/obsidian/src/components/NodeTypeFilterMenu.tsx @@ -0,0 +1,292 @@ +import { setIcon } from "obsidian"; +import { useEffect, useMemo, useRef, useState, type ReactElement } from "react"; +import { DiscourseNode } from "~/types"; +import { getAllDiscourseNodeColors } from "~/utils/colorUtils"; +import { + NODE_TYPE_FILTER_SEARCH_THRESHOLD, + filterNodeTypesByQuery, + fromPanelSelectedIds, + getSelectAllCheckState, + hasActiveTypeFilter, + toPanelSelectedIds, +} from "~/utils/discourseNodeTypeFilter"; + +const FilterIcon = ({ name }: { name: string }): ReactElement => ( + // Emptied first because React reuses the node across renders and `setIcon` + // appends rather than replaces. + { + if (!el) return; + el.empty(); + setIcon(el, name); + }} + /> +); + +const NodeTypeFilterRow = ({ + color, + isChecked, + nodeType, + onSelectOnly, + onToggle, +}: { + color: string | undefined; + isChecked: boolean; + nodeType: DiscourseNode; + onSelectOnly: () => void; + onToggle: () => void; +}): ReactElement => ( +
+ + +
+); + +const NodeTypeFilterPanel = ({ + nodeTypes, + onSelectedIdsChange, + selectedIds, +}: { + nodeTypes: DiscourseNode[]; + onSelectedIdsChange: (ids: string[]) => void; + selectedIds: string[]; +}): ReactElement => { + const [query, setQuery] = useState(""); + const searchRef = useRef(null); + + const showTypeSearch = nodeTypes.length > NODE_TYPE_FILTER_SEARCH_THRESHOLD; + + const colorsById = useMemo(() => { + const byId = new Map(); + getAllDiscourseNodeColors(nodeTypes).forEach(({ nodeType, colors }) => { + byId.set(nodeType.id, colors.backgroundColor); + }); + return byId; + }, [nodeTypes]); + + const filteredNodeTypes = useMemo( + () => filterNodeTypesByQuery(nodeTypes, query), + [nodeTypes, query], + ); + + const selectedIdSet = useMemo(() => new Set(selectedIds), [selectedIds]); + + const selectAllState = getSelectAllCheckState({ + selectedIds, + totalCount: nodeTypes.length, + }); + + useEffect(() => { + if (showTypeSearch) searchRef.current?.focus(); + }, [showTypeSearch]); + + const handleSelectAll = (): void => { + if (selectAllState === "off") { + onSelectedIdsChange(nodeTypes.map((nodeType) => nodeType.id)); + return; + } + onSelectedIdsChange([]); + }; + + const toggleType = (id: string): void => { + onSelectedIdsChange( + selectedIdSet.has(id) + ? selectedIds.filter((selectedId) => selectedId !== id) + : [...selectedIds, id], + ); + }; + + const hasTypeSearchQuery = query.trim().length > 0; + + return ( +
+ {showTypeSearch && ( +
+ setQuery(event.target.value)} + className="w-full" + /> +
+ )} +
+ {filteredNodeTypes.length === 0 ? ( +
+ No matching node types +
+ ) : ( + <> + {/* A partial list has no "all" to speak of, so the row is dropped + while searching rather than acting on the hidden types too. */} + {!hasTypeSearchQuery && ( + + )} + {filteredNodeTypes.map((nodeType) => ( + onSelectedIdsChange([nodeType.id])} + onToggle={() => toggleType(nodeType.id)} + /> + ))} + + )} +
+
+ ); +}; + +export const NodeTypeFilterMenu = ({ + isOpen, + nodeTypes, + onOpenChange, + onSelectedNodeTypeIdsChange, + selectedNodeTypeIds, +}: { + isOpen: boolean; + nodeTypes: DiscourseNode[]; + onOpenChange: (isOpen: boolean) => void; + onSelectedNodeTypeIdsChange: (ids: string[]) => void; + selectedNodeTypeIds: string[]; +}): ReactElement => { + const containerRef = useRef(null); + + const allTypeIds = useMemo( + () => nodeTypes.map((nodeType) => nodeType.id), + [nodeTypes], + ); + + const isFilterActive = hasActiveTypeFilter({ + selectedTypeIds: selectedNodeTypeIds, + allTypeIds, + }); + + const panelSelectedIds = useMemo( + () => + toPanelSelectedIds({ selectedTypeIds: selectedNodeTypeIds, allTypeIds }), + [allTypeIds, selectedNodeTypeIds], + ); + + // `activeDocument` rather than `document`, so the listener lands in whichever + // window holds the modal when Obsidian is running a popout. + useEffect(() => { + if (!isOpen) return; + const handlePointerDown = (event: MouseEvent) => { + if (containerRef.current?.contains(event.target as Node)) return; + onOpenChange(false); + }; + activeDocument.addEventListener("mousedown", handlePointerDown, true); + return () => + activeDocument.removeEventListener("mousedown", handlePointerDown, true); + }, [isOpen, onOpenChange]); + + const activeFilterCount = isFilterActive ? selectedNodeTypeIds.length : 0; + + return ( +
{ + if (event.key !== "Escape" || !isOpen) return; + // Obsidian's Modal closes on Escape from its own keymap scope, so the + // native event has to stop here or the whole modal goes with the panel. + event.preventDefault(); + event.nativeEvent.stopImmediatePropagation(); + // The modal's own Escape handler is an ancestor of this one; stopping the + // synthetic event too keeps it from closing the panel a second time. + event.stopPropagation(); + onOpenChange(false); + }} + > + + {isOpen && ( + + onSelectedNodeTypeIdsChange( + fromPanelSelectedIds({ panelSelectedIds: panelIds, allTypeIds }), + ) + } + selectedIds={panelSelectedIds} + /> + )} +
+ ); +}; diff --git a/apps/obsidian/src/utils/discourseNodeTypeFilter.ts b/apps/obsidian/src/utils/discourseNodeTypeFilter.ts new file mode 100644 index 000000000..c9a4df53e --- /dev/null +++ b/apps/obsidian/src/utils/discourseNodeTypeFilter.ts @@ -0,0 +1,78 @@ +import { DiscourseNode } from "~/types"; + +/** + * Node type filtering for the search modal. An empty `selectedTypeIds` means no + * filter, matching `filterCandidatesByNodeTypeIds` in QueryEngine — so "nothing + * selected" and "every type selected" are the same state and both show all nodes. + * Ported from Roam's advanced search so both apps filter alike. + */ + +/** Below this many types the list is short enough to scan without a search box. */ +export const NODE_TYPE_FILTER_SEARCH_THRESHOLD = 7; + +export type SelectAllCheckState = "off" | "indeterminate" | "on"; + +export const hasActiveTypeFilter = ({ + selectedTypeIds, + allTypeIds, +}: { + selectedTypeIds: string[]; + allTypeIds: string[]; +}): boolean => + selectedTypeIds.length > 0 && selectedTypeIds.length < allTypeIds.length; + +/** + * The stored empty set means "no filter", which the panel shows as every row + * checked — otherwise an unfiltered search would render as an empty checklist. + */ +export const toPanelSelectedIds = ({ + selectedTypeIds, + allTypeIds, +}: { + selectedTypeIds: string[]; + allTypeIds: string[]; +}): string[] => (selectedTypeIds.length === 0 ? allTypeIds : selectedTypeIds); + +/** + * Collapses both "all checked" and "none checked" back to the empty set, so the + * count badge and the search agree that neither is a filter. + */ +export const fromPanelSelectedIds = ({ + panelSelectedIds, + allTypeIds, +}: { + panelSelectedIds: string[]; + allTypeIds: string[]; +}): string[] => { + if ( + panelSelectedIds.length === 0 || + panelSelectedIds.length === allTypeIds.length + ) { + return []; + } + return panelSelectedIds; +}; + +export const filterNodeTypesByQuery = ( + nodeTypes: DiscourseNode[], + query: string, +): DiscourseNode[] => { + const trimmedQuery = query.trim().toLowerCase(); + if (!trimmedQuery) return nodeTypes; + + return nodeTypes.filter((nodeType) => + nodeType.name.toLowerCase().includes(trimmedQuery), + ); +}; + +export const getSelectAllCheckState = ({ + selectedIds, + totalCount, +}: { + selectedIds: string[]; + totalCount: number; +}): SelectAllCheckState => { + if (selectedIds.length === 0) return "off"; + if (selectedIds.length === totalCount) return "on"; + return "indeterminate"; +}; From f398094a7647a4d5ae2634d4f7747f4aff44c168 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 19 Aug 2026 17:15:35 -0400 Subject: [PATCH 16/29] ENG-2110 Address review: contain panel keystrokes, swap Select all for Clear filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Panel keystrokes no longer reach the modal's key handler. Only Escape was stopped before, so Enter typed in the type search ran the modal's "open the highlighted result" branch — closing the modal and opening an unrelated note — and the arrows moved the result selection. Replaces the "Select all" checkbox with a "Clear filter (n)" button shown only while a filter is active. Because an empty selection and a full one are the same state, the checkbox sat checked and inert whenever nothing was filtered, so clicking it appeared to do nothing. Clearing is the control's only real function, so it now says that. Co-Authored-By: Claude Opus 5 --- .../src/components/NodeTypeFilterMenu.tsx | 92 ++++++++----------- 1 file changed, 39 insertions(+), 53 deletions(-) diff --git a/apps/obsidian/src/components/NodeTypeFilterMenu.tsx b/apps/obsidian/src/components/NodeTypeFilterMenu.tsx index a0171537e..d53ba9274 100644 --- a/apps/obsidian/src/components/NodeTypeFilterMenu.tsx +++ b/apps/obsidian/src/components/NodeTypeFilterMenu.tsx @@ -6,7 +6,6 @@ import { NODE_TYPE_FILTER_SEARCH_THRESHOLD, filterNodeTypesByQuery, fromPanelSelectedIds, - getSelectAllCheckState, hasActiveTypeFilter, toPanelSelectedIds, } from "~/utils/discourseNodeTypeFilter"; @@ -70,10 +69,12 @@ const NodeTypeFilterRow = ({ ); const NodeTypeFilterPanel = ({ + isFilterActive, nodeTypes, onSelectedIdsChange, selectedIds, }: { + isFilterActive: boolean; nodeTypes: DiscourseNode[]; onSelectedIdsChange: (ids: string[]) => void; selectedIds: string[]; @@ -98,23 +99,10 @@ const NodeTypeFilterPanel = ({ const selectedIdSet = useMemo(() => new Set(selectedIds), [selectedIds]); - const selectAllState = getSelectAllCheckState({ - selectedIds, - totalCount: nodeTypes.length, - }); - useEffect(() => { if (showTypeSearch) searchRef.current?.focus(); }, [showTypeSearch]); - const handleSelectAll = (): void => { - if (selectAllState === "off") { - onSelectedIdsChange(nodeTypes.map((nodeType) => nodeType.id)); - return; - } - onSelectedIdsChange([]); - }; - const toggleType = (id: string): void => { onSelectedIdsChange( selectedIdSet.has(id) @@ -123,10 +111,24 @@ const NodeTypeFilterPanel = ({ ); }; - const hasTypeSearchQuery = query.trim().length > 0; - return (
+ {/* Clearing is the only thing this control ever does, so it says so and + appears only when there is a filter to clear. A "select all" checkbox + would sit checked-and-inert whenever no filter is active, since an empty + selection and a full one are the same state. */} + {isFilterActive && ( +
+ +
+ )} {showTypeSearch && (
) : ( - <> - {/* A partial list has no "all" to speak of, so the row is dropped - while searching rather than acting on the hidden types too. */} - {!hasTypeSearchQuery && ( - - )} - {filteredNodeTypes.map((nodeType) => ( - onSelectedIdsChange([nodeType.id])} - onToggle={() => toggleType(nodeType.id)} - /> - ))} - + filteredNodeTypes.map((nodeType) => ( + onSelectedIdsChange([nodeType.id])} + onToggle={() => toggleType(nodeType.id)} + /> + )) )}
@@ -233,14 +214,18 @@ export const NodeTypeFilterMenu = ({ ref={containerRef} className="relative shrink-0" onKeyDown={(event) => { - if (event.key !== "Escape" || !isOpen) return; - // Obsidian's Modal closes on Escape from its own keymap scope, so the - // native event has to stop here or the whole modal goes with the panel. + if (!isOpen) return; + // Every keystroke stops here while the panel is open. The modal's handler + // is an ancestor and reads Enter as "open the highlighted result" and the + // arrows as "move the selection", so typing in the type search would + // otherwise open a note and close the whole modal. + event.stopPropagation(); + if (event.key !== "Escape") return; + // Obsidian's Modal closes on Escape from its own keymap scope, which sits + // outside React, so the native event has to stop too or the modal goes + // with the panel. event.preventDefault(); event.nativeEvent.stopImmediatePropagation(); - // The modal's own Escape handler is an ancestor of this one; stopping the - // synthetic event too keeps it from closing the panel a second time. - event.stopPropagation(); onOpenChange(false); }} > @@ -278,6 +263,7 @@ export const NodeTypeFilterMenu = ({ {isOpen && ( onSelectedNodeTypeIdsChange( From 5bbc1cd15e0775aa54f12b637ff4a533a21c800a Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 19 Aug 2026 22:34:16 -0400 Subject: [PATCH 17/29] ENG-2110 Drop the unused select-all check state helper getSelectAllCheckState and SelectAllCheckState lost their only consumer when the Select all checkbox became the Clear filter button. Roam keeps its own copy, which its tri-state checkbox still uses. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/utils/discourseNodeTypeFilter.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/apps/obsidian/src/utils/discourseNodeTypeFilter.ts b/apps/obsidian/src/utils/discourseNodeTypeFilter.ts index c9a4df53e..07c9a7be9 100644 --- a/apps/obsidian/src/utils/discourseNodeTypeFilter.ts +++ b/apps/obsidian/src/utils/discourseNodeTypeFilter.ts @@ -10,8 +10,6 @@ import { DiscourseNode } from "~/types"; /** Below this many types the list is short enough to scan without a search box. */ export const NODE_TYPE_FILTER_SEARCH_THRESHOLD = 7; -export type SelectAllCheckState = "off" | "indeterminate" | "on"; - export const hasActiveTypeFilter = ({ selectedTypeIds, allTypeIds, @@ -64,15 +62,3 @@ export const filterNodeTypesByQuery = ( nodeType.name.toLowerCase().includes(trimmedQuery), ); }; - -export const getSelectAllCheckState = ({ - selectedIds, - totalCount, -}: { - selectedIds: string[]; - totalCount: number; -}): SelectAllCheckState => { - if (selectedIds.length === 0) return "off"; - if (selectedIds.length === totalCount) return "on"; - return "indeterminate"; -}; From bf53409eec0ece0dca672a70d112831b738a2815 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 19 Aug 2026 22:43:07 -0400 Subject: [PATCH 18/29] ENG-2110 Close the filter panel on Escape via a pushed keymap scope Escape cannot be intercepted from the DOM. Obsidian registers the Modal's close-on-Escape before any plugin React tree exists, so a listener added later always runs second: preventDefault plus stopImmediatePropagation in a React handler does not work, nor does a capture-phase window listener, nor registering on the Modal's own scope, since Scope resolves in registration order. The previous DOM attempt here closed the whole modal instead of the panel. Pushing a Scope while the panel is open lands above the modal in the stack, so the panel gets Escape first. The dead DOM handlers in both the panel and the modal are gone with it. Diagnosis credit to the SearchDropdown work on ENG-2112 (#1320), which fixes the same bug for the sort dropdown. Migrating this component onto that shared shell would drop ~120 duplicated lines and is best done once both land, since SearchDropdown does not exist on this branch. Co-Authored-By: Claude Opus 5 --- .../src/components/NodeSearchModal.tsx | 18 +--------- .../src/components/NodeTypeFilterMenu.tsx | 33 +++++++++++++------ 2 files changed, 24 insertions(+), 27 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 592084afc..e57be967a 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -435,18 +435,6 @@ const NodeSearch = ({ }); }; - // Focus stays in the search input while the panel is open, so Escape arrives - // here rather than at the panel. Obsidian's Modal closes on Escape from its own - // keymap scope, so the native event has to stop or the modal goes too. - const closeTypeFilterOnEscape = ( - event: KeyboardEvent, - ): void => { - event.preventDefault(); - event.nativeEvent.stopImmediatePropagation(); - setIsTypeFilterOpen(false); - inputRef.current?.focus(); - }; - const handleTypeFilterOpenChange = (nextOpen: boolean): void => { setIsTypeFilterOpen(nextOpen); // Returns the keyboard path to the results the moment the panel closes. @@ -454,11 +442,6 @@ const NodeSearch = ({ }; const handleKeyDown = (event: KeyboardEvent) => { - if (event.key === "Escape" && isTypeFilterOpen) { - closeTypeFilterOnEscape(event); - return; - } - if (event.key === "ArrowDown" || event.key === "ArrowUp") { // Otherwise the caret jumps to the start or end of the query. event.preventDefault(); @@ -500,6 +483,7 @@ const NodeSearch = ({ className="min-w-0 flex-1" /> void; @@ -194,6 +196,23 @@ export const NodeTypeFilterMenu = ({ [allTypeIds, selectedNodeTypeIds], ); + // Escape cannot be intercepted from the DOM. Obsidian registers the Modal's + // close-on-Escape before any plugin React tree exists, so a listener added + // later always runs second — preventDefault plus stopImmediatePropagation in a + // React handler, a capture-phase window listener, and registering on the + // Modal's own scope all fail, the last because Scope resolves in registration + // order. A pushed scope is the only thing that lands above the modal. + useEffect(() => { + if (!isOpen) return; + const scope = new Scope(); + scope.register([], "Escape", () => { + onOpenChange(false); + return false; + }); + app.keymap.pushScope(scope); + return () => app.keymap.popScope(scope); + }, [app, isOpen, onOpenChange]); + // `activeDocument` rather than `document`, so the listener lands in whichever // window holds the modal when Obsidian is running a popout. useEffect(() => { @@ -218,15 +237,9 @@ export const NodeTypeFilterMenu = ({ // Every keystroke stops here while the panel is open. The modal's handler // is an ancestor and reads Enter as "open the highlighted result" and the // arrows as "move the selection", so typing in the type search would - // otherwise open a note and close the whole modal. + // otherwise open a note and close the whole modal. Escape is not handled + // here because it never reaches the DOM — see the pushed scope above. event.stopPropagation(); - if (event.key !== "Escape") return; - // Obsidian's Modal closes on Escape from its own keymap scope, which sits - // outside React, so the native event has to stop too or the modal goes - // with the panel. - event.preventDefault(); - event.nativeEvent.stopImmediatePropagation(); - onOpenChange(false); }} > + +); + +/** Chips and the query caret in one field; `NodeSearch` owns the state and, as an ancestor, already handles the arrows, Enter and Escape that bubble out of here. */ +export const NodeTypeChipsSearchInput = ({ + inputRef, + nodeTypes, + onQueryChange, + onSelectedNodeTypeIdsChange, + query, + selectedNodeTypeIds, +}: { + inputRef: RefObject; + nodeTypes: DiscourseNode[]; + onQueryChange: (query: string) => void; + onSelectedNodeTypeIdsChange: (ids: string[]) => void; + query: string; + selectedNodeTypeIds: string[]; +}): ReactElement => { + const [focusedChipIndex, setFocusedChipIndex] = useState(NO_FOCUSED_CHIP); + const chipRefs = useRef<(HTMLSpanElement | null)[]>([]); + + const chipsById = useMemo(() => { + const byId = new Map(); + getAllDiscourseNodeColors(nodeTypes).forEach(({ nodeType, colors }) => { + byId.set(nodeType.id, { + backgroundColor: colors.backgroundColor, + id: nodeType.id, + name: nodeType.name, + textColor: colors.textColor, + }); + }); + return byId; + }, [nodeTypes]); + + const chips = useMemo( + () => selectedNodeTypeIds.flatMap((id) => chipsById.get(id) ?? []), + [chipsById, selectedNodeTypeIds], + ); + + const bestPrefixMatch = getBestPrefixMatch({ + nodeTypes, + query, + selectedTypeIds: selectedNodeTypeIds, + }); + + const completionSuffix = getCompletionSuffix({ bestPrefixMatch, query }); + + /** + * The query text is uncontrolled: React owns the chips but never the editable's + * content, so re-rendering on each keystroke cannot move the caret. Programmatic + * changes therefore have to write the text themselves. + */ + const writeQuery = (value: string): void => { + const field = inputRef.current; + if (field) field.textContent = value; + onQueryChange(value); + }; + + const focusQuery = (): void => { + setFocusedChipIndex(NO_FOCUSED_CHIP); + const field = inputRef.current; + if (!field) return; + field.focus(); + setCaretToEnd(field); + }; + + useEffect(() => { + if (focusedChipIndex === NO_FOCUSED_CHIP) return; + if (focusedChipIndex < selectedNodeTypeIds.length) { + chipRefs.current[focusedChipIndex]?.focus(); + return; + } + // The dropdown can clear the filter mid-focus, stranding focus on a removed node. + setFocusedChipIndex(NO_FOCUSED_CHIP); + inputRef.current?.focus(); + }, [focusedChipIndex, inputRef, selectedNodeTypeIds]); + + const commitNodeType = (nodeType: DiscourseNode): void => { + if (selectedNodeTypeIds.includes(nodeType.id)) return; + // Raw, not canonicalised: collapsing a full selection would vanish the new chip. + onSelectedNodeTypeIdsChange([...selectedNodeTypeIds, nodeType.id]); + writeQuery(""); + }; + + const removeChipAt = (chipIndex: number): string[] => { + const nextIds = selectedNodeTypeIds.filter( + (_, index) => index !== chipIndex, + ); + onSelectedNodeTypeIdsChange(nextIds); + return nextIds; + }; + + const handleChipKeyDown = ( + event: KeyboardEvent, + chipIndex: number, + ): void => { + if (event.key === "ArrowLeft") { + event.preventDefault(); + setFocusedChipIndex(Math.max(0, chipIndex - 1)); + return; + } + + if (event.key === "ArrowRight") { + event.preventDefault(); + if (chipIndex >= selectedNodeTypeIds.length - 1) { + focusQuery(); + return; + } + setFocusedChipIndex(chipIndex + 1); + return; + } + + if (event.key === "Backspace" || event.key === "Delete") { + event.preventDefault(); + const nextIds = removeChipAt(chipIndex); + if (!nextIds.length) { + focusQuery(); + return; + } + // Backspace walks left, Delete takes the chip that closed the gap: no focus jump. + const nextIndex = + event.key === "Backspace" + ? chipIndex - 1 + : Math.min(chipIndex, nextIds.length - 1); + if (nextIndex < 0) { + focusQuery(); + return; + } + setFocusedChipIndex(nextIndex); + return; + } + + // Typing with a chip focused is a return to the query, not a lost keystroke. + if (isPlainCharacterKey(event)) { + event.preventDefault(); + writeQuery(event.key); + focusQuery(); + } + }; + + const handleQueryKeyDown = (event: KeyboardEvent): void => { + // Suppress the line break only; Enter still bubbles, and modified Enter reaches nothing else. + if (event.key === "Enter" && !event.nativeEvent.isComposing) { + event.preventDefault(); + } + + if (event.key === "Tab") { + // With nothing pending, Tab is left alone so it still reaches the footer actions. + if (!bestPrefixMatch) return; + event.preventDefault(); + commitNodeType(bestPrefixMatch); + return; + } + + if (!selectedNodeTypeIds.length) return; + if (!isCaretAtStart(inputRef.current)) return; + + // Highlight first, so an over-eager Backspace cannot silently drop a filter. + if (event.key === "Backspace" && !query.length) { + event.preventDefault(); + setFocusedChipIndex(selectedNodeTypeIds.length - 1); + return; + } + + if (event.key === "ArrowLeft") { + event.preventDefault(); + setFocusedChipIndex(selectedNodeTypeIds.length - 1); + } + }; + + return ( +
{ + if (focusedChipIndex === NO_FOCUSED_CHIP) focusQuery(); + }} + > + {chips.map((chip, index) => ( + setFocusedChipIndex(index)} + onKeyDown={(event) => handleChipKeyDown(event, index)} + onRemove={() => { + removeChipAt(index); + focusQuery(); + }} + registerRef={(element) => { + chipRefs.current[index] = element; + }} + /> + ))} + {/* `plaintext-only` so a pasted selection cannot bring markup in with it. */} + + onQueryChange(event.currentTarget.textContent ?? "") + } + onKeyDown={handleQueryKeyDown} + className="dg-search-chip-input whitespace-pre-wrap break-words align-middle outline-none" + /> + {/* Inline rather than an overlay, so it follows the caret and wraps with the text. */} + {!!bestPrefixMatch && ( + + {completionSuffix} + + {getHintKeys(["Tab"])[0]} + + + )} + {!query && !chips.length && ( + + {QUERY_PLACEHOLDER} + + )} +
+ ); +}; diff --git a/apps/obsidian/src/utils/keyboardHints.ts b/apps/obsidian/src/utils/keyboardHints.ts index a14586ab3..ec38800a8 100644 --- a/apps/obsidian/src/utils/keyboardHints.ts +++ b/apps/obsidian/src/utils/keyboardHints.ts @@ -1,6 +1,6 @@ import { Platform } from "obsidian"; -export type HintKey = "Mod" | "Alt" | "Shift" | "Enter" | "Escape"; +export type HintKey = "Mod" | "Alt" | "Shift" | "Enter" | "Escape" | "Tab"; // Obsidian shows glyphs on macOS and spelled-out words everywhere else. const MAC_SYMBOLS: Record = { @@ -9,6 +9,7 @@ const MAC_SYMBOLS: Record = { Shift: "⇧", Enter: "↵", Escape: "esc", + Tab: "⇥", }; const NON_MAC_SYMBOLS: Record = { @@ -17,6 +18,7 @@ const NON_MAC_SYMBOLS: Record = { Shift: "Shift", Enter: "Enter", Escape: "Esc", + Tab: "Tab", }; /** Takes `isMacOS` so the non-mac branch can be checked without that platform. */ diff --git a/apps/obsidian/src/utils/nodeTypeChipCompletion.ts b/apps/obsidian/src/utils/nodeTypeChipCompletion.ts new file mode 100644 index 000000000..1d0151a00 --- /dev/null +++ b/apps/obsidian/src/utils/nodeTypeChipCompletion.ts @@ -0,0 +1,45 @@ +import { DiscourseNode } from "~/types"; + +/** Ghost completion for the search modal's chips; prefix-only, as a substring match has no suffix to draw. */ + +/** Exact match beats the first partial, so a name that prefixes another stays reachable. */ +export const getBestPrefixMatch = ({ + nodeTypes, + query, + selectedTypeIds, +}: { + nodeTypes: DiscourseNode[]; + query: string; + selectedTypeIds: string[]; +}): DiscourseNode | null => { + const normalizedQuery = query.trim().toLowerCase(); + if (!normalizedQuery) return null; + + const selectedTypeIdSet = new Set(selectedTypeIds); + const matches = nodeTypes.filter( + (nodeType) => + !selectedTypeIdSet.has(nodeType.id) && + nodeType.name.toLowerCase().startsWith(normalizedQuery), + ); + if (!matches.length) return null; + + const exactMatch = matches.find( + (nodeType) => nodeType.name.toLowerCase() === normalizedQuery, + ); + return exactMatch ?? matches[0] ?? null; +}; + +/** What is left to type; empty once the name is fully spelled, though still committable. */ +export const getCompletionSuffix = ({ + bestPrefixMatch, + query, +}: { + bestPrefixMatch: DiscourseNode | null; + query: string; +}): string => { + if (!bestPrefixMatch) return ""; + const trimmedQuery = query.trim(); + const { name } = bestPrefixMatch; + if (name.toLowerCase() === trimmedQuery.toLowerCase()) return ""; + return name.slice(trimmedQuery.length); +}; From eda7a345bc425795ac13b13f451d6703613d75ae Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 19 Aug 2026 23:02:04 -0400 Subject: [PATCH 20/29] ENG-2111 Address review: remove chips by id, drop the inline max-width MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Chip removal and keyboard bounds used positions in `selectedNodeTypeIds` while the index came from `chips`, the rendered list. The two cannot diverge today — the modal reads settings once with no subscription — but the assumption was load-bearing for nothing, so removal is by id and navigation is bounded by what is actually on screen. - `max-w-40` replaces the inline max-width, per apps/obsidian/AGENTS.md:100. The chip's colours stay inline: they are computed per type. Co-Authored-By: Claude Opus 5 --- .../components/NodeTypeChipsSearchInput.tsx | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/apps/obsidian/src/components/NodeTypeChipsSearchInput.tsx b/apps/obsidian/src/components/NodeTypeChipsSearchInput.tsx index 454cfe1ca..6abe80494 100644 --- a/apps/obsidian/src/components/NodeTypeChipsSearchInput.tsx +++ b/apps/obsidian/src/components/NodeTypeChipsSearchInput.tsx @@ -18,7 +18,6 @@ import { /** `-1` means the caret is in the query rather than on a chip. */ const NO_FOCUSED_CHIP = -1; -const CHIP_LABEL_MAX_WIDTH = "10rem"; const QUERY_PLACEHOLDER = "Search discourse nodes by title"; type NodeTypeChip = { @@ -80,9 +79,7 @@ const NodeTypeChipTag = ({ isFocused ? "outline-accent outline outline-2 outline-offset-1" : "" }`} > - - {chip.name} - + {chip.name} {/* `clickable-icon`, because Obsidian's `button:not(.clickable-icon)` rule outranks a utility class and would paint its own box behind the ×. */} +); + +export const NodeSortMenu = ({ + app, + isOpen, + onOpenChange, + onSortChange, + sortDirection, + sortKey, +}: { + app: App; + isOpen: boolean; + onOpenChange: (isOpen: boolean) => void; + onSortChange: (next: { sortKey: SortKey; direction: SortDirection }) => void; + sortDirection: SortDirection; + sortKey: SortKey; +}): ReactElement => { + const directionLabel = getSortDirectionLabel({ + sortKey, + direction: sortDirection, + }); + + const toggleDirection = (): void => + onSortChange({ + sortKey, + direction: sortDirection === "asc" ? "desc" : "asc", + }); + + // Re-picking the active dimension flips it, which is the gesture most sortable + // tables use; picking a different one starts from that dimension's default. + const selectSortKey = (nextKey: SortKey): void => { + if (nextKey === sortKey) { + toggleDirection(); + return; + } + onSortChange({ + sortKey: nextKey, + direction: getDefaultDirectionForKey(nextKey), + }); + }; + + return ( + +
+ {SORT_OPTIONS.map((option) => ( + selectSortKey(option.key)} + /> + ))} +
+
+ +
+ + ); +}; diff --git a/apps/obsidian/src/components/ObsidianIcon.tsx b/apps/obsidian/src/components/ObsidianIcon.tsx new file mode 100644 index 000000000..0f91d529b --- /dev/null +++ b/apps/obsidian/src/components/ObsidianIcon.tsx @@ -0,0 +1,24 @@ +import { setIcon } from "obsidian"; +import type { ReactElement } from "react"; + +/** + * Renders one of Obsidian's built-in icons. The host node is emptied first + * because React reuses it across renders and `setIcon` appends rather than + * replaces. + */ +export const ObsidianIcon = ({ + name, + className = "flex items-center", +}: { + name: string; + className?: string; +}): ReactElement => ( + { + if (!el) return; + el.empty(); + setIcon(el, name); + }} + /> +); diff --git a/apps/obsidian/src/components/SearchDropdown.tsx b/apps/obsidian/src/components/SearchDropdown.tsx new file mode 100644 index 000000000..8f472ef48 --- /dev/null +++ b/apps/obsidian/src/components/SearchDropdown.tsx @@ -0,0 +1,134 @@ +import { App, Scope } from "obsidian"; +import { useEffect, useRef, type ReactElement, type ReactNode } from "react"; +import { ObsidianIcon } from "~/components/ObsidianIcon"; + +/** + * The trigger-plus-panel shell shared by the search modal's toolbar controls. + * Every one of them has to solve the same three problems — closing on an + * outside click, keeping its keystrokes away from the modal's result + * navigation, and closing on Escape without taking the modal with it — so the + * answers live here once. + */ + +/** + * Which toolbar panel is open, or null. One value rather than a boolean per + * control, so two panels can never be open at the same time. + */ +export type SearchDropdownId = "type-filter" | "sort" | null; + +export const SearchDropdown = ({ + app, + ariaLabel, + badgeCount = 0, + children, + iconName, + isActive, + isDisabled = false, + isOpen, + onOpenChange, + panelClassName = "w-64", + title, + triggerLabel, +}: { + app: App; + ariaLabel: string; + /** Rendered as a superscript count on the trigger when above zero. */ + badgeCount?: number; + children: ReactNode; + iconName: string; + /** Highlights the trigger to show the control is doing something. */ + isActive: boolean; + isDisabled?: boolean; + isOpen: boolean; + onOpenChange: (isOpen: boolean) => void; + panelClassName?: string; + title: string; + /** Optional text beside the icon, for controls whose current value is worth + showing without opening the panel. */ + triggerLabel?: string; +}): ReactElement => { + const containerRef = useRef(null); + + /** + * Escape cannot be taken from the DOM. Obsidian's Modal registers its + * close-on-Escape before any plugin React tree exists, so a listener added + * later runs second at every phase and on every node — even a capture + * listener on `window` — by which point the modal is already closing. + * Registering on the modal's own scope does not help either: the built-in + * handler was registered first and wins. Pushing a scope puts this above the + * modal in the stack, which is the one place that gets Escape first. + */ + useEffect(() => { + if (!isOpen) return; + const scope = new Scope(); + scope.register([], "Escape", () => { + onOpenChange(false); + return false; + }); + app.keymap.pushScope(scope); + return () => app.keymap.popScope(scope); + }, [app, isOpen, onOpenChange]); + + // `activeDocument` rather than `document`, so the listener lands in whichever + // window holds the modal when Obsidian is running a popout. + useEffect(() => { + if (!isOpen) return; + const handlePointerDown = (event: MouseEvent) => { + if (containerRef.current?.contains(event.target as Node)) return; + onOpenChange(false); + }; + activeDocument.addEventListener("mousedown", handlePointerDown, true); + return () => + activeDocument.removeEventListener("mousedown", handlePointerDown, true); + }, [isOpen, onOpenChange]); + + return ( +
{ + if (!isOpen) return; + // Every keystroke stops here while the panel is open. The modal's handler + // is an ancestor and reads Enter as "open the highlighted result" and the + // arrows as "move the selection", so typing in a panel would otherwise + // open a note and close the whole modal. Escape is deliberately not + // handled here — it never reaches React, and the modal's keymap scope + // closes the panel instead. + event.stopPropagation(); + }} + > + + {isOpen && ( +
+ {children} +
+ )} +
+ ); +}; diff --git a/apps/obsidian/src/utils/discourseNodeAuthor.ts b/apps/obsidian/src/utils/discourseNodeAuthor.ts new file mode 100644 index 000000000..97b84033e --- /dev/null +++ b/apps/obsidian/src/utils/discourseNodeAuthor.ts @@ -0,0 +1,121 @@ +import { App, TFile } from "obsidian"; +import { useEffect, useState } from "react"; +import type DiscourseGraphPlugin from "~/index"; +import type { DiscourseNodeCandidate } from "~/services/QueryEngine"; +import { fetchUserNames } from "~/utils/importNodes"; +import { getLoggedInClient } from "~/utils/supabaseContext"; + +/** + * Author resolution for discourse node search. This is the single place that + * turns a note into a display name, so the preview header and the author sort + * can never disagree about who wrote something. + * + * Known limitation: Obsidian Sync is not a source here. The 1.8.7 typings + * expose no sync, user, or collaborator API, so per-file attribution from Sync + * cannot be read by a plugin. Names therefore come from the `authorId` + * frontmatter written by the importer, resolved against the ids cached in + * settings. + */ + +export const LOCAL_AUTHOR_NAME = "You"; +export const UNRESOLVED_AUTHOR_NAME = "Unknown"; + +/** Frontmatter is untyped, so the raw value is narrowed by each caller. */ +const getFrontmatterAuthorId = (app: App, file: TFile): unknown => { + const frontmatter = app.metadataCache.getFileCache(file)?.frontmatter as + | Record + | undefined; + return frontmatter?.authorId; +}; + +/** + * "You" belongs only to a note with no `authorId` at all — every note in an + * unsynced vault. An id that is present but unresolvable stays "Unknown" rather + * than claiming local authorship. `useAuthorNames` has already cached the + * names, so this stays synchronous. + */ +export const resolveAuthorName = ({ + app, + file, + userNames, +}: { + app: App; + file: TFile; + userNames: Record; +}): string => { + const authorId = getFrontmatterAuthorId(app, file); + if (authorId === undefined || authorId === null) return LOCAL_AUTHOR_NAME; + if (typeof authorId !== "number") return UNRESOLVED_AUTHOR_NAME; + return userNames[authorId] ?? UNRESOLVED_AUTHOR_NAME; +}; + +/** + * A name that identifies no one. Sorting keeps these out of the alphabetical + * run so an unreadable id never lands between two real authors. + */ +export const isUnattributedAuthorName = (authorName: string): boolean => + authorName === UNRESOLVED_AUTHOR_NAME; + +/** + * Author sort needs a name for every candidate, not just the previewed one, so + * the whole list is resolved up front and keyed by path. + */ +export const buildAuthorNameByPath = ({ + app, + files, + userNames, +}: { + app: App; + files: TFile[]; + userNames: Record; +}): Map => { + const byPath = new Map(); + files.forEach((file) => { + byPath.set(file.path, resolveAuthorName({ app, file, userNames })); + }); + return byPath; +}; + +/** + * `fetchUserNames` returns every person in the vault's spaces in one query, so + * this refreshes once per open when a name is missing rather than querying per + * author. + */ +export const useAuthorNames = ({ + app, + plugin, + candidates, +}: { + app: App; + plugin: DiscourseGraphPlugin; + /** Null until the candidate load finishes; nothing to resolve before then. */ + candidates: DiscourseNodeCandidate[] | null; +}): Record => { + const [userNames, setUserNames] = useState(plugin.settings.userNames ?? {}); + + useEffect(() => { + if (!candidates) return; + if (!plugin.settings.syncModeEnabled) return; + + const isMissingName = (candidate: DiscourseNodeCandidate): boolean => { + const authorId = getFrontmatterAuthorId(app, candidate.file); + return ( + typeof authorId === "number" && !plugin.settings.userNames?.[authorId] + ); + }; + if (!candidates.some(isMissingName)) return; + + let cancelled = false; + void (async () => { + const client = await getLoggedInClient(plugin); + if (!client || cancelled) return; + await fetchUserNames(plugin, client); + if (!cancelled) setUserNames(plugin.settings.userNames ?? {}); + })(); + return () => { + cancelled = true; + }; + }, [app, plugin, candidates]); + + return userNames; +}; diff --git a/apps/obsidian/src/utils/discourseNodeSort.ts b/apps/obsidian/src/utils/discourseNodeSort.ts new file mode 100644 index 000000000..61e4ecad4 --- /dev/null +++ b/apps/obsidian/src/utils/discourseNodeSort.ts @@ -0,0 +1,164 @@ +import { TFile } from "obsidian"; +import { isUnattributedAuthorName } from "~/utils/discourseNodeAuthor"; + +/** + * Client-side ordering for discourse node search results. Sorting runs over the + * full ranked list before it is truncated for display — sorting a + * relevance-truncated window would show the alphabetically-first 50 of the + * best-matching 50, which is not what any of these options mean. + * Ported from Roam's advanced search so both apps offer the same dimensions. + */ + +export type SortKey = + | "relevance" + | "title" + | "dateCreated" + | "dateModified" + | "author"; + +export type SortDirection = "asc" | "desc"; + +export const SORT_OPTIONS: { key: SortKey; label: string }[] = [ + { key: "relevance", label: "Relevance" }, + { key: "title", label: "Alphabetical" }, + { key: "dateCreated", label: "Date created" }, + { key: "dateModified", label: "Date modified" }, + { key: "author", label: "Author" }, +]; + +export const DEFAULT_SORT_KEY: SortKey = "relevance"; +/** Descending reads as "best first" for scores and "newest first" for dates. */ +export const DEFAULT_SORT_DIRECTION: SortDirection = "desc"; + +/** + * Structural rather than tied to `RankedDiscourseNode`, so the sort can run on + * the ranked list or on rows that have already been decorated for display. + */ +export type SortableSearchResult = { + file: TFile; + title: string; + match: { score: number }; +}; + +const DIRECTION_LABELS: Record> = { + relevance: { desc: "Best match first", asc: "Worst match first" }, + title: { asc: "A to Z", desc: "Z to A" }, + dateCreated: { desc: "Newest first", asc: "Oldest first" }, + dateModified: { desc: "Newest first", asc: "Oldest first" }, + author: { asc: "A to Z", desc: "Z to A" }, +}; + +export const getSortDirectionLabel = ({ + sortKey, + direction, +}: { + sortKey: SortKey; + direction: SortDirection; +}): string => DIRECTION_LABELS[sortKey][direction]; + +/** + * Switching dimension resets the direction, because "descending" means + * something different per dimension: keeping it would turn a switch to + * alphabetical into an unasked-for Z-to-A. + */ +export const getDefaultDirectionForKey = (sortKey: SortKey): SortDirection => + sortKey === "title" || sortKey === "author" ? "asc" : "desc"; + +export const isDefaultSort = ({ + sortKey, + direction, +}: { + sortKey: SortKey; + direction: SortDirection; +}): boolean => + sortKey === DEFAULT_SORT_KEY && direction === DEFAULT_SORT_DIRECTION; + +export const getSortOptionLabel = (sortKey: SortKey): string => + SORT_OPTIONS.find((option) => option.key === sortKey)?.label ?? ""; + +const getAuthorName = ({ + result, + authorNameByPath, +}: { + result: SortableSearchResult; + authorNameByPath: Map | undefined; +}): string => authorNameByPath?.get(result.file.path) ?? ""; + +/** + * Unattributed notes sit after every named author in both directions, so + * reversing the sort never buries the readable names under a block of + * "Unknown". Returns 0 when the two sides agree, leaving the ordering to the + * name comparison. + */ +const compareUnattributedLast = ({ + a, + b, + authorNameByPath, +}: { + a: SortableSearchResult; + b: SortableSearchResult; + authorNameByPath: Map | undefined; +}): number => { + const isAUnattributed = isUnattributedAuthorName( + getAuthorName({ result: a, authorNameByPath }), + ); + const isBUnattributed = isUnattributedAuthorName( + getAuthorName({ result: b, authorNameByPath }), + ); + if (isAUnattributed === isBUnattributed) return 0; + return isAUnattributed ? 1 : -1; +}; + +const compareAscending = ({ + a, + b, + sortKey, + authorNameByPath, +}: { + a: SortableSearchResult; + b: SortableSearchResult; + sortKey: SortKey; + authorNameByPath: Map | undefined; +}): number => { + if (sortKey === "relevance") return a.match.score - b.match.score; + if (sortKey === "title") return a.title.localeCompare(b.title); + if (sortKey === "dateCreated") return a.file.stat.ctime - b.file.stat.ctime; + if (sortKey === "dateModified") return a.file.stat.mtime - b.file.stat.mtime; + + // Same-author notes end up adjacent, and the title breaks the tie inside each + // group so the order is stable rather than left to vault iteration. + const authorDelta = getAuthorName({ + result: a, + authorNameByPath, + }).localeCompare(getAuthorName({ result: b, authorNameByPath })); + return authorDelta !== 0 ? authorDelta : a.title.localeCompare(b.title); +}; + +/** + * `authorNameByPath` is only needed for the author sort; the other keys ignore + * it so callers can skip resolving names for the whole list. + */ +export const sortSearchResults = ({ + results, + sortKey, + direction, + authorNameByPath, +}: { + results: T[]; + sortKey: SortKey; + direction: SortDirection; + authorNameByPath?: Map; +}): T[] => + [...results].sort((a, b) => { + if (sortKey === "author") { + // Outside the direction flip on purpose: this partition is not reversible. + const unattributedDelta = compareUnattributedLast({ + a, + b, + authorNameByPath, + }); + if (unattributedDelta !== 0) return unattributedDelta; + } + const delta = compareAscending({ a, b, sortKey, authorNameByPath }); + return direction === "asc" ? delta : -delta; + }); From 41ba82c29744ab86c5cb318dbd80c809d104539f Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 19 Aug 2026 22:15:33 -0400 Subject: [PATCH 23/29] ENG-2112 Mirror Roam's sort menu styling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The option rows were rendered as buttons, so Obsidian's button chrome gave each one its own filled, padded box — five separate widgets stacked in a panel rather than a menu. Rows are now flat divs on the panel's own background that react to hover, with the active row carrying the accent fill and the check. Follows Roam's sort menu (ENG-1732) the rest of the way: a "Sort by" header, and explicit Asc / Desc controls instead of one toggle whose label changed with the dimension. The trigger goes back to icon-only, as in Roam, with the direction arrow and the accent highlight carrying the active state and the full phrase moving to the tooltip. Verified over CDP: 29/29, including that an inactive row's background matches the panel's and that only the active row is coloured. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/components/NodeSortMenu.tsx | 111 ++++++++++++------ .../src/components/SearchDropdown.tsx | 9 +- 2 files changed, 73 insertions(+), 47 deletions(-) diff --git a/apps/obsidian/src/components/NodeSortMenu.tsx b/apps/obsidian/src/components/NodeSortMenu.tsx index f98d83a84..47d1878b1 100644 --- a/apps/obsidian/src/components/NodeSortMenu.tsx +++ b/apps/obsidian/src/components/NodeSortMenu.tsx @@ -12,9 +12,20 @@ import { type SortKey, } from "~/utils/discourseNodeSort"; +const DIRECTIONS: { direction: SortDirection; label: string }[] = [ + { direction: "asc", label: "Asc" }, + { direction: "desc", label: "Desc" }, +]; + const getDirectionIconName = (direction: SortDirection): string => direction === "asc" ? "arrow-up-narrow-wide" : "arrow-down-wide-narrow"; +/** + * Rows are divs rather than buttons: Obsidian's button chrome is a filled, + * padded control, which reads as five separate widgets stacked in a panel + * instead of a menu. Flat rows on the panel's own background, reacting only to + * hover, are what a menu looks like in both Obsidian and Roam. + */ const SortOptionRow = ({ isSelected, label, @@ -24,22 +35,59 @@ const SortOptionRow = ({ label: string; onSelect: () => void; }): ReactElement => ( - + {label} +
+); + +const DirectionToggle = ({ + onSelect, + sortDirection, + sortKey, +}: { + onSelect: (direction: SortDirection) => void; + sortDirection: SortDirection; + sortKey: SortKey; +}): ReactElement => ( +
+ {DIRECTIONS.map(({ direction, label }) => ( +
onSelect(direction)} + onMouseDown={(event) => event.preventDefault()} + className={`flex flex-1 cursor-pointer items-center justify-center gap-1 rounded px-2 py-1 text-sm ${ + direction === sortDirection + ? "bg-accent text-on-accent" + : "text-normal hover:bg-modifier-hover" + }`} + > + + {label} +
+ ))} +
); export const NodeSortMenu = ({ @@ -62,25 +110,6 @@ export const NodeSortMenu = ({ direction: sortDirection, }); - const toggleDirection = (): void => - onSortChange({ - sortKey, - direction: sortDirection === "asc" ? "desc" : "asc", - }); - - // Re-picking the active dimension flips it, which is the gesture most sortable - // tables use; picking a different one starts from that dimension's default. - const selectSortKey = (nextKey: SortKey): void => { - if (nextKey === sortKey) { - toggleDirection(); - return; - } - onSortChange({ - sortKey: nextKey, - direction: getDefaultDirectionForKey(nextKey), - }); - }; - return ( -
+
+
Sort by
{SORT_OPTIONS.map((option) => ( selectSortKey(option.key)} + onSelect={() => + onSortChange({ + sortKey: option.key, + // Switching dimension starts from that dimension's own default; + // re-picking the active one leaves the direction alone. + direction: + option.key === sortKey + ? sortDirection + : getDefaultDirectionForKey(option.key), + }) + } /> ))}
-
- -
+ onSortChange({ sortKey, direction })} + sortDirection={sortDirection} + sortKey={sortKey} + /> ); }; diff --git a/apps/obsidian/src/components/SearchDropdown.tsx b/apps/obsidian/src/components/SearchDropdown.tsx index 8f472ef48..f75db7258 100644 --- a/apps/obsidian/src/components/SearchDropdown.tsx +++ b/apps/obsidian/src/components/SearchDropdown.tsx @@ -28,7 +28,6 @@ export const SearchDropdown = ({ onOpenChange, panelClassName = "w-64", title, - triggerLabel, }: { app: App; ariaLabel: string; @@ -43,9 +42,6 @@ export const SearchDropdown = ({ onOpenChange: (isOpen: boolean) => void; panelClassName?: string; title: string; - /** Optional text beside the icon, for controls whose current value is worth - showing without opening the panel. */ - triggerLabel?: string; }): ReactElement => { const containerRef = useRef(null); @@ -107,12 +103,9 @@ export const SearchDropdown = ({ // Keeps focus in the search input, so arrow and Enter navigation stays // live while the panel is open. onMouseDown={(event) => event.preventDefault()} - className={`clickable-icon relative ${triggerLabel ? "gap-1" : ""} ${ - isOpen || isActive ? "is-active" : "" - }`} + className={`clickable-icon relative ${isOpen || isActive ? "is-active" : ""}`} > - {triggerLabel && {triggerLabel}} {badgeCount > 0 && ( Date: Wed, 19 Aug 2026 22:34:28 -0400 Subject: [PATCH 24/29] ENG-2112 Address review: drive dropdown styling from Obsidian variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The panel shadow and badge foreground were hardcoded, against the rule in apps/obsidian/AGENTS.md. Both now come from Obsidian: the badge uses the mapped `text-on-accent` token, and the shadow uses `--shadow-s`. The shadow needs Tailwind's `shadow:` type hint. Without it, `shadow-[var(--shadow-s)]` is ambiguous and Tailwind resolves it as a shadow *colour* — it emits `--tw-shadow-color` and no `box-shadow` at all, so the class is inert and the panel renders with no shadow. `ModifyNodeModal.tsx:500` has the same silently-broken usage; not touched here. Verified over CDP: the panel's computed box-shadow now ends with exactly the layers a probe element carrying `var(--shadow-s)` produces. 30/30. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/components/SearchDropdown.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/obsidian/src/components/SearchDropdown.tsx b/apps/obsidian/src/components/SearchDropdown.tsx index f75db7258..b4711fc78 100644 --- a/apps/obsidian/src/components/SearchDropdown.tsx +++ b/apps/obsidian/src/components/SearchDropdown.tsx @@ -109,7 +109,7 @@ export const SearchDropdown = ({ {badgeCount > 0 && ( {badgeCount} @@ -117,7 +117,7 @@ export const SearchDropdown = ({ {isOpen && (
{children}
From b7a106db86a7961299db4220d3888fda44c436f5 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 19 Aug 2026 22:40:34 -0400 Subject: [PATCH 25/29] ENG-2112 Reduce comments to one line each Every comment added on this branch is now a single line. Behaviour unchanged; 30/30 still passing. Co-Authored-By: Claude Opus 5 --- .../src/components/NodeSearchModal.tsx | 14 +++----- apps/obsidian/src/components/NodeSortMenu.tsx | 17 +++------ apps/obsidian/src/components/ObsidianIcon.tsx | 6 +--- .../src/components/SearchDropdown.tsx | 36 ++++--------------- .../obsidian/src/utils/discourseNodeAuthor.ts | 35 +++--------------- apps/obsidian/src/utils/discourseNodeSort.ts | 34 ++++-------------- 6 files changed, 26 insertions(+), 116 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 0a66effdf..91a2d66c1 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -271,9 +271,7 @@ const NodeSearch = ({ // The single source of truth for active type filters: F6's chips will read and // write this same state, so either surface can manage them. const [selectedNodeTypeIds, setSelectedNodeTypeIds] = useState([]); - // One value rather than a boolean per control, so two toolbar panels can never - // be open at once. Each control still owns its own Escape, via the keymap - // scope it pushes while open. + // One value per toolbar, so two panels can never be open at once. const [openDropdown, setOpenDropdown] = useState(null); const [sortKey, setSortKey] = useState(DEFAULT_SORT_KEY); const [sortDirection, setSortDirection] = useState( @@ -326,9 +324,7 @@ const NodeSearch = ({ return () => window.clearTimeout(timeout); }, [query]); - // Sorting runs on the whole ranked list and the truncation comes after, so a - // date or alphabetical sort covers every match rather than reordering the - // best-matching 50. + // Sort before truncating, so a date or alphabetical sort covers every match. const results = useMemo(() => { if (candidateState.status !== "ready") return []; const ranked = rankDiscourseNodesByTitle({ @@ -336,8 +332,7 @@ const NodeSearch = ({ query: debouncedQuery, nodeTypeIds: selectedNodeTypeIds, }); - // Only the author sort needs a name per row, and resolving the full list is - // wasted work on every other keystroke. + // Only the author sort needs a name per row. const authorNameByPath = sortKey === "author" ? buildAuthorNameByPath({ @@ -456,8 +451,7 @@ const NodeSearch = ({ // Bound here rather than on the input so navigation survives focus moving // elsewhere in the modal, and so result actions have one place to live.
- {/* Padded so a trigger's count badge, which sits outside the button box, - is not clipped by the modal's overflow-hidden content. */} + {/* Padded so a trigger's count badge is not clipped by the modal's overflow-hidden content. */} {/* Top-aligned: the field grows downwards, so the triggers stay on its first line. */}
direction === "asc" ? "arrow-up-narrow-wide" : "arrow-down-wide-narrow"; -/** - * Rows are divs rather than buttons: Obsidian's button chrome is a filled, - * padded control, which reads as five separate widgets stacked in a panel - * instead of a menu. Flat rows on the panel's own background, reacting only to - * hover, are what a menu looks like in both Obsidian and Roam. - */ +/** Rows are divs, not buttons: Obsidian's button chrome reads as separate widgets rather than a menu. */ const SortOptionRow = ({ isSelected, label, @@ -39,8 +34,7 @@ const SortOptionRow = ({ role="menuitemradio" aria-checked={isSelected} onClick={onSelect} - // Keeps focus in the search input, so the result list stays keyboard-driven - // while the panel is open. + // Keeps focus in the search input, so the result list stays keyboard-driven. onMouseDown={(event) => event.preventDefault()} className={`flex cursor-pointer items-center gap-2 px-3 py-1.5 text-sm ${ isSelected @@ -71,9 +65,7 @@ const DirectionToggle = ({ key={direction} role="button" aria-pressed={direction === sortDirection} - // The wording of a direction depends on the dimension — "newest first" - // and "A to Z" are the same direction — so the full phrase is the title - // rather than the label. + // A direction's wording depends on the dimension, so the full phrase is the title. title={getSortDirectionLabel({ sortKey, direction })} onClick={() => onSelect(direction)} onMouseDown={(event) => event.preventDefault()} @@ -131,8 +123,7 @@ export const NodeSortMenu = ({ onSelect={() => onSortChange({ sortKey: option.key, - // Switching dimension starts from that dimension's own default; - // re-picking the active one leaves the direction alone. + // Switching dimension uses that dimension's default; re-picking keeps the direction. direction: option.key === sortKey ? sortDirection diff --git a/apps/obsidian/src/components/ObsidianIcon.tsx b/apps/obsidian/src/components/ObsidianIcon.tsx index 0f91d529b..e665e23ed 100644 --- a/apps/obsidian/src/components/ObsidianIcon.tsx +++ b/apps/obsidian/src/components/ObsidianIcon.tsx @@ -1,11 +1,7 @@ import { setIcon } from "obsidian"; import type { ReactElement } from "react"; -/** - * Renders one of Obsidian's built-in icons. The host node is emptied first - * because React reuses it across renders and `setIcon` appends rather than - * replaces. - */ +/** Obsidian icon; the node is emptied first because `setIcon` appends and React reuses it. */ export const ObsidianIcon = ({ name, className = "flex items-center", diff --git a/apps/obsidian/src/components/SearchDropdown.tsx b/apps/obsidian/src/components/SearchDropdown.tsx index b4711fc78..790592004 100644 --- a/apps/obsidian/src/components/SearchDropdown.tsx +++ b/apps/obsidian/src/components/SearchDropdown.tsx @@ -2,18 +2,9 @@ import { App, Scope } from "obsidian"; import { useEffect, useRef, type ReactElement, type ReactNode } from "react"; import { ObsidianIcon } from "~/components/ObsidianIcon"; -/** - * The trigger-plus-panel shell shared by the search modal's toolbar controls. - * Every one of them has to solve the same three problems — closing on an - * outside click, keeping its keystrokes away from the modal's result - * navigation, and closing on Escape without taking the modal with it — so the - * answers live here once. - */ +/** Trigger-plus-panel shell shared by the search modal's toolbar controls. */ -/** - * Which toolbar panel is open, or null. One value rather than a boolean per - * control, so two panels can never be open at the same time. - */ +/** Which toolbar panel is open, so two can never be open at once. */ export type SearchDropdownId = "type-filter" | "sort" | null; export const SearchDropdown = ({ @@ -45,15 +36,7 @@ export const SearchDropdown = ({ }): ReactElement => { const containerRef = useRef(null); - /** - * Escape cannot be taken from the DOM. Obsidian's Modal registers its - * close-on-Escape before any plugin React tree exists, so a listener added - * later runs second at every phase and on every node — even a capture - * listener on `window` — by which point the modal is already closing. - * Registering on the modal's own scope does not help either: the built-in - * handler was registered first and wins. Pushing a scope puts this above the - * modal in the stack, which is the one place that gets Escape first. - */ + // Obsidian's modal Escape is registered before React exists, and wins on its own scope too, so only a pushed scope gets it first. useEffect(() => { if (!isOpen) return; const scope = new Scope(); @@ -65,8 +48,7 @@ export const SearchDropdown = ({ return () => app.keymap.popScope(scope); }, [app, isOpen, onOpenChange]); - // `activeDocument` rather than `document`, so the listener lands in whichever - // window holds the modal when Obsidian is running a popout. + // `activeDocument`, so the listener lands in the popout window holding the modal. useEffect(() => { if (!isOpen) return; const handlePointerDown = (event: MouseEvent) => { @@ -84,12 +66,7 @@ export const SearchDropdown = ({ className="relative shrink-0" onKeyDown={(event) => { if (!isOpen) return; - // Every keystroke stops here while the panel is open. The modal's handler - // is an ancestor and reads Enter as "open the highlighted result" and the - // arrows as "move the selection", so typing in a panel would otherwise - // open a note and close the whole modal. Escape is deliberately not - // handled here — it never reaches React, and the modal's keymap scope - // closes the panel instead. + // Panel keystrokes must not reach the modal's Enter and arrow result navigation; Escape never arrives here at all. event.stopPropagation(); }} > @@ -100,8 +77,7 @@ export const SearchDropdown = ({ disabled={isDisabled} title={title} onClick={() => onOpenChange(!isOpen)} - // Keeps focus in the search input, so arrow and Enter navigation stays - // live while the panel is open. + // Keeps focus in the search input, so arrow and Enter navigation stays live. onMouseDown={(event) => event.preventDefault()} className={`clickable-icon relative ${isOpen || isActive ? "is-active" : ""}`} > diff --git a/apps/obsidian/src/utils/discourseNodeAuthor.ts b/apps/obsidian/src/utils/discourseNodeAuthor.ts index 97b84033e..dfba756ee 100644 --- a/apps/obsidian/src/utils/discourseNodeAuthor.ts +++ b/apps/obsidian/src/utils/discourseNodeAuthor.ts @@ -5,17 +5,7 @@ import type { DiscourseNodeCandidate } from "~/services/QueryEngine"; import { fetchUserNames } from "~/utils/importNodes"; import { getLoggedInClient } from "~/utils/supabaseContext"; -/** - * Author resolution for discourse node search. This is the single place that - * turns a note into a display name, so the preview header and the author sort - * can never disagree about who wrote something. - * - * Known limitation: Obsidian Sync is not a source here. The 1.8.7 typings - * expose no sync, user, or collaborator API, so per-file attribution from Sync - * cannot be read by a plugin. Names therefore come from the `authorId` - * frontmatter written by the importer, resolved against the ids cached in - * settings. - */ +/** Single source for a note's author name. Obsidian exposes no Sync user API, so names come from `authorId` frontmatter. */ export const LOCAL_AUTHOR_NAME = "You"; export const UNRESOLVED_AUTHOR_NAME = "Unknown"; @@ -28,12 +18,7 @@ const getFrontmatterAuthorId = (app: App, file: TFile): unknown => { return frontmatter?.authorId; }; -/** - * "You" belongs only to a note with no `authorId` at all — every note in an - * unsynced vault. An id that is present but unresolvable stays "Unknown" rather - * than claiming local authorship. `useAuthorNames` has already cached the - * names, so this stays synchronous. - */ +/** "You" only when there is no `authorId`; a present but unresolvable id stays "Unknown". */ export const resolveAuthorName = ({ app, file, @@ -49,17 +34,11 @@ export const resolveAuthorName = ({ return userNames[authorId] ?? UNRESOLVED_AUTHOR_NAME; }; -/** - * A name that identifies no one. Sorting keeps these out of the alphabetical - * run so an unreadable id never lands between two real authors. - */ +/** A name that identifies no one, kept out of the alphabetical run. */ export const isUnattributedAuthorName = (authorName: string): boolean => authorName === UNRESOLVED_AUTHOR_NAME; -/** - * Author sort needs a name for every candidate, not just the previewed one, so - * the whole list is resolved up front and keyed by path. - */ +/** Author sort needs a name for every candidate, not just the previewed one. */ export const buildAuthorNameByPath = ({ app, files, @@ -76,11 +55,7 @@ export const buildAuthorNameByPath = ({ return byPath; }; -/** - * `fetchUserNames` returns every person in the vault's spaces in one query, so - * this refreshes once per open when a name is missing rather than querying per - * author. - */ +/** One query returns every person, so this refreshes once per open when a name is missing. */ export const useAuthorNames = ({ app, plugin, diff --git a/apps/obsidian/src/utils/discourseNodeSort.ts b/apps/obsidian/src/utils/discourseNodeSort.ts index 61e4ecad4..6ddf66a31 100644 --- a/apps/obsidian/src/utils/discourseNodeSort.ts +++ b/apps/obsidian/src/utils/discourseNodeSort.ts @@ -1,13 +1,7 @@ import { TFile } from "obsidian"; import { isUnattributedAuthorName } from "~/utils/discourseNodeAuthor"; -/** - * Client-side ordering for discourse node search results. Sorting runs over the - * full ranked list before it is truncated for display — sorting a - * relevance-truncated window would show the alphabetically-first 50 of the - * best-matching 50, which is not what any of these options mean. - * Ported from Roam's advanced search so both apps offer the same dimensions. - */ +/** Client-side result ordering, over the full ranked list before display truncation. Mirrors Roam's advanced search. */ export type SortKey = | "relevance" @@ -30,10 +24,7 @@ export const DEFAULT_SORT_KEY: SortKey = "relevance"; /** Descending reads as "best first" for scores and "newest first" for dates. */ export const DEFAULT_SORT_DIRECTION: SortDirection = "desc"; -/** - * Structural rather than tied to `RankedDiscourseNode`, so the sort can run on - * the ranked list or on rows that have already been decorated for display. - */ +/** Structural, so the sort runs on ranked results or decorated rows alike. */ export type SortableSearchResult = { file: TFile; title: string; @@ -56,11 +47,7 @@ export const getSortDirectionLabel = ({ direction: SortDirection; }): string => DIRECTION_LABELS[sortKey][direction]; -/** - * Switching dimension resets the direction, because "descending" means - * something different per dimension: keeping it would turn a switch to - * alphabetical into an unasked-for Z-to-A. - */ +/** "Descending" means something different per dimension, so switching resets it. */ export const getDefaultDirectionForKey = (sortKey: SortKey): SortDirection => sortKey === "title" || sortKey === "author" ? "asc" : "desc"; @@ -84,12 +71,7 @@ const getAuthorName = ({ authorNameByPath: Map | undefined; }): string => authorNameByPath?.get(result.file.path) ?? ""; -/** - * Unattributed notes sit after every named author in both directions, so - * reversing the sort never buries the readable names under a block of - * "Unknown". Returns 0 when the two sides agree, leaving the ordering to the - * name comparison. - */ +/** Unattributed notes sit after every named author, in both directions. */ const compareUnattributedLast = ({ a, b, @@ -125,8 +107,7 @@ const compareAscending = ({ if (sortKey === "dateCreated") return a.file.stat.ctime - b.file.stat.ctime; if (sortKey === "dateModified") return a.file.stat.mtime - b.file.stat.mtime; - // Same-author notes end up adjacent, and the title breaks the tie inside each - // group so the order is stable rather than left to vault iteration. + // Title breaks the tie inside an author group, so the order is not vault order. const authorDelta = getAuthorName({ result: a, authorNameByPath, @@ -134,10 +115,7 @@ const compareAscending = ({ return authorDelta !== 0 ? authorDelta : a.title.localeCompare(b.title); }; -/** - * `authorNameByPath` is only needed for the author sort; the other keys ignore - * it so callers can skip resolving names for the whole list. - */ +/** `authorNameByPath` is only needed for the author sort. */ export const sortSearchResults = ({ results, sortKey, From 45b670a85d04947548c15ef3d184e5aae219384d Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Thu, 20 Aug 2026 12:49:26 -0400 Subject: [PATCH 26/29] ENG-2112 Migrate the type filter onto SearchDropdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacking on ENG-2111 makes the deduplication its own commit message deferred possible. NodeTypeFilterMenu keeps its rows, colours and type search and gives up everything it shared with the sort menu: the trigger button and count badge, the pushed Escape scope, the outside-mousedown close, the keystroke containment, the positioned panel container, and its local FilterIcon copy. 291 lines down to 220. Two fixes fall out of it. The panel container it dropped had the hardcoded `shadow-[0_4px_12px_rgba(0,0,0,0.15)]`, so the filter now takes its shadow from `--shadow-s` like everything else. And `isTypeFilterOpen` becomes `openDropdown === "type-filter"`, so the filter and sort panels can no longer be open simultaneously. Verified over CDP, 35/35, with five assertions added for what this stack now covers: neither panel can be open while the other is, Escape still spares the modal after the migration, the trigger badges its count, and all five sort options honour an active type filter — 16 rows of the filtered type, none off type, identical under every dimension. That last one was the Done When criterion ENG-2109 could not exercise. Co-Authored-By: Claude Opus 5 --- .../src/components/NodeTypeFilterMenu.tsx | 137 +++++------------- 1 file changed, 33 insertions(+), 104 deletions(-) diff --git a/apps/obsidian/src/components/NodeTypeFilterMenu.tsx b/apps/obsidian/src/components/NodeTypeFilterMenu.tsx index 1c166ed41..e6f9e95a6 100644 --- a/apps/obsidian/src/components/NodeTypeFilterMenu.tsx +++ b/apps/obsidian/src/components/NodeTypeFilterMenu.tsx @@ -1,5 +1,6 @@ -import { App, Scope, setIcon } from "obsidian"; +import { App } from "obsidian"; import { useEffect, useMemo, useRef, useState, type ReactElement } from "react"; +import { SearchDropdown } from "~/components/SearchDropdown"; import { DiscourseNode } from "~/types"; import { getAllDiscourseNodeColors } from "~/utils/colorUtils"; import { @@ -10,19 +11,6 @@ import { toPanelSelectedIds, } from "~/utils/discourseNodeTypeFilter"; -const FilterIcon = ({ name }: { name: string }): ReactElement => ( - // Emptied first because React reuses the node across renders and `setIcon` - // appends rather than replaces. - { - if (!el) return; - el.empty(); - setIcon(el, name); - }} - /> -); - const NodeTypeFilterRow = ({ color, isChecked, @@ -112,7 +100,7 @@ const NodeTypeFilterPanel = ({ }; return ( -
+ <> {/* Clearing is the only thing this control ever does, so it says so and appears only when there is a filter to clear. A "select all" checkbox would sit checked-and-inert whenever no filter is active, since an empty @@ -159,7 +147,7 @@ const NodeTypeFilterPanel = ({ )) )}
-
+ ); }; @@ -178,8 +166,6 @@ export const NodeTypeFilterMenu = ({ onSelectedNodeTypeIdsChange: (ids: string[]) => void; selectedNodeTypeIds: string[]; }): ReactElement => { - const containerRef = useRef(null); - const allTypeIds = useMemo( () => nodeTypes.map((nodeType) => nodeType.id), [nodeTypes], @@ -196,96 +182,39 @@ export const NodeTypeFilterMenu = ({ [allTypeIds, selectedNodeTypeIds], ); - // Escape cannot be intercepted from the DOM. Obsidian registers the Modal's - // close-on-Escape before any plugin React tree exists, so a listener added - // later always runs second — preventDefault plus stopImmediatePropagation in a - // React handler, a capture-phase window listener, and registering on the - // Modal's own scope all fail, the last because Scope resolves in registration - // order. A pushed scope is the only thing that lands above the modal. - useEffect(() => { - if (!isOpen) return; - const scope = new Scope(); - scope.register([], "Escape", () => { - onOpenChange(false); - return false; - }); - app.keymap.pushScope(scope); - return () => app.keymap.popScope(scope); - }, [app, isOpen, onOpenChange]); - - // `activeDocument` rather than `document`, so the listener lands in whichever - // window holds the modal when Obsidian is running a popout. - useEffect(() => { - if (!isOpen) return; - const handlePointerDown = (event: MouseEvent) => { - if (containerRef.current?.contains(event.target as Node)) return; - onOpenChange(false); - }; - activeDocument.addEventListener("mousedown", handlePointerDown, true); - return () => - activeDocument.removeEventListener("mousedown", handlePointerDown, true); - }, [isOpen, onOpenChange]); - const activeFilterCount = isFilterActive ? selectedNodeTypeIds.length : 0; return ( -
{ - if (!isOpen) return; - // Every keystroke stops here while the panel is open. The modal's handler - // is an ancestor and reads Enter as "open the highlighted result" and the - // arrows as "move the selection", so typing in the type search would - // otherwise open a note and close the whole modal. Escape is not handled - // here because it never reaches the DOM — see the pushed scope above. - event.stopPropagation(); - }} + 0 + ? `Filter by type, ${activeFilterCount} selected` + : "Filter by type" + } + badgeCount={activeFilterCount} + iconName="filter" + isActive={isFilterActive} + isDisabled={nodeTypes.length === 0} + isOpen={isOpen} + onOpenChange={onOpenChange} + panelClassName="w-64" + title={ + nodeTypes.length === 0 + ? "No discourse node types configured" + : "Filter by type" + } > - - {isOpen && ( - - onSelectedNodeTypeIdsChange( - fromPanelSelectedIds({ panelSelectedIds: panelIds, allTypeIds }), - ) - } - selectedIds={panelSelectedIds} - /> - )} -
+ selectedIds={panelSelectedIds} + /> + ); }; From 65856ae91d4efe09105006c4c04ceb2dc29fe6ac Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Fri, 21 Aug 2026 12:06:13 -0400 Subject: [PATCH 27/29] ENG-2112 Drop ObsidianIcon and inline setIcon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repo already had a pattern for this — every other call site writes `ref={(el) => (el && setIcon(el, "name")) || undefined}` inline — so the wrapper was a new abstraction over three call sites rather than reuse. The one behaviour worth keeping is the empty-before-write, and only for SearchDropdown's trigger, whose icon name changes with the control's state: `setIcon` appends and React reuses the host node, so without it the arrows stack up. The sort menu's check and direction icons have a name fixed per element, so they use the repo's one-liner. 36/36, with a new assertion that the trigger still holds exactly one icon after repeated direction changes. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/components/NodeSortMenu.tsx | 14 +++++++++---- apps/obsidian/src/components/ObsidianIcon.tsx | 20 ------------------- .../src/components/SearchDropdown.tsx | 13 +++++++++--- 3 files changed, 20 insertions(+), 27 deletions(-) delete mode 100644 apps/obsidian/src/components/ObsidianIcon.tsx diff --git a/apps/obsidian/src/components/NodeSortMenu.tsx b/apps/obsidian/src/components/NodeSortMenu.tsx index 17db8d728..196bc1920 100644 --- a/apps/obsidian/src/components/NodeSortMenu.tsx +++ b/apps/obsidian/src/components/NodeSortMenu.tsx @@ -1,6 +1,5 @@ -import { App } from "obsidian"; +import { App, setIcon } from "obsidian"; import type { ReactElement } from "react"; -import { ObsidianIcon } from "~/components/ObsidianIcon"; import { SearchDropdown } from "~/components/SearchDropdown"; import { SORT_OPTIONS, @@ -44,7 +43,9 @@ const SortOptionRow = ({ > {/* Always occupies its slot, so selecting an option does not shift the labels. */} - {isSelected && } + {isSelected && ( + (el && setIcon(el, "check")) || undefined} /> + )} {label}
@@ -75,7 +76,12 @@ const DirectionToggle = ({ : "text-normal hover:bg-modifier-hover" }`} > - + + (el && setIcon(el, getDirectionIconName(direction))) || undefined + } + /> {label}
))} diff --git a/apps/obsidian/src/components/ObsidianIcon.tsx b/apps/obsidian/src/components/ObsidianIcon.tsx deleted file mode 100644 index e665e23ed..000000000 --- a/apps/obsidian/src/components/ObsidianIcon.tsx +++ /dev/null @@ -1,20 +0,0 @@ -import { setIcon } from "obsidian"; -import type { ReactElement } from "react"; - -/** Obsidian icon; the node is emptied first because `setIcon` appends and React reuses it. */ -export const ObsidianIcon = ({ - name, - className = "flex items-center", -}: { - name: string; - className?: string; -}): ReactElement => ( - { - if (!el) return; - el.empty(); - setIcon(el, name); - }} - /> -); diff --git a/apps/obsidian/src/components/SearchDropdown.tsx b/apps/obsidian/src/components/SearchDropdown.tsx index 790592004..d837d00be 100644 --- a/apps/obsidian/src/components/SearchDropdown.tsx +++ b/apps/obsidian/src/components/SearchDropdown.tsx @@ -1,6 +1,5 @@ -import { App, Scope } from "obsidian"; +import { App, Scope, setIcon } from "obsidian"; import { useEffect, useRef, type ReactElement, type ReactNode } from "react"; -import { ObsidianIcon } from "~/components/ObsidianIcon"; /** Trigger-plus-panel shell shared by the search modal's toolbar controls. */ @@ -81,7 +80,15 @@ export const SearchDropdown = ({ onMouseDown={(event) => event.preventDefault()} className={`clickable-icon relative ${isOpen || isActive ? "is-active" : ""}`} > - + { + if (!el) return; + el.empty(); + setIcon(el, iconName); + }} + /> {badgeCount > 0 && ( Date: Fri, 21 Aug 2026 17:51:35 -0400 Subject: [PATCH 28/29] ENG-2112 Annotate the frontmatter read instead of asserting it CI's eslint flagged the cast as an unnecessary assertion: `FrontMatterCache` indexes to `any`, so asserting it to `Record | undefined` does not change the type. Annotating the local instead keeps the read typed as `unknown` for the caller and leaves nothing for the rule to fire on. Carried over verbatim from ENG-2109 when this function moved into its own util, which is why it surfaced here. 36/36 unchanged. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/utils/discourseNodeAuthor.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/obsidian/src/utils/discourseNodeAuthor.ts b/apps/obsidian/src/utils/discourseNodeAuthor.ts index dfba756ee..e646247d5 100644 --- a/apps/obsidian/src/utils/discourseNodeAuthor.ts +++ b/apps/obsidian/src/utils/discourseNodeAuthor.ts @@ -12,9 +12,10 @@ export const UNRESOLVED_AUTHOR_NAME = "Unknown"; /** Frontmatter is untyped, so the raw value is narrowed by each caller. */ const getFrontmatterAuthorId = (app: App, file: TFile): unknown => { - const frontmatter = app.metadataCache.getFileCache(file)?.frontmatter as - | Record - | undefined; + // Annotated rather than asserted: `FrontMatterCache` indexes to `any`, so the + // cast was a no-op, and this keeps the read typed as `unknown`. + const frontmatter: Record | undefined = + app.metadataCache.getFileCache(file)?.frontmatter; return frontmatter?.authorId; }; From a4c0e6b5024df8eb52657bfd3644b8f4c15055d6 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sat, 22 Aug 2026 00:04:07 -0400 Subject: [PATCH 29/29] ENG-2112 Keep only comments that explain a decision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cuts the eleven comments that restated the code they sat above — prop descriptions the identifier already gave, a label for the shell's own file, the ternary conditions in the sort menu and the results memo. What survives is what someone could otherwise get wrong: the Obsidian gotchas (Escape needs a pushed scope, `activeDocument` for popouts, `setIcon` appends), the invariants (sort precedes truncation, unattributed last in both directions, the partition sits outside the direction flip), and the deliberate deviations (rows are divs, not buttons; the frontmatter read is annotated, not asserted). 24 comments down to 13. 36/36 unchanged. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/components/NodeSearchModal.tsx | 1 - apps/obsidian/src/components/NodeSortMenu.tsx | 3 --- apps/obsidian/src/components/SearchDropdown.tsx | 4 ---- apps/obsidian/src/utils/discourseNodeAuthor.ts | 6 +----- apps/obsidian/src/utils/discourseNodeSort.ts | 2 -- 5 files changed, 1 insertion(+), 15 deletions(-) diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 91a2d66c1..f71142cbb 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -332,7 +332,6 @@ const NodeSearch = ({ query: debouncedQuery, nodeTypeIds: selectedNodeTypeIds, }); - // Only the author sort needs a name per row. const authorNameByPath = sortKey === "author" ? buildAuthorNameByPath({ diff --git a/apps/obsidian/src/components/NodeSortMenu.tsx b/apps/obsidian/src/components/NodeSortMenu.tsx index 196bc1920..5f29a8cba 100644 --- a/apps/obsidian/src/components/NodeSortMenu.tsx +++ b/apps/obsidian/src/components/NodeSortMenu.tsx @@ -33,7 +33,6 @@ const SortOptionRow = ({ role="menuitemradio" aria-checked={isSelected} onClick={onSelect} - // Keeps focus in the search input, so the result list stays keyboard-driven. onMouseDown={(event) => event.preventDefault()} className={`flex cursor-pointer items-center gap-2 px-3 py-1.5 text-sm ${ isSelected @@ -66,7 +65,6 @@ const DirectionToggle = ({ key={direction} role="button" aria-pressed={direction === sortDirection} - // A direction's wording depends on the dimension, so the full phrase is the title. title={getSortDirectionLabel({ sortKey, direction })} onClick={() => onSelect(direction)} onMouseDown={(event) => event.preventDefault()} @@ -129,7 +127,6 @@ export const NodeSortMenu = ({ onSelect={() => onSortChange({ sortKey: option.key, - // Switching dimension uses that dimension's default; re-picking keeps the direction. direction: option.key === sortKey ? sortDirection diff --git a/apps/obsidian/src/components/SearchDropdown.tsx b/apps/obsidian/src/components/SearchDropdown.tsx index d837d00be..818b4c7a4 100644 --- a/apps/obsidian/src/components/SearchDropdown.tsx +++ b/apps/obsidian/src/components/SearchDropdown.tsx @@ -1,8 +1,6 @@ import { App, Scope, setIcon } from "obsidian"; import { useEffect, useRef, type ReactElement, type ReactNode } from "react"; -/** Trigger-plus-panel shell shared by the search modal's toolbar controls. */ - /** Which toolbar panel is open, so two can never be open at once. */ export type SearchDropdownId = "type-filter" | "sort" | null; @@ -21,11 +19,9 @@ export const SearchDropdown = ({ }: { app: App; ariaLabel: string; - /** Rendered as a superscript count on the trigger when above zero. */ badgeCount?: number; children: ReactNode; iconName: string; - /** Highlights the trigger to show the control is doing something. */ isActive: boolean; isDisabled?: boolean; isOpen: boolean; diff --git a/apps/obsidian/src/utils/discourseNodeAuthor.ts b/apps/obsidian/src/utils/discourseNodeAuthor.ts index e646247d5..57efa0fb9 100644 --- a/apps/obsidian/src/utils/discourseNodeAuthor.ts +++ b/apps/obsidian/src/utils/discourseNodeAuthor.ts @@ -12,8 +12,7 @@ export const UNRESOLVED_AUTHOR_NAME = "Unknown"; /** Frontmatter is untyped, so the raw value is narrowed by each caller. */ const getFrontmatterAuthorId = (app: App, file: TFile): unknown => { - // Annotated rather than asserted: `FrontMatterCache` indexes to `any`, so the - // cast was a no-op, and this keeps the read typed as `unknown`. + // Annotated rather than asserted: `FrontMatterCache` indexes to `any`, so a cast is a no-op. const frontmatter: Record | undefined = app.metadataCache.getFileCache(file)?.frontmatter; return frontmatter?.authorId; @@ -35,11 +34,9 @@ export const resolveAuthorName = ({ return userNames[authorId] ?? UNRESOLVED_AUTHOR_NAME; }; -/** A name that identifies no one, kept out of the alphabetical run. */ export const isUnattributedAuthorName = (authorName: string): boolean => authorName === UNRESOLVED_AUTHOR_NAME; -/** Author sort needs a name for every candidate, not just the previewed one. */ export const buildAuthorNameByPath = ({ app, files, @@ -64,7 +61,6 @@ export const useAuthorNames = ({ }: { app: App; plugin: DiscourseGraphPlugin; - /** Null until the candidate load finishes; nothing to resolve before then. */ candidates: DiscourseNodeCandidate[] | null; }): Record => { const [userNames, setUserNames] = useState(plugin.settings.userNames ?? {}); diff --git a/apps/obsidian/src/utils/discourseNodeSort.ts b/apps/obsidian/src/utils/discourseNodeSort.ts index 6ddf66a31..cd1b9b737 100644 --- a/apps/obsidian/src/utils/discourseNodeSort.ts +++ b/apps/obsidian/src/utils/discourseNodeSort.ts @@ -21,7 +21,6 @@ export const SORT_OPTIONS: { key: SortKey; label: string }[] = [ ]; export const DEFAULT_SORT_KEY: SortKey = "relevance"; -/** Descending reads as "best first" for scores and "newest first" for dates. */ export const DEFAULT_SORT_DIRECTION: SortDirection = "desc"; /** Structural, so the sort runs on ranked results or decorated rows alike. */ @@ -107,7 +106,6 @@ const compareAscending = ({ if (sortKey === "dateCreated") return a.file.stat.ctime - b.file.stat.ctime; if (sortKey === "dateModified") return a.file.stat.mtime - b.file.stat.mtime; - // Title breaks the tie inside an author group, so the order is not vault order. const authorDelta = getAuthorName({ result: a, authorNameByPath,