ENG-2367 Scroll to a candidate result in context of its page in the Roam search preview

[trangdoan982]

Reviewer brief

• Result: Selecting a candidate result previews its parent page, centred on the tagged block, which flashes for 3s. A candidate under a collapsed parent previews as the zoomed block instead, and the graph's collapse state isn't changed. Regular node previews render as before.
• Stacked on ENG-2366 Show candidate nodes as results in Roam advanced node search #1521 (ENG-2366). This PR targets that branch, and ENG-2366 Show candidate nodes as results in Roam advanced node search #1521 must merge first.
• Review focus: the two effects in CandidatePreview (apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchDialog.tsx). The render effect owns the mount node and unmounts only after its render settles. The reveal effect runs only when the rendered uid matches the one this result needs, which stops a fast switch from scrolling or flashing the wrong block.
• Risk or follow-up: hasCollapsedAncestor has no unit test. Its query was checked live (collapsed parent, collapsed grandparent, all ancestors open).

The diagram shows what happens when a result is selected:

flowchart TD
  A[Select result in AdvancedSearchDialog] --> B{result.candidate?}
  B -- no --> N[RenderRoamPage / RenderRoamBlock zoomPath<br/>unchanged; scrollTop = 0 if previous was a candidate]
  B -- yes --> C{hasCollapsedAncestor uid<br/>:block/parents with :block/open false}
  C -- no --> P[renderPage candidate.pageUid, hide-mentions]
  C -- yes --> Z[renderBlock uid, zoom-path]
  P --> W[await render, then waitForImages ≤1000ms]
  Z --> W
  W --> G{renderedUid === renderUid?<br/>not cancelled}
  G -- yes --> R[revealBlockInPreview<br/>.rm-block data-block-uid, skip embeds<br/>set container scrollTop to centre, add flash class]
  R -- not found --> T[scrollTop = 0, no flash]

Loading

Verification

Ran live in Roam on the test graph with dg-roam-load-extension at a8585bb8. pnpm ci:validate passes.

Scenario	Input	Expected	Actual	Pass
Candidate on a long page	Top-level candidate after 60+ blocks, embedded earlier on the same page	Page renders, block centred, only the real block's row flashes	Centre offset 0px, flash on the real row only, embed copy untouched	✅
Deeply nested candidate	Candidate 4 levels deep, ancestors open	Found, centred, flashed	Centre offset 0px, flashed	✅
Collapsed parent or grandparent	Candidate under :block/open false ancestors	Zoomed block shown and flashed, collapse state unchanged	Zoom path shown, flashed, :block/open false/false/true before and after	✅
Same page in the main window	Page open in Roam's main window	Main window doesn't scroll, only the preview copy flashes	Main scroller stayed at 400, flash only in the dialog	✅
Fast switching	22 arrow presses at 20ms, then a hover sweep	One flash at most, on the active result, one render mounted	Max 1 flash, ended on the active row, 1 mount at every checkpoint	✅
Two candidates on one page	Arrow between them	Page not re-rendered, re-scroll and new flash	Marker on the rendered page survived, scroll moved 1688 → 819	✅
Image above the candidate	800×1200 image above it	Scroll lands after the image loads	Position stable 1.5s later	✅
Candidate to node result	Long candidate page, then a long node page	Node preview starts at the top	scrollTop 1688 → 0, never painted at the old offset	✅
Node to node result	Scroll a node preview to 600, select another node	Unchanged from before	scrollTop stays 600	✅
Page deleted after search	Select a stale candidate	No error	Roam's "could not be found" text, no console errors or rejections	✅
Regular node result	Select a node page	Renders as before, no flash	Normal page render, no flash	✅

Scenario	Screenshot
Candidate shown in its page, centred and flashed
Collapsed parent falls back to the zoomed block
Roam main window keeps its scroll
Candidate to node result starts at the top
Regular node preview unchanged
Deleted page shows Roam's not-found text

