ENG-1217: Port section component in left sidebar

[sid597]

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

---

Summary by CodeRabbit

• Bug Fixes

  • Fixed left sidebar page changes not persisting when reordering, adding, or removing pages.

• Improvements

  • Enhanced synchronization of personal settings to ensure configuration changes are properly saved.
  • Improved flexibility in settings panel configuration options.

[linear]

ENG-1217 Section components in Left sidebar

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

[sid597]

• ENG-1468: Port node and relations tree based consumers #822
• ENG-1467: Port global setting consumer reads (→ getGlobalSetting) #821
• ENG-1473: Port non-boolean personal setting consumers #818
• ENG-1456: Migrate personal boolean flag consumers (getSetting → getPersonalSetting) #814
• ENG-1472: Refactor BlockPropSettingPanels to add accessor-backed default reads #813
• ENG-1455: Dual-read from old-system settings and from blockprops #812
• ENG-1465: Add "Use new settings store" feature flag  #811
• ENG-1217: Port section component in left sidebar #804 👈 (View in Graphite)
• ENG-1328: Port small blueprint based misc settings into props based #805
• eng-1438-keyboard-quick-shortcut-keystriggers

This stack of pull requests is managed by Graphite. Learn more about stacking.

[sid597]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR migrates left-sidebar settings components to new block-prop driven persistence approaches. Global settings now wire GlobalFlagPanel with setGlobalSetting for page operations (add/move/remove), while personal settings implement PersonalNumberPanel with a new sectionsToBlockProps synchronization layer and setPersonalSetting persistence. Zod schemas are updated to use UID-based identifiers and array-based structures.

Changes

Cohort / File(s)	Summary
Global Settings Migration apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx	Replaced FlagPanel with GlobalFlagPanel for global-section flags. Added pagesToUids helper to map pages to UIDs. Wired setGlobalSetting persistence for page reorder (movePage), add (addPage), and remove (removePage) operations. Updated state-first-then-persist flow.
Personal Settings Migration apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx	Replaced NumberPanel with PersonalNumberPanel. Introduced sectionsToBlockProps and syncAllSectionsToBlockProps helpers to map section config to nested block-props structure. Added sectionsRef shared reference to avoid stale closures. Wired setPersonalSetting persistence across multiple mutation paths (add/move/edit sections, alias changes). Expanded aliasDebounceRef usage for debounced sync operations.
Block Prop Components apps/roam/src/components/settings/components/BlockPropSettingPanels.tsx	Extended NumberWrapperProps with optional setter?: NumberSetter field. Updated PersonalNumberPanel to destructure and forward setter prop, defaulting to personalAccessors.number.setter when not provided, enabling per-instance setter override.
Schema Updates apps/roam/src/components/settings/utils/zodSchema.ts	Added required name: string field to PersonalSectionSchema. Replaced Page string with uid: string in Children items. Changed LeftSidebarPersonalSettingsSchema from record<string, ...> (default \{\}) to array of PersonalSectionSchema (default \[\]).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• ENG-1187: Prod: Implement get and set for block prop based settings #669 — Introduces the new setGlobalSetting and setPersonalSetting accessors that this PR directly wires into the settings components for persistence.
• ENG-1105: zod schema for all the settings #664 — Modifies PersonalSection and LeftSidebarPersonalSettings Zod schemas, directly overlapping with the schema field and structure changes in this PR.
• ENG-853 left sidebar global and personal settings #441 — Updates the same LeftSidebarGlobalSettings and LeftSidebarPersonalSettings files to alter add/remove/reorder logic and persistence, sharing control-flow concerns with this PR.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Port section component in left sidebar' directly relates to the main changes in the PR, which involve porting/refactoring section-related components in the left sidebar settings files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[mdroidian]

The audio in the loom is mostly inaudible. Please review the looms before submitting.
Also, a couple files are not formatted again.

There's a single comment asking to explain why caused the PersonalNumberPanel to break the pattern from the others. I think it would be worth exploring that deeper to see if there are alternative solutions prior to breaking the pattern.

[mdroidian]

Why is this panel breaking the pattern with setter? What is the reason/need for this?

[sid597]

usually the number panel setters are something that can be reached directly in the tree, but for this case we have an array and insde the array we have the key map. So to get to the numberPanel in this case we have to find the array for which we want to target, therefore we needed a custom setter for it. The standard setter can't target a specific array item so we needed this.

[sid597]

This reminded me we also have an Alias Field in setting and we should be using a similar solution there so I checked, and I was using old flagpanel component so I ported that also with similar logic. Following is a loom video showing its working state (checked the audio and its working sorry for the one not working one)

