ENG-2109 Create node search modal with ranked results and preview - #1285
Open
trangdoan982 wants to merge 12 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
trangdoan982
force-pushed
the
eng-2108-add-ranked-search-to-queryengine-with-a-vault-iteration
branch
from
August 8, 2026 17:32
44c784b to
07bb663
Compare
trangdoan982
force-pushed
the
eng-2109-create-node-search-modal-with-ranked-results-and-preview
branch
from
August 8, 2026 17:33
adea63a to
2d09490
Compare
1 task
Contributor
PR size/scope checkThis PR is over our review-size guideline.
Please split this into smaller PRs unless there is a clear reason the changes need to land together. If keeping it as one PR, please add a brief justification covering:
|
trangdoan982
force-pushed
the
eng-2108-add-ranked-search-to-queryengine-with-a-vault-iteration
branch
from
August 9, 2026 03:55
07bb663 to
f89d51d
Compare
trangdoan982
force-pushed
the
eng-2109-create-node-search-modal-with-ranked-results-and-preview
branch
from
August 9, 2026 03:56
45ee16f to
e5f9945
Compare
trangdoan982
force-pushed
the
eng-2108-add-ranked-search-to-queryengine-with-a-vault-iteration
branch
from
August 9, 2026 17:22
f89d51d to
79938ba
Compare
trangdoan982
force-pushed
the
eng-2109-create-node-search-modal-with-ranked-results-and-preview
branch
from
August 9, 2026 17:23
6a5112f to
fe08c76
Compare
trangdoan982
force-pushed
the
eng-2108-add-ranked-search-to-queryengine-with-a-vault-iteration
branch
from
August 9, 2026 17:28
79938ba to
9708e95
Compare
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
trangdoan982
force-pushed
the
eng-2109-create-node-search-modal-with-ranked-results-and-preview
branch
from
August 9, 2026 17:29
fe08c76 to
33fb1fc
Compare
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://www.loom.com/share/877db7d2de834548a81bca1750bbbecb
Scope check
$scope-checkagainst ENG-2109 and the final diff.Done When: Match highlighting, pulled forward from F11 (ENG-2115).RankedDiscourseNodealready carries the wholeSearchResult, and ENG-2108 scoresfile.basenamespecifically so those offsets stay valid forrenderResults— rendering a plain title discards data the design deliberately preserved. It is also what makes the ranking legible enough to evaluate live, which is why this ticket was stacked rather than deferred.Done When) and ENG-2115 (struck through, leaving theme verification).Nothing else exceeds the boundary. F11 keeps badge colour/prefix-stripping and the community-theme pass; the node type renders as plain text here.
What this does
The first UI for the Obsidian advanced node search (F2 + F3 + F5), on top of the ENG-2108 seam.
createRoot, matchingModifyNodeModalrankDiscourseNodesByTitle, input debounced 250ms to matchSearchBar.tsx, capped at 50renderResults(el, title, match)MarkdownRenderer.renderwith aComponentfor lifecycle — theNodeTypeSettings.tsxpattern — showingcreated · modified · authoropen-node-search,hotkeys: []🤖 Generated with Claude Code