[trangdoan982]

Reviewer brief

Stacked on #1525 (Roam settings menu overhaul). Merge that first; this PR targets roam-setting-overhaul.

• Result: each of the six node type sections collapses on its own, and a Collapse all / Expand all button sits in the node page header. Section headings stick to the top of the scroll area on every settings page that has them. A settings search jump into Grammar > Nodes expands every section, then scrolls to the row and flashes it.
• Review focus:
  • apps/roam/src/components/settings/components/SettingsHeadings.tsx: SettingsGroup keeps collapsed rows mounted with Blueprint Collapse and marks them inert. When a stuck section collapses, it keeps the heading in place by adjusting only the settings scroller. StickyHeading copies the dialog's background, so Roam themes stay opaque and matched.
  • apps/roam/src/components/settings/navigation/useSettingAnchorScroll.ts: the search jump now waits until every ancestor Collapse reaches height: auto, with a 500ms limit.
• Risk or follow-up (known limits):
  • The heading colour is read when the heading mounts. It goes stale if the Roam theme changes while Settings is open.
  • getNodeConfigSections repeats NodeConfig.tsx's section list and its isSyncEnabled() condition by hand.
  • Collapse all doesn't preserve the scroll position. The page shrinks to its headings and scrolls to the top.
  • A setting search jump into Grammar > Nodes expands every section, not only the target's (agreed during planning).

Collapse state lives in the dialog and flows down to the node page. Search jumps reset it before scrolling.

flowchart LR
  SD["Settings.tsx<br/>collapsedNodeSections state<br/>search jump: reset to all expanded"] --> GNR["GrammarNodesRoute.tsx<br/>Collapse all / Expand all"]
  GNR --> NC["NodeConfig.tsx<br/>6 × SettingsGroup collapsible"]
  NC --> SG["SettingsGroup<br/>sticky heading + Collapse keepChildrenMounted + inert"]
  SD --> Hook["useSettingAnchorScroll<br/>wait for Collapse height: auto, then scroll + flash"]
  Hook -.-> SG

Loading

Verification

Ran live in Roam with dg-roam-load-extension on slot 1 against 2f843b81.

Scenario	Input	Expected	Actual	Pass
Default	Open Grammar > Nodes > Claim	All 6 sections expanded, button reads "Collapse all"	As expected	✅
Collapse all	Click Collapse all	Only headings visible, button reads "Expand all", content inert	As expected	✅
Sticky heading	Scroll into Canvas	Canvas heading pinned at the scroll box top	184.1 = 184.1	✅
Collapse a stuck heading	Click the stuck Canvas heading	Canvas collapses, heading stays at the top, overlay doesn't scroll	Offset 0 on every frame for 450ms; overlay scrollTop 0	✅
Node switch	Collapse Legacy on Claim, open Evidence	Legacy still collapsed	As expected	✅
Search jump	Collapse all, then search "Overlay"	All sections expand, row scrolls into view and flashes	Flash after expand (317ms); row inside the box at max scroll	✅
Keyboard	Enter, then Space on a heading	Toggles	As expected	✅
Tab order	Collapse Canvas, Tab from its heading	Focus skips Canvas controls	Lands on the Legacy heading	✅
Other pages	Features > Canvas, Left sidebar, Preferences > General	Headings sticky; Node trigger renders as before	Sticky at the box top; Node trigger can't reach the top (last, short group)	✅
Dark and custom themes	bp3-dark probe; injected .bp3-dialog background	Heading background matches the dialog	rgb(41,55,66) and rgb(30,30,46) match	✅

Scenario	Screenshot
Collapse all leaves only headings
Canvas heading stuck while scrolling
Clicking the stuck heading collapses it in place
Search jump to Overlay after Collapse all
Tab skips the collapsed Canvas section
Dark theme: heading matches dialog
Custom theme: heading copies dialog background
Left sidebar before: heading scrolls away
Left sidebar after: Global heading sticks

Tests: apps/roam/src/utils/__tests__/settingsCollapsibleSections.test.ts covers the section helpers and the SettingsGroup toggle, its inert state and its keyboard handling. Rerun with pnpm -C apps/roam exec vitest run src/utils/__tests__/settingsCollapsibleSections.test.ts.

Not verified: a real third-party Roam theme. The theme checks used Blueprint's bp3-dark class and an injected .bp3-dialog background.

Loom video

https://www.loom.com/share/d7d764a701f046b2875609cfaa6285b8

Scope check

• Ran $scope-check against the ENG ticket and final diff.
• Scope beyond Done When:
  • Collapse state persists across node types and tabs for the dialog's lifetime.
  • Clicking a stuck heading collapses it and keeps the heading in place.
  • A setting search jump into Grammar > Nodes expands every section rather than only the target's.
• Required now: the first two keep collapse usable when you scroll or switch node types; without them, collapsing loses your place. The third guarantees the target row is visible without a per-section lookup.
• Anyone affected or consulted: Yes, the ticket author approved all three during planning.
• Decision: Not documented in Linear yet; to be recorded in an ENG-2409 comment before merge.

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.

Fixed from review: theme-matched heading background, scroller-only realign, inert collapsed rows, a time-based expand wait, and functional state updates. The remaining findings are the known limits listed in Reviewer brief.

🤖 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 17, 2026 2:58pm UTC

[linear-code]

ENG-2409

[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 ↗︎.

[devin-ai-integration]

Devin Review found 1 potential issue.