Another change I did was move this to graphite stack because we needed a baseTextPanel which had debounce built in

https://www.loom.com/share/537a13cccb764c18967f14bf1a7180e5

[devin-ai-integration]

Devin Review found 1 new potential issue.

⚠️ 1 issue in files not directly in the diff

⚠️ Pull watcher handler uses Object.keys() on array after schema change from record to array (apps/roam/src/components/settings/utils/pullWatchers.ts:128-145)

The "Left sidebar" handler in personalSettingsHandlers calls Object.keys(oldValue || {}) and Object.keys(newValue || {}) to compare sections. This was correct when LeftSidebarPersonalSettings was a record (Record<string, PersonalSection>), but the schema was changed in this PR to an array (PersonalSection[]).

Root Cause

With the new array-based schema (apps/roam/src/components/settings/utils/zodSchema.ts:222-224), PersonalSettings["Left sidebar"] is now PersonalSection[]. Calling Object.keys() on an array returns string indices (["0", "1", ...]) rather than section names.

For example, if the old value had 2 sections and the new value has 3 sections:

• oldSections = ["0", "1"]
• newSections = ["0", "1", "2"]
• addedSections = ["2"] (an index, not a section name)

The comparisons at lines 137-138 (newSections.filter((s) => !oldSections.includes(s))) compare array indices instead of meaningful section identifiers, making the added/removed section detection incorrect.

Impact: The debug logging output for left sidebar personal section changes will show array indices instead of section names, making the pull watcher diagnostics misleading. Since this is currently only used for debug logging (the comment says "stub for testing"), the functional impact is limited.

View 14 additional findings in Devin Review.

[devin-ai-integration]

Devin Review found 1 new potential issue.

⚠️ 1 issue in files not directly in the diff

⚠️ Pull watcher handler uses Object.keys() on array after schema change from record to array (apps/roam/src/components/settings/utils/pullWatchers.ts:128-145)

The "Left sidebar" handler in personalSettingsHandlers calls Object.keys(oldValue || {}) and Object.keys(newValue || {}) to compare sections. This was correct when LeftSidebarPersonalSettings was a record (Record<string, PersonalSection>), but the schema was changed in this PR to an array (PersonalSection[]).

Root Cause

With the new array-based schema (apps/roam/src/components/settings/utils/zodSchema.ts:222-224), PersonalSettings["Left sidebar"] is now PersonalSection[]. Calling Object.keys() on an array returns string indices (["0", "1", ...]) rather than section names.

For example, if the old value had 2 sections and the new value has 3 sections:

• oldSections = ["0", "1"]
• newSections = ["0", "1", "2"]
• addedSections = ["2"] (an index, not a section name)

The comparisons at lines 137-138 (newSections.filter((s) => !oldSections.includes(s))) compare array indices instead of meaningful section identifiers, making the added/removed section detection incorrect.

Impact: The debug logging output for left sidebar personal section changes will show array indices instead of section names, making the pull watcher diagnostics misleading. Since this is currently only used for debug logging (the comment says "stub for testing"), the functional impact is limited.

View 14 additional findings in Devin Review.

[devin-ai-integration]

Devin Review found 1 new potential issue.

⚠️ 1 issue in files not directly in the diff

⚠️ Pull watcher handler uses Object.keys() on array after schema change from record to array (apps/roam/src/components/settings/utils/pullWatchers.ts:128-145)

The "Left sidebar" handler in personalSettingsHandlers calls Object.keys(oldValue || {}) and Object.keys(newValue || {}) to compare sections. This was correct when LeftSidebarPersonalSettings was a record (Record<string, PersonalSection>), but the schema was changed in this PR to an array (PersonalSection[]).

Root Cause

With the new array-based schema (apps/roam/src/components/settings/utils/zodSchema.ts:222-224), PersonalSettings["Left sidebar"] is now PersonalSection[]. Calling Object.keys() on an array returns string indices (["0", "1", ...]) rather than section names.

For example, if the old value had 2 sections and the new value has 3 sections:

• oldSections = ["0", "1"]
• newSections = ["0", "1", "2"]
• addedSections = ["2"] (an index, not a section name)

The comparisons at lines 137-138 (newSections.filter((s) => !oldSections.includes(s))) compare array indices instead of meaningful section identifiers, making the added/removed section detection incorrect.

Impact: The debug logging output for left sidebar personal section changes will show array indices instead of section names, making the pull watcher diagnostics misleading. Since this is currently only used for debug logging (the comment says "stub for testing"), the functional impact is limited.

View 14 additional findings in Devin Review.