From 6892a51afb4f1b266ec4aa6dc71ed8592ebe7106 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 23 Aug 2026 15:37:56 -0400 Subject: [PATCH 1/2] ENG-2112 Add sort button with sort dropdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five client-side sort dimensions over the search result list — relevance, alphabetical, date created, date modified, author — with Asc/Desc controls, mirroring Roam's sort menu (ENG-1732). Sorting runs over the full ranked list and the 50-row truncation comes after it; sorting the truncated window would have shown the alphabetically-first 50 of the best-matching 50. Author resolution moves out of NodeSearchModal into discourseNodeAuthor with its behaviour unchanged, and unattributed notes are pinned last in both directions. The trigger-plus-panel shell lives in SearchDropdown rather than inside the sort menu, and NodeTypeFilterMenu now renders through it — dropping its duplicate trigger, badge, Escape scope, outside-click, keystroke containment and panel container, along with the hardcoded shadow that container carried. isTypeFilterOpen becomes a shared openDropdown, so the two panels can no longer be open at once. Escape reaches the panel only through a pushed keymap scope: Obsidian registers the Modal's close-on-Escape before any plugin React tree exists, so a DOM listener added later always runs second, and registering on the Modal's own scope loses to the built-in handler. Rebased from the ENG-2109/2110/2111 stack onto main after all three merged. The eight commits from that stack's lifetime are collapsed into one, because replaying them over the squash-merged base meant re-resolving the same NodeSearchModal regions eight times; the resolved tree is byte-identical to the pre-rebase branch except for one comment main revised on its own. Verified over CDP: 30/30. Two assertions skipped for want of data in the available vault — full-list-vs-truncated-window needs over 50 matches, and unattributed-last needs a note whose authorId does not resolve. Co-Authored-By: Claude Opus 5 --- .../src/components/NodeSearchModal.tsx | 169 +++++++++--------- apps/obsidian/src/components/NodeSortMenu.tsx | 146 +++++++++++++++ .../src/components/NodeTypeFilterMenu.tsx | 137 ++++---------- .../src/components/SearchDropdown.tsx | 106 +++++++++++ .../obsidian/src/utils/discourseNodeAuthor.ts | 93 ++++++++++ apps/obsidian/src/utils/discourseNodeSort.ts | 140 +++++++++++++++ 6 files changed, 599 insertions(+), 192 deletions(-) create mode 100644 apps/obsidian/src/components/NodeSortMenu.tsx create mode 100644 apps/obsidian/src/components/SearchDropdown.tsx create mode 100644 apps/obsidian/src/utils/discourseNodeAuthor.ts create mode 100644 apps/obsidian/src/utils/discourseNodeSort.ts diff --git a/apps/obsidian/src/components/NodeSearchModal.tsx b/apps/obsidian/src/components/NodeSearchModal.tsx index 7ef5bbadd..7afd2759d 100644 --- a/apps/obsidian/src/components/NodeSearchModal.tsx +++ b/apps/obsidian/src/components/NodeSearchModal.tsx @@ -21,8 +21,10 @@ import { import { createRoot, Root } from "react-dom/client"; import type DiscourseGraphPlugin from "~/index"; import { NodeSearchFooter } from "~/components/NodeSearchFooter"; +import { NodeSortMenu } from "~/components/NodeSortMenu"; import { NodeTypeChipsSearchInput } from "~/components/NodeTypeChipsSearchInput"; import { NodeTypeFilterMenu } from "~/components/NodeTypeFilterMenu"; +import type { SearchDropdownId } from "~/components/SearchDropdown"; import { openFileInNewLeaf, openFileInNewTab, @@ -43,8 +45,18 @@ import { getFallbackNodeTypeBadge, type NodeTypeBadge, } from "~/utils/nodeTypeBadge"; -import { fetchUserNames } from "~/utils/importNodes"; -import { getLoggedInClient } from "~/utils/supabaseContext"; +import { + buildAuthorNameByPath, + resolveAuthorName, + useAuthorNames, +} from "~/utils/discourseNodeAuthor"; +import { + DEFAULT_SORT_DIRECTION, + DEFAULT_SORT_KEY, + sortSearchResults, + type SortDirection, + type SortKey, +} from "~/utils/discourseNodeSort"; const MAX_VISIBLE_RESULTS = 50; const SEARCH_DEBOUNCE_MS = 250; @@ -64,80 +76,6 @@ type SearchResultRow = RankedDiscourseNode & { nodeType: NodeTypeDisplay; }; -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: Record | undefined = - app.metadataCache.getFileCache(file)?.frontmatter; - 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. - */ -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; -}; - -/** - * `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, - 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 ( - typeof authorId === "number" && !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 => new Date(epochMs).toLocaleString(undefined, { dateStyle: "medium", @@ -339,9 +277,20 @@ const NodeSearch = ({ const [activeIndex, setActiveIndex] = useState(0); // Single source of truth: ENG-2111's tag chips will read and write this too. const [selectedNodeTypeIds, setSelectedNodeTypeIds] = useState([]); - const [isTypeFilterOpen, setIsTypeFilterOpen] = useState(false); + // 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( + DEFAULT_SORT_DIRECTION, + ); + // An editable span, so the query shares its line boxes with the filter chips. const inputRef = useRef(null); - const userNames = useAuthorNames({ app, plugin, candidateState }); + const userNames = useAuthorNames({ + app, + plugin, + candidates: + candidateState.status === "ready" ? candidateState.candidates : null, + }); const nodeTypesById = useMemo(() => { const byId = new Map(); @@ -381,12 +330,27 @@ const NodeSearch = ({ return () => window.clearTimeout(timeout); }, [query]); + // Sort before truncating, so a date or alphabetical sort covers every match. const results = useMemo(() => { if (candidateState.status !== "ready") return []; - return rankDiscourseNodesByTitle({ + const ranked = rankDiscourseNodesByTitle({ candidates: candidateState.candidates, query: debouncedQuery, nodeTypeIds: selectedNodeTypeIds, + }); + const authorNameByPath = + sortKey === "author" + ? buildAuthorNameByPath({ + app, + files: ranked.map((result) => result.file), + userNames, + }) + : undefined; + return sortSearchResults({ + results: ranked, + sortKey, + direction: sortDirection, + authorNameByPath, }) .slice(0, MAX_VISIBLE_RESULTS) .map((result) => ({ @@ -396,7 +360,16 @@ const NodeSearch = ({ badge: getFallbackNodeTypeBadge(result.title), }, })); - }, [candidateState, debouncedQuery, nodeTypesById, selectedNodeTypeIds]); + }, [ + app, + candidateState, + debouncedQuery, + nodeTypesById, + selectedNodeTypeIds, + sortDirection, + sortKey, + userNames, + ]); // 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 @@ -441,10 +414,16 @@ const NodeSearch = ({ }); }; - const handleTypeFilterOpenChange = (nextOpen: boolean): void => { - setIsTypeFilterOpen(nextOpen); + const handleDropdownOpenChange = ({ + id, + isOpen, + }: { + id: NonNullable; + isOpen: boolean; + }): void => { + setOpenDropdown(isOpen ? id : null); // Returns the keyboard path to the results the moment the panel closes. - if (!nextOpen) inputRef.current?.focus(); + if (!isOpen) inputRef.current?.focus(); }; // Closes before inserting, like `openActiveResult`. @@ -502,9 +481,8 @@ 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 the filter trigger's count badge, which sits outside the - button box, is not clipped by the modal's overflow-hidden content. */} - {/* Top-aligned: the field grows downwards, so the trigger stays on its first line. */} + {/* 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. */}
+ handleDropdownOpenChange({ id: "type-filter", isOpen }) + } onSelectedNodeTypeIdsChange={setSelectedNodeTypeIds} selectedNodeTypeIds={selectedNodeTypeIds} /> + + handleDropdownOpenChange({ id: "sort", isOpen }) + } + onSortChange={({ sortKey: nextKey, direction }) => { + setSortKey(nextKey); + setSortDirection(direction); + }} + sortDirection={sortDirection} + sortKey={sortKey} + />
diff --git a/apps/obsidian/src/components/NodeSortMenu.tsx b/apps/obsidian/src/components/NodeSortMenu.tsx new file mode 100644 index 000000000..5f29a8cba --- /dev/null +++ b/apps/obsidian/src/components/NodeSortMenu.tsx @@ -0,0 +1,146 @@ +import { App, setIcon } from "obsidian"; +import type { ReactElement } from "react"; +import { SearchDropdown } from "~/components/SearchDropdown"; +import { + SORT_OPTIONS, + getDefaultDirectionForKey, + getSortDirectionLabel, + getSortOptionLabel, + isDefaultSort, + type SortDirection, + 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, not buttons: Obsidian's button chrome reads as separate widgets rather than a menu. */ +const SortOptionRow = ({ + isSelected, + label, + onSelect, +}: { + isSelected: boolean; + label: string; + onSelect: () => void; +}): ReactElement => ( +
event.preventDefault()} + className={`flex cursor-pointer items-center gap-2 px-3 py-1.5 text-sm ${ + isSelected + ? "bg-accent text-on-accent" + : "text-normal hover:bg-modifier-hover" + }`} + > + {/* Always occupies its slot, so selecting an option does not shift the labels. */} + + {isSelected && ( + (el && setIcon(el, "check")) || undefined} /> + )} + + {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" + }`} + > + + (el && setIcon(el, getDirectionIconName(direction))) || undefined + } + /> + {label} +
+ ))} +
+); + +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, + }); + + return ( + +
+
Sort by
+ {SORT_OPTIONS.map((option) => ( + + onSortChange({ + sortKey: option.key, + direction: + option.key === sortKey + ? sortDirection + : getDefaultDirectionForKey(option.key), + }) + } + /> + ))} +
+ onSortChange({ sortKey, direction })} + sortDirection={sortDirection} + sortKey={sortKey} + /> + + ); +}; 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} + /> + ); }; diff --git a/apps/obsidian/src/components/SearchDropdown.tsx b/apps/obsidian/src/components/SearchDropdown.tsx new file mode 100644 index 000000000..818b4c7a4 --- /dev/null +++ b/apps/obsidian/src/components/SearchDropdown.tsx @@ -0,0 +1,106 @@ +import { App, Scope, setIcon } from "obsidian"; +import { useEffect, useRef, type ReactElement, type ReactNode } from "react"; + +/** Which toolbar panel is open, so two can never be open at once. */ +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, +}: { + app: App; + ariaLabel: string; + badgeCount?: number; + children: ReactNode; + iconName: string; + isActive: boolean; + isDisabled?: boolean; + isOpen: boolean; + onOpenChange: (isOpen: boolean) => void; + panelClassName?: string; + title: string; +}): ReactElement => { + const containerRef = useRef(null); + + // 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(); + scope.register([], "Escape", () => { + onOpenChange(false); + return false; + }); + app.keymap.pushScope(scope); + return () => app.keymap.popScope(scope); + }, [app, isOpen, onOpenChange]); + + // `activeDocument`, so the listener lands in the popout window holding the modal. + 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; + // Panel keystrokes must not reach the modal's Enter and arrow result navigation; Escape never arrives here at all. + 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..57efa0fb9 --- /dev/null +++ b/apps/obsidian/src/utils/discourseNodeAuthor.ts @@ -0,0 +1,93 @@ +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"; + +/** 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"; + +/** 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 a cast is a no-op. + const frontmatter: Record | undefined = + app.metadataCache.getFileCache(file)?.frontmatter; + return frontmatter?.authorId; +}; + +/** "You" only when there is no `authorId`; a present but unresolvable id stays "Unknown". */ +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; +}; + +export const isUnattributedAuthorName = (authorName: string): boolean => + authorName === UNRESOLVED_AUTHOR_NAME; + +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; +}; + +/** One query returns every person, so this refreshes once per open when a name is missing. */ +export const useAuthorNames = ({ + app, + plugin, + candidates, +}: { + app: App; + plugin: DiscourseGraphPlugin; + 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..cd1b9b737 --- /dev/null +++ b/apps/obsidian/src/utils/discourseNodeSort.ts @@ -0,0 +1,140 @@ +import { TFile } from "obsidian"; +import { isUnattributedAuthorName } from "~/utils/discourseNodeAuthor"; + +/** Client-side result ordering, over the full ranked list before display truncation. Mirrors Roam's advanced search. */ + +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"; +export const DEFAULT_SORT_DIRECTION: SortDirection = "desc"; + +/** Structural, so the sort runs on ranked results or decorated rows alike. */ +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]; + +/** "Descending" means something different per dimension, so switching resets it. */ +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. */ +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; + + 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. */ +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 17eb56b416b3bd325082dbfe130f8d4866f02560 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 23 Aug 2026 17:38:51 -0400 Subject: [PATCH 2/2] ENG-2112 Make the sort controls keyboard-operable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The option rows and direction controls are divs, so Enter and Space needed wiring up and the rows needed tabIndex to be reachable at all. Raised in review; the filter panel got this for free from its native checkboxes. Also drops the empty-before-setIcon in SearchDropdown. The claim it rested on — that `setIcon` appends — does not hold on Obsidian 1.13.7: driving the panel through ten direction changes and five option switches leaves exactly one svg in every icon container, with or without the empty. All three call sites now use the same one-liner as the rest of the app. 35/35, with three assertions added: every control carries tabIndex, Enter on a focused option applies that sort, and Space on a focused direction control applies it. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/components/NodeSortMenu.tsx | 16 +++++++++++++++- apps/obsidian/src/components/SearchDropdown.tsx | 7 +------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/apps/obsidian/src/components/NodeSortMenu.tsx b/apps/obsidian/src/components/NodeSortMenu.tsx index 5f29a8cba..ade1326ec 100644 --- a/apps/obsidian/src/components/NodeSortMenu.tsx +++ b/apps/obsidian/src/components/NodeSortMenu.tsx @@ -1,5 +1,5 @@ import { App, setIcon } from "obsidian"; -import type { ReactElement } from "react"; +import type { KeyboardEvent, ReactElement } from "react"; import { SearchDropdown } from "~/components/SearchDropdown"; import { SORT_OPTIONS, @@ -16,6 +16,16 @@ const DIRECTIONS: { direction: SortDirection; label: string }[] = [ { direction: "desc", label: "Desc" }, ]; +// Rows are divs, so Enter and Space have to be wired up the way a button gets them free. +const activateOnKey = ( + event: KeyboardEvent, + activate: () => void, +): void => { + if (event.key !== "Enter" && event.key !== " ") return; + event.preventDefault(); + activate(); +}; + const getDirectionIconName = (direction: SortDirection): string => direction === "asc" ? "arrow-up-narrow-wide" : "arrow-down-wide-narrow"; @@ -32,7 +42,9 @@ const SortOptionRow = ({
activateOnKey(event, onSelect)} onMouseDown={(event) => event.preventDefault()} className={`flex cursor-pointer items-center gap-2 px-3 py-1.5 text-sm ${ isSelected @@ -65,8 +77,10 @@ const DirectionToggle = ({ key={direction} role="button" aria-pressed={direction === sortDirection} + tabIndex={0} title={getSortDirectionLabel({ sortKey, direction })} onClick={() => onSelect(direction)} + onKeyDown={(event) => activateOnKey(event, () => 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 diff --git a/apps/obsidian/src/components/SearchDropdown.tsx b/apps/obsidian/src/components/SearchDropdown.tsx index 818b4c7a4..d4030164e 100644 --- a/apps/obsidian/src/components/SearchDropdown.tsx +++ b/apps/obsidian/src/components/SearchDropdown.tsx @@ -78,12 +78,7 @@ export const SearchDropdown = ({ > { - if (!el) return; - el.empty(); - setIcon(el, iconName); - }} + ref={(el) => (el && setIcon(el, iconName)) || undefined} /> {badgeCount > 0 && (