Tests added:

• apps/roam/src/utils/__tests__/revealBlockInPreview.test.ts (jsdom): flashes only the tagged block's own row, skips embedded copies, ignores copies outside the preview, centres the row, scrolls to the top when the block isn't rendered, and clears the flash on cleanup and on animationend.
• apps/roam/src/utils/__tests__/candidateNodeSearch.test.ts: candidates carry pageUid.

Rerun with pnpm -C apps/roam test.

Not verified:

• Pages over 2,000 blocks. Every such page in the test graph was mostly collapsed, so lazy rendering on very long open pages wasn't exercised.
• The Roam desktop app, mobile, and non-default Roam themes.

Loom video

pending

Scope check

• Ran $scope-check against ENG-2367 and the final diff.
• Scope beyond Done When: When the tagged block isn't found in the rendered page for a reason other than a collapsed parent, the preview scrolls back to the top and doesn't flash. The zoomed fallback for a collapsed parent also flashes the block.
• Required now: Obsidian parity (ENG-2273 does the same). Without the reset, switching to another candidate on the same page could leave the preview mid-page with nothing highlighted. Flashing the zoomed block keeps one behaviour for every candidate.
• Anyone affected or consulted: Not documented.
• Decision: Not documented.

Standards check

• Ran $dg-pr-adherence-check against the final diff and PR metadata.

Local delegated full review

• Ran a comprehensive review of the entire final diff in a subagent with a fresh context. Use $dg-delegated-full-review when no other full-review workflow is available.

No findings on a8585bb8. Earlier rounds found three Low issues, all fixed in this branch:

• unmount before the render settled;
• render rejections not caught;
• a candidate's scroll carrying over into a node preview.

🤖 Generated with Claude Code

---

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
discourse-graph	Skipped		Oct 9, 2026 8:57pm UTC

[supabase]

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[linear-code]

ENG-2367

[devin-ai-integration]

Devin Review found 2 potential issues.

[devin-ai-integration]

🟡 Moved candidates preview the wrong page

When a candidate moves between indexing and selection, pageUid still identifies its former page. The preview shows that page and cannot reveal the candidate.

Learn more

Candidate results store the page UID when queryCandidatesForType builds the search index. The index persists while the dialog is open. Moving a candidate to another page during that time leaves the candidate UID valid but the stored page UID stale. The preview renders the former page, and revealBlockInPreview cannot find the candidate there.

Example: Search indexes block b1 on page p1. Move b1 to page p2, then select its search row. The preview renders p1 and scrolls to its top instead of displaying b1 on p2.

Recommended fix: Resolve the candidate's current page UID when rendering its preview, or verify the indexed page UID still owns the block and fall back to rendering the block if it changed. Refresh the preview when that relationship changes.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🟡 Slow images displace the candidate

When an image above the candidate loads after one second, waitForImages releases the reveal early. The image shifts the page afterward, leaving the candidate off-center or outside the preview.

Learn more

The candidate preview waits for images before measuring the target row and setting the scroll position. The one-second timer can resolve before an image above that row finishes loading. The later image height change shifts the row, but the reveal effect only runs once per selected UID.

Example: An image above block b1 takes two seconds to load. At one second the preview centers b1 using the image's empty height; one second later the image expands by 1200 pixels and pushes b1 below the visible area.

Recommended fix: Keep the initial timeout for responsiveness, but remeasure and adjust the scroll when pending images finish loading, provided the same candidate remains active. Clean up image listeners and timers when switching results.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[trangdoan982]

Leaving this as is. The 1000 ms cap is deliberate and matches the Obsidian preview (ENG-2273); without it, a slow or broken image would hold the reveal indefinitely. An image that finishes after the cap can still shift the row, which is an accepted limit shared with Obsidian. Live check: an 800×1200 image above the candidate loaded inside the cap and the row stayed centred.

🤖 Addressed by Claude Code