Skip to content

ENG-1478: Port left sidebar to dual-readers + reactivity#844

Open
sid597 wants to merge 7 commits intomigration-block-init-staging-branchfrom
eng-1478-port-both-global-and-personal-left-sidebar-to-dual-readers-v2
Open

ENG-1478: Port left sidebar to dual-readers + reactivity#844
sid597 wants to merge 7 commits intomigration-block-init-staging-branchfrom
eng-1478-port-both-global-and-personal-left-sidebar-to-dual-readers-v2

Conversation

@sid597
Copy link
Collaborator

@sid597 sid597 commented Mar 2, 2026

https://www.loom.com/share/6258dfb98cd0424a9c1b3ef7794955c3


Open with Devin

Summary by CodeRabbit

Release Notes

  • New Features

    • Added support for both global and per-section configuration of left sidebar folding behavior.
    • Enabled dynamic toggling of the left sidebar via feature flag.
  • Improvements

    • Enhanced settings validation and synchronization across configuration systems to ensure consistency.

@linear
Copy link

linear bot commented Mar 2, 2026

@supabase
Copy link

supabase bot commented Mar 2, 2026

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
Copy link
Collaborator Author

sid597 commented Mar 2, 2026

@coderabbitai full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 2, 2026

✅ Actions performed

Full review triggered.

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 6 additional findings.

Open in Devin Review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 2, 2026

📝 Walkthrough

Walkthrough

This PR implements dual-read and dual-write support for left sidebar configuration. It adds a buildConfig helper to merge accessor-based values with legacy UID-backed settings, modifies useConfig to rebuild state from this merged config, wires dual-write support for folding state across global and per-section levels, and integrates feature flag handling into the extension lifecycle with proper cleanup.

Changes

Cohort / File(s) Summary
Left Sidebar Config Logic
apps/roam/src/components/LeftSidebarView.tsx
Introduces buildConfig helper merging accessor values with legacy UIDs; reworks useConfig to initialize/refresh from buildConfig; adds sectionIndex and isGlobal parameter propagation for dual-write folding state support via setGlobalSetting/setPersonalSetting; imports new accessor and type imports.
Settings Components Dual-Read
apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx, apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx
Adds dual-read validation via getGlobalSetting and getPersonalSetting with type parameters; logs mismatches when flag is ON; uses legacy UID-backed data for CRUD operations.
Feature Flag and Lifecycle Integration
apps/roam/src/components/settings/utils/pullWatchers.ts, apps/roam/src/index.ts
Introduces setLeftSidebarFlagHandler and leftSidebarFlagHandler callback registration; adds featureFlagHandler for "Enable left sidebar" flag; registers handler in index.ts to mount/unmount LeftSidebarView dynamically; captures and invokes cleanup from setupPullWatchOnSettingsPage in unload().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: porting the left sidebar to dual-readers and adding reactivity, which aligns with the substantial changes across multiple components.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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


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

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

coderabbitai[bot]

This comment was marked as resolved.

@sid597 sid597 force-pushed the eng-1478-port-both-global-and-personal-left-sidebar-to-dual-readers-v2 branch from c97dff8 to 00eb897 Compare March 3, 2026 11:53
devin-ai-integration[bot]

This comment was marked as resolved.

sid597 added 3 commits March 3, 2026 18:31
…er-based reactivity

- Extract mergeGlobalSectionWithAccessor/mergePersonalSectionsWithAccessor to
  getLeftSidebarSettings.ts (used by LeftSidebarView, GlobalSettings, PersonalSettings)
- Settings panels now actually use accessor values (not fire-and-forget)
- Replace setLeftSidebarFlagHandler with settingsEmitter pub/sub
- useConfig subscribes to emitter instead of old subscribe()
- Wire up global/personal/flag handlers in pullWatchers to emit
- Remove dead newValue===oldValue guard (hasPropChanged already checks)
- Remove dead ?./?? in personal merge (Zod defaults guarantee values)
toggleFoldedState mutates folded.uid in-place then dual-writes to
block props, triggering pull watch → emitter → buildConfig(). Without
refreshConfigTree(), buildConfig() reads stale UIDs from the cached
tree, overwriting the in-place mutation and orphaning Roam blocks.
@sid597 sid597 force-pushed the migration-block-init-staging-branch branch from 21b6000 to 3b4c80a Compare March 3, 2026 13:03
@sid597 sid597 force-pushed the eng-1478-port-both-global-and-personal-left-sidebar-to-dual-readers-v2 branch from 4411ee5 to 686fd05 Compare March 3, 2026 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant