From f6cc8c2990e977d5c2fcc81834edbd5d297da7a0 Mon Sep 17 00:00:00 2001 From: sid597 Date: Mon, 2 Mar 2026 13:14:16 +0530 Subject: [PATCH 1/7] ENG-1478: Port left sidebar to dual-readers + reactivity --- apps/roam/src/components/LeftSidebarView.tsx | 112 +++++++++++++++++- .../settings/LeftSidebarGlobalSettings.tsx | 10 +- .../settings/LeftSidebarPersonalSettings.tsx | 11 +- .../components/settings/utils/pullWatchers.ts | 43 +++---- apps/roam/src/index.ts | 31 ++++- 5 files changed, 170 insertions(+), 37 deletions(-) diff --git a/apps/roam/src/components/LeftSidebarView.tsx b/apps/roam/src/components/LeftSidebarView.tsx index 4647d3238..b6d009f2e 100644 --- a/apps/roam/src/components/LeftSidebarView.tsx +++ b/apps/roam/src/components/LeftSidebarView.tsx @@ -30,6 +30,16 @@ import type { LeftSidebarConfig, LeftSidebarPersonalSectionConfig, } from "~/utils/getLeftSidebarSettings"; +import { + getGlobalSetting, + getPersonalSetting, + setGlobalSetting, + setPersonalSetting, +} from "~/components/settings/utils/accessors"; +import type { + LeftSidebarGlobalSettings, + PersonalSection, +} from "~/components/settings/utils/zodSchema"; import { createBlock } from "roamjs-components/writes"; import deleteBlock from "roamjs-components/writes/deleteBlock"; import getTextByBlockUid from "roamjs-components/queries/getTextByBlockUid"; @@ -95,12 +105,18 @@ const toggleFoldedState = ({ setIsOpen, folded, parentUid, + isGlobal, + sectionIndex, }: { isOpen: boolean; setIsOpen: Dispatch>; folded: { uid?: string; value: boolean }; parentUid: string; + isGlobal?: boolean; + sectionIndex?: number; }) => { + const newFolded = !isOpen; + if (isOpen) { setIsOpen(false); if (folded.uid) { @@ -118,6 +134,18 @@ const toggleFoldedState = ({ folded.uid = newUid; folded.value = true; } + + // Dual-write to block props + if (isGlobal) { + setGlobalSetting(["Left sidebar", "Settings", "Folded"], newFolded); + } else if (sectionIndex !== undefined) { + const sections = + getPersonalSetting(["Left sidebar"]) || []; + if (sections[sectionIndex]) { + sections[sectionIndex].Settings.Folded = newFolded; + setPersonalSetting(["Left sidebar"], sections); + } + } }; const SectionChildren = ({ @@ -160,8 +188,10 @@ const SectionChildren = ({ const PersonalSectionItem = ({ section, + sectionIndex, }: { section: LeftSidebarPersonalSectionConfig; + sectionIndex: number; }) => { const titleRef = parseReference(section.text); const blockText = useMemo( @@ -182,6 +212,7 @@ const PersonalSectionItem = ({ setIsOpen, folded: section.settings.folded, parentUid: section.settings.uid || "", + sectionIndex, }); }; @@ -226,9 +257,9 @@ const PersonalSections = ({ config }: { config: LeftSidebarConfig }) => { return (
- {sections.map((section) => ( + {sections.map((section, index) => (
- +
))}
@@ -253,6 +284,7 @@ const GlobalSection = ({ config }: { config: LeftSidebarConfig["global"] }) => { setIsOpen, folded: config.settings.folded, parentUid: config.settings.uid, + isGlobal: true, }); }} > @@ -276,13 +308,81 @@ const GlobalSection = ({ config }: { config: LeftSidebarConfig["global"] }) => { ); }; +// TODO(ENG-1471): Remove old-system merge when migration complete — just use accessor values directly +const buildConfig = (): LeftSidebarConfig => { + // Read VALUES from accessor (handles flag routing + mismatch detection) + const globalValues = getGlobalSetting([ + "Left sidebar", + ]); + const personalValues = getPersonalSetting([ + "Left sidebar", + ]); + + // Read UIDs from old system (needed for fold CRUD during dual-write) + const oldConfig = getFormattedConfigTree().leftSidebar; + + // Merge: accessor values + old-system UIDs + return { + uid: oldConfig.uid, + favoritesMigrated: oldConfig.favoritesMigrated, + sidebarMigrated: oldConfig.sidebarMigrated, + global: { + uid: oldConfig.global.uid, + childrenUid: oldConfig.global.childrenUid, + children: oldConfig.global.children, + settings: oldConfig.global.settings + ? { + uid: oldConfig.global.settings.uid, + collapsable: { + uid: oldConfig.global.settings.collapsable.uid, + value: + globalValues?.Settings.Collapsable ?? + oldConfig.global.settings.collapsable.value, + }, + folded: { + uid: oldConfig.global.settings.folded.uid, + value: + globalValues?.Settings.Folded ?? + oldConfig.global.settings.folded.value, + }, + } + : undefined, + }, + personal: { + uid: oldConfig.personal.uid, + sections: oldConfig.personal.sections.map((oldSection, i) => { + const newSection = personalValues?.[i]; + return { + ...oldSection, + settings: oldSection.settings + ? { + ...oldSection.settings, + truncateResult: { + ...oldSection.settings.truncateResult, + value: + newSection?.Settings?.["Truncate-result?"] ?? + oldSection.settings.truncateResult.value, + }, + folded: { + ...oldSection.settings.folded, + value: + newSection?.Settings?.Folded ?? + oldSection.settings.folded.value, + }, + } + : oldSection.settings, + }; + }), + }, + allPersonalSections: oldConfig.allPersonalSections, + }; +}; + export const useConfig = () => { - const [config, setConfig] = useState( - () => getFormattedConfigTree().leftSidebar, - ); + const [config, setConfig] = useState(() => buildConfig()); useEffect(() => { const handleUpdate = () => { - setConfig(getFormattedConfigTree().leftSidebar); + setConfig(buildConfig()); }; const unsubscribe = subscribe(handleUpdate); return () => { diff --git a/apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx b/apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx index 838346ab5..645f6cdf7 100644 --- a/apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx +++ b/apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx @@ -1,7 +1,11 @@ import React, { useCallback, useEffect, useMemo, useState, memo } from "react"; import { Button, ButtonGroup, Collapse } from "@blueprintjs/core"; import { GlobalFlagPanel } from "~/components/settings/components/BlockPropSettingPanels"; -import { setGlobalSetting } from "~/components/settings/utils/accessors"; +import { + setGlobalSetting, + getGlobalSetting, +} from "~/components/settings/utils/accessors"; +import type { LeftSidebarGlobalSettings } from "~/components/settings/utils/zodSchema"; import AutocompleteInput from "roamjs-components/components/AutocompleteInput"; import getAllPageNames from "roamjs-components/queries/getAllPageNames"; import createBlock from "roamjs-components/writes/createBlock"; @@ -98,6 +102,10 @@ const LeftSidebarGlobalSectionsContent = ({ const initialize = async () => { setIsInitializing(true); const globalSectionText = "Global-Section"; + // Dual-read: accessor validates values and logs mismatches when flag ON + getGlobalSetting(["Left sidebar"]); + + // Use old system data (has UIDs needed for CRUD operations) const config = getLeftSidebarGlobalSectionConfig(leftSidebar.children); const existingGlobalSection = leftSidebar.children.find( diff --git a/apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx b/apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx index ab4eeb61a..d36032a78 100644 --- a/apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx +++ b/apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx @@ -19,7 +19,11 @@ import createBlock from "roamjs-components/writes/createBlock"; import deleteBlock from "roamjs-components/writes/deleteBlock"; import updateBlock from "roamjs-components/writes/updateBlock"; import type { RoamBasicNode } from "roamjs-components/types"; -import { setPersonalSetting } from "~/components/settings/utils/accessors"; +import { + setPersonalSetting, + getPersonalSetting, +} from "~/components/settings/utils/accessors"; +import type { PersonalSection } from "~/components/settings/utils/zodSchema"; import { PersonalNumberPanel, PersonalTextPanel, @@ -599,6 +603,11 @@ const LeftSidebarPersonalSectionsContent = ({ setSections([]); } else { setPersonalSectionUid(personalSection.uid); + + // Dual-read: accessor validates values and logs mismatches when flag ON + getPersonalSetting(["Left sidebar"]); + + // Use old system data (has UIDs needed for CRUD operations) const loadedSections = getLeftSidebarPersonalSectionConfig( leftSidebar.children, ).sections; diff --git a/apps/roam/src/components/settings/utils/pullWatchers.ts b/apps/roam/src/components/settings/utils/pullWatchers.ts index 81026957c..1b20c113a 100644 --- a/apps/roam/src/components/settings/utils/pullWatchers.ts +++ b/apps/roam/src/components/settings/utils/pullWatchers.ts @@ -84,16 +84,25 @@ const addPullWatch = ( watches.push([pattern, entityId, callback]); }; +let leftSidebarFlagHandler: ((newValue: boolean) => void) | null = null; +export const setLeftSidebarFlagHandler = ( + handler: (newValue: boolean) => void, +) => { + leftSidebarFlagHandler = handler; +}; + export const featureFlagHandlers: Partial< Record< keyof FeatureFlags, (newValue: boolean, oldValue: boolean, allFlags: FeatureFlags) => void > > = { - // Add handlers as needed: - // "Enable Left Sidebar": (newValue) => { ... }, - // "Suggestive Mode Enabled": (newValue) => { ... }, - // "Reified Relation Triples": (newValue) => { ... }, + /* eslint-disable @typescript-eslint/naming-convention */ + "Enable left sidebar": (newValue, oldValue) => { + if (newValue === oldValue) return; + leftSidebarFlagHandler?.(newValue); + }, + /* eslint-enable @typescript-eslint/naming-convention */ }; type GlobalSettingsHandlers = { @@ -122,30 +131,8 @@ type PersonalSettingsHandlers = { }; export const personalSettingsHandlers: PersonalSettingsHandlers = { - // "Left Sidebar" stub for testing with stubSetLeftSidebarPersonalSections() in accessors.ts - /* eslint-disable @typescript-eslint/naming-convention */ - "Left sidebar": (newValue, oldValue) => { - const oldSections = Object.keys(oldValue || {}); - const newSections = Object.keys(newValue || {}); - - if (newSections.length === 0 && oldSections.length === 0) return; - - console.group("👤 [PullWatch] Personal Settings Changed: Left Sidebar"); - console.log("Old value:", JSON.stringify(oldValue, null, 2)); - console.log("New value:", JSON.stringify(newValue, null, 2)); - - const addedSections = newSections.filter((s) => !oldSections.includes(s)); - const removedSections = oldSections.filter((s) => !newSections.includes(s)); - - if (addedSections.length > 0) { - console.log(" → Sections added:", addedSections); - } - if (removedSections.length > 0) { - console.log(" → Sections removed:", removedSections); - } - console.groupEnd(); - }, - /* eslint-enable @typescript-eslint/naming-convention */ + // Add handlers as needed: + // "Left sidebar": (newValue) => { ... }, }; export const discourseNodeHandlers: Array< diff --git a/apps/roam/src/index.ts b/apps/roam/src/index.ts index 12f363e64..4a7473530 100644 --- a/apps/roam/src/index.ts +++ b/apps/roam/src/index.ts @@ -1,3 +1,4 @@ +import ReactDOM from "react-dom"; import { addStyle } from "roamjs-components/dom"; import { render as renderToast } from "roamjs-components/components/Toast"; import { runExtension } from "roamjs-components/util"; @@ -37,6 +38,11 @@ import { getFeatureFlag, getPersonalSetting, } from "./components/settings/utils/accessors"; +import { + setupPullWatchOnSettingsPage, + setLeftSidebarFlagHandler, +} from "./components/settings/utils/pullWatchers"; +import { mountLeftSidebar } from "./components/LeftSidebarView"; export const DEFAULT_CANVAS_PAGE_FORMAT = "Canvas/*"; @@ -149,7 +155,29 @@ export default runExtension(async (onloadArgs) => { }); } - await initSchema(); + // Register left sidebar flag handler before pull watchers start + const handleLeftSidebarFlag = (enabled: boolean) => { + const wrapper = document.querySelector( + ".starred-pages-wrapper", + ); + if (!wrapper) return; + if (enabled) { + wrapper.style.padding = "0"; + void mountLeftSidebar(wrapper, onloadArgs); + } else { + const root = wrapper.querySelector("#dg-left-sidebar-root"); + if (root) { + // eslint-disable-next-line react/no-deprecated + ReactDOM.unmountComponentAtNode(root); + root.remove(); + } + wrapper.style.padding = ""; + } + }; + setLeftSidebarFlagHandler(handleLeftSidebarFlag); + + const { blockUids } = await initSchema(); + const cleanupPullWatchers = setupPullWatchOnSettingsPage(blockUids); return { elements: [ @@ -161,6 +189,7 @@ export default runExtension(async (onloadArgs) => { ], observers: observers, unload: () => { + cleanupPullWatchers(); setSyncActivity(false); window.roamjs.extension?.smartblocks?.unregisterCommand("QUERYBUILDER"); // @ts-expect-error - tldraw throws a warning on multiple loads From 5155a2c21e06849caf59e93fe97545d89c701c17 Mon Sep 17 00:00:00 2001 From: sid597 Date: Mon, 2 Mar 2026 20:54:53 +0530 Subject: [PATCH 2/7] ENG-1478: DRY merge helpers, real dual-read in settings panels, emitter-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) --- apps/roam/src/components/LeftSidebarView.tsx | 80 ++++++------------- .../settings/LeftSidebarGlobalSettings.tsx | 21 ++--- .../settings/LeftSidebarPersonalSettings.tsx | 12 +-- .../components/settings/utils/pullWatchers.ts | 29 +++---- .../settings/utils/settingsEmitter.ts | 24 ++++++ apps/roam/src/index.ts | 47 +++++------ apps/roam/src/utils/getLeftSidebarSettings.ts | 56 +++++++++++++ 7 files changed, 158 insertions(+), 111 deletions(-) create mode 100644 apps/roam/src/components/settings/utils/settingsEmitter.ts diff --git a/apps/roam/src/components/LeftSidebarView.tsx b/apps/roam/src/components/LeftSidebarView.tsx index b6d009f2e..af5cf2cbb 100644 --- a/apps/roam/src/components/LeftSidebarView.tsx +++ b/apps/roam/src/components/LeftSidebarView.tsx @@ -21,14 +21,13 @@ import { import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; import openBlockInSidebar from "roamjs-components/writes/openBlockInSidebar"; import extractRef from "roamjs-components/util/extractRef"; +import { getFormattedConfigTree, notify } from "~/utils/discourseConfigRef"; +import { onSettingChange } from "~/components/settings/utils/settingsEmitter"; import { - getFormattedConfigTree, - notify, - subscribe, -} from "~/utils/discourseConfigRef"; -import type { - LeftSidebarConfig, - LeftSidebarPersonalSectionConfig, + type LeftSidebarConfig, + type LeftSidebarPersonalSectionConfig, + mergeGlobalSectionWithAccessor, + mergePersonalSectionsWithAccessor, } from "~/utils/getLeftSidebarSettings"; import { getGlobalSetting, @@ -135,7 +134,6 @@ const toggleFoldedState = ({ folded.value = true; } - // Dual-write to block props if (isGlobal) { setGlobalSetting(["Left sidebar", "Settings", "Folded"], newFolded); } else if (sectionIndex !== undefined) { @@ -308,7 +306,8 @@ const GlobalSection = ({ config }: { config: LeftSidebarConfig["global"] }) => { ); }; -// TODO(ENG-1471): Remove old-system merge when migration complete — just use accessor values directly +// TODO(ENG-1471): Remove old-system merge when migration complete — just use accessor values directly. +// See mergeGlobalSectionWithAccessor/mergePersonalSectionsWithAccessor for why the merge exists. const buildConfig = (): LeftSidebarConfig => { // Read VALUES from accessor (handles flag routing + mismatch detection) const globalValues = getGlobalSetting([ @@ -321,58 +320,17 @@ const buildConfig = (): LeftSidebarConfig => { // Read UIDs from old system (needed for fold CRUD during dual-write) const oldConfig = getFormattedConfigTree().leftSidebar; - // Merge: accessor values + old-system UIDs return { uid: oldConfig.uid, favoritesMigrated: oldConfig.favoritesMigrated, sidebarMigrated: oldConfig.sidebarMigrated, - global: { - uid: oldConfig.global.uid, - childrenUid: oldConfig.global.childrenUid, - children: oldConfig.global.children, - settings: oldConfig.global.settings - ? { - uid: oldConfig.global.settings.uid, - collapsable: { - uid: oldConfig.global.settings.collapsable.uid, - value: - globalValues?.Settings.Collapsable ?? - oldConfig.global.settings.collapsable.value, - }, - folded: { - uid: oldConfig.global.settings.folded.uid, - value: - globalValues?.Settings.Folded ?? - oldConfig.global.settings.folded.value, - }, - } - : undefined, - }, + global: mergeGlobalSectionWithAccessor(oldConfig.global, globalValues), personal: { uid: oldConfig.personal.uid, - sections: oldConfig.personal.sections.map((oldSection, i) => { - const newSection = personalValues?.[i]; - return { - ...oldSection, - settings: oldSection.settings - ? { - ...oldSection.settings, - truncateResult: { - ...oldSection.settings.truncateResult, - value: - newSection?.Settings?.["Truncate-result?"] ?? - oldSection.settings.truncateResult.value, - }, - folded: { - ...oldSection.settings.folded, - value: - newSection?.Settings?.Folded ?? - oldSection.settings.folded.value, - }, - } - : oldSection.settings, - }; - }), + sections: mergePersonalSectionsWithAccessor( + oldConfig.personal.sections, + personalValues, + ), }, allPersonalSections: oldConfig.allPersonalSections, }; @@ -384,14 +342,22 @@ export const useConfig = () => { const handleUpdate = () => { setConfig(buildConfig()); }; - const unsubscribe = subscribe(handleUpdate); + const unsubGlobal = onSettingChange("global:Left sidebar", handleUpdate); + const unsubPersonal = onSettingChange( + "personal:Left sidebar", + handleUpdate, + ); return () => { - unsubscribe(); + unsubGlobal(); + unsubPersonal(); }; }, []); return { config, setConfig }; }; +// TODO(ENG-1471): refreshAndNotify still needed for other subscribe() consumers +// (PanelManager, Canvas). Left sidebar reactivity now uses emitter, but other +// components still depend on notify(). Remove when all consumers migrate to emitter. export const refreshAndNotify = () => { refreshConfigTree(); notify(); diff --git a/apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx b/apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx index 645f6cdf7..4cd214fae 100644 --- a/apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx +++ b/apps/roam/src/components/settings/LeftSidebarGlobalSettings.tsx @@ -15,8 +15,11 @@ import { extractRef, getSubTree } from "roamjs-components/util"; import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; import discourseConfigRef from "~/utils/discourseConfigRef"; import { DISCOURSE_CONFIG_PAGE_TITLE } from "~/utils/renderNodeConfigPage"; -import { getLeftSidebarGlobalSectionConfig } from "~/utils/getLeftSidebarSettings"; -import { LeftSidebarGlobalSectionConfig } from "~/utils/getLeftSidebarSettings"; +import { + getLeftSidebarGlobalSectionConfig, + mergeGlobalSectionWithAccessor, + type LeftSidebarGlobalSectionConfig, +} from "~/utils/getLeftSidebarSettings"; import { render as renderToast } from "roamjs-components/components/Toast"; import refreshConfigTree from "~/utils/refreshConfigTree"; import { refreshAndNotify } from "~/components/LeftSidebarView"; @@ -102,10 +105,9 @@ const LeftSidebarGlobalSectionsContent = ({ const initialize = async () => { setIsInitializing(true); const globalSectionText = "Global-Section"; - // Dual-read: accessor validates values and logs mismatches when flag ON - getGlobalSetting(["Left sidebar"]); - - // Use old system data (has UIDs needed for CRUD operations) + const globalValues = getGlobalSetting([ + "Left sidebar", + ]); const config = getLeftSidebarGlobalSectionConfig(leftSidebar.children); const existingGlobalSection = leftSidebar.children.find( @@ -150,9 +152,10 @@ const LeftSidebarGlobalSectionsContent = ({ }); } } else { - setChildrenUid(config.childrenUid || null); - setPages(config.children || []); - setGlobalSection(config); + const merged = mergeGlobalSectionWithAccessor(config, globalValues); + setChildrenUid(merged.childrenUid || null); + setPages(merged.children || []); + setGlobalSection(merged); } setIsInitializing(false); }; diff --git a/apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx b/apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx index d36032a78..e897f2762 100644 --- a/apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx +++ b/apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx @@ -31,6 +31,7 @@ import { import { LeftSidebarPersonalSectionConfig, getLeftSidebarPersonalSectionConfig, + mergePersonalSectionsWithAccessor, PersonalSectionChild, } from "~/utils/getLeftSidebarSettings"; import { extractRef, getSubTree } from "roamjs-components/util"; @@ -604,14 +605,15 @@ const LeftSidebarPersonalSectionsContent = ({ } else { setPersonalSectionUid(personalSection.uid); - // Dual-read: accessor validates values and logs mismatches when flag ON - getPersonalSetting(["Left sidebar"]); - - // Use old system data (has UIDs needed for CRUD operations) + const personalValues = getPersonalSetting([ + "Left sidebar", + ]); const loadedSections = getLeftSidebarPersonalSectionConfig( leftSidebar.children, ).sections; - setSections(loadedSections); + setSections( + mergePersonalSectionsWithAccessor(loadedSections, personalValues), + ); } }; diff --git a/apps/roam/src/components/settings/utils/pullWatchers.ts b/apps/roam/src/components/settings/utils/pullWatchers.ts index 1b20c113a..992dc26c0 100644 --- a/apps/roam/src/components/settings/utils/pullWatchers.ts +++ b/apps/roam/src/components/settings/utils/pullWatchers.ts @@ -12,6 +12,7 @@ import { type PersonalSettings, type DiscourseNodeSettings, } from "./zodSchema"; +import { emitSettingChange } from "./settingsEmitter"; type PullWatchCallback = Parameters[2]; @@ -84,13 +85,6 @@ const addPullWatch = ( watches.push([pattern, entityId, callback]); }; -let leftSidebarFlagHandler: ((newValue: boolean) => void) | null = null; -export const setLeftSidebarFlagHandler = ( - handler: (newValue: boolean) => void, -) => { - leftSidebarFlagHandler = handler; -}; - export const featureFlagHandlers: Partial< Record< keyof FeatureFlags, @@ -99,8 +93,7 @@ export const featureFlagHandlers: Partial< > = { /* eslint-disable @typescript-eslint/naming-convention */ "Enable left sidebar": (newValue, oldValue) => { - if (newValue === oldValue) return; - leftSidebarFlagHandler?.(newValue); + emitSettingChange("Enable left sidebar", newValue, oldValue); }, /* eslint-enable @typescript-eslint/naming-convention */ }; @@ -114,12 +107,11 @@ type GlobalSettingsHandlers = { }; export const globalSettingsHandlers: GlobalSettingsHandlers = { - // Add handlers as needed: - // "Trigger": (newValue) => { ... }, - // "Canvas Page Format": (newValue) => { ... }, - // "Left Sidebar": (newValue) => { ... }, - // "Export": (newValue) => { ... }, - // "Suggestive Mode": (newValue) => { ... }, + /* eslint-disable @typescript-eslint/naming-convention */ + "Left sidebar": (newValue, oldValue) => { + emitSettingChange("global:Left sidebar", newValue, oldValue); + }, + /* eslint-enable @typescript-eslint/naming-convention */ }; type PersonalSettingsHandlers = { @@ -131,8 +123,11 @@ type PersonalSettingsHandlers = { }; export const personalSettingsHandlers: PersonalSettingsHandlers = { - // Add handlers as needed: - // "Left sidebar": (newValue) => { ... }, + /* eslint-disable @typescript-eslint/naming-convention */ + "Left sidebar": (newValue, oldValue) => { + emitSettingChange("personal:Left sidebar", newValue, oldValue); + }, + /* eslint-enable @typescript-eslint/naming-convention */ }; export const discourseNodeHandlers: Array< diff --git a/apps/roam/src/components/settings/utils/settingsEmitter.ts b/apps/roam/src/components/settings/utils/settingsEmitter.ts new file mode 100644 index 000000000..ad9029bd1 --- /dev/null +++ b/apps/roam/src/components/settings/utils/settingsEmitter.ts @@ -0,0 +1,24 @@ +type SettingChangeCallback = (newValue: unknown, oldValue: unknown) => void; + +const listeners = new Map>(); + +export const onSettingChange = ( + key: string, + callback: SettingChangeCallback, +): (() => void) => { + if (!listeners.has(key)) { + listeners.set(key, new Set()); + } + listeners.get(key)!.add(callback); + return () => { + listeners.get(key)?.delete(callback); + }; +}; + +export const emitSettingChange = ( + key: string, + newValue: unknown, + oldValue: unknown, +): void => { + listeners.get(key)?.forEach((cb) => cb(newValue, oldValue)); +}; diff --git a/apps/roam/src/index.ts b/apps/roam/src/index.ts index 4a7473530..176c095c4 100644 --- a/apps/roam/src/index.ts +++ b/apps/roam/src/index.ts @@ -38,10 +38,8 @@ import { getFeatureFlag, getPersonalSetting, } from "./components/settings/utils/accessors"; -import { - setupPullWatchOnSettingsPage, - setLeftSidebarFlagHandler, -} from "./components/settings/utils/pullWatchers"; +import { setupPullWatchOnSettingsPage } from "./components/settings/utils/pullWatchers"; +import { onSettingChange } from "./components/settings/utils/settingsEmitter"; import { mountLeftSidebar } from "./components/LeftSidebarView"; export const DEFAULT_CANVAS_PAGE_FORMAT = "Canvas/*"; @@ -155,26 +153,28 @@ export default runExtension(async (onloadArgs) => { }); } - // Register left sidebar flag handler before pull watchers start - const handleLeftSidebarFlag = (enabled: boolean) => { - const wrapper = document.querySelector( - ".starred-pages-wrapper", - ); - if (!wrapper) return; - if (enabled) { - wrapper.style.padding = "0"; - void mountLeftSidebar(wrapper, onloadArgs); - } else { - const root = wrapper.querySelector("#dg-left-sidebar-root"); - if (root) { - // eslint-disable-next-line react/no-deprecated - ReactDOM.unmountComponentAtNode(root); - root.remove(); + const unsubLeftSidebarFlag = onSettingChange( + "Enable left sidebar", + (newValue) => { + const enabled = Boolean(newValue); + const wrapper = document.querySelector( + ".starred-pages-wrapper", + ); + if (!wrapper) return; + if (enabled) { + wrapper.style.padding = "0"; + void mountLeftSidebar(wrapper, onloadArgs); + } else { + const root = wrapper.querySelector("#dg-left-sidebar-root"); + if (root) { + // eslint-disable-next-line react/no-deprecated + ReactDOM.unmountComponentAtNode(root); + root.remove(); + } + wrapper.style.padding = ""; } - wrapper.style.padding = ""; - } - }; - setLeftSidebarFlagHandler(handleLeftSidebarFlag); + }, + ); const { blockUids } = await initSchema(); const cleanupPullWatchers = setupPullWatchOnSettingsPage(blockUids); @@ -189,6 +189,7 @@ export default runExtension(async (onloadArgs) => { ], observers: observers, unload: () => { + unsubLeftSidebarFlag(); cleanupPullWatchers(); setSyncActivity(false); window.roamjs.extension?.smartblocks?.unregisterCommand("QUERYBUILDER"); diff --git a/apps/roam/src/utils/getLeftSidebarSettings.ts b/apps/roam/src/utils/getLeftSidebarSettings.ts index 94d018690..928f6a067 100644 --- a/apps/roam/src/utils/getLeftSidebarSettings.ts +++ b/apps/roam/src/utils/getLeftSidebarSettings.ts @@ -8,6 +8,10 @@ import { getUidAndStringSetting, } from "./getExportSettings"; import { getSubTree } from "roamjs-components/util"; +import type { + LeftSidebarGlobalSettings, + PersonalSection, +} from "~/components/settings/utils/zodSchema"; type LeftSidebarPersonalSectionSettings = { uid: string; @@ -199,6 +203,58 @@ export const getAllLeftSidebarPersonalSectionConfigs = ( return result; }; + +// TODO(ENG-1471): Remove when migration complete — just use accessor values directly. +// During dual-read, we need old-system UIDs for block CRUD (fold toggle, reorder, delete) +// but read setting VALUES from accessors (which route through the feature flag and log +// mismatches). These helpers merge accessor values onto old-system config objects. +export const mergeGlobalSectionWithAccessor = ( + config: LeftSidebarGlobalSectionConfig, + globalValues: LeftSidebarGlobalSettings | undefined, +): LeftSidebarGlobalSectionConfig => { + if (!config.settings) return config; + return { + ...config, + settings: { + uid: config.settings.uid, + collapsable: { + uid: config.settings.collapsable.uid, + value: + globalValues?.Settings.Collapsable ?? + config.settings.collapsable.value, + }, + folded: { + uid: config.settings.folded.uid, + value: globalValues?.Settings.Folded ?? config.settings.folded.value, + }, + }, + }; +}; + +export const mergePersonalSectionsWithAccessor = ( + sections: LeftSidebarPersonalSectionConfig[], + personalValues: PersonalSection[] | undefined, +): LeftSidebarPersonalSectionConfig[] => { + return sections.map((section, i) => { + const newSection = personalValues?.[i]; + if (!section.settings || !newSection) return section; + return { + ...section, + settings: { + ...section.settings, + truncateResult: { + ...section.settings.truncateResult, + value: newSection.Settings["Truncate-result?"], + }, + folded: { + ...section.settings.folded, + value: newSection.Settings.Folded, + }, + }, + }; + }); +}; + export const getLeftSidebarSettings = ( globalTree: RoamBasicNode[], ): LeftSidebarConfig => { From 686fd05ed0e1d69b72205d4c58e77436f86908fe Mon Sep 17 00:00:00 2001 From: sid597 Date: Tue, 3 Mar 2026 18:25:29 +0530 Subject: [PATCH 3/7] ENG-1478: refresh config tree before emitter-driven rebuild MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/roam/src/components/LeftSidebarView.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/roam/src/components/LeftSidebarView.tsx b/apps/roam/src/components/LeftSidebarView.tsx index af5cf2cbb..46b2dbb81 100644 --- a/apps/roam/src/components/LeftSidebarView.tsx +++ b/apps/roam/src/components/LeftSidebarView.tsx @@ -340,6 +340,7 @@ export const useConfig = () => { const [config, setConfig] = useState(() => buildConfig()); useEffect(() => { const handleUpdate = () => { + refreshConfigTree(); setConfig(buildConfig()); }; const unsubGlobal = onSettingChange("global:Left sidebar", handleUpdate); From eea529af7900ad3b23628813d8c05964614d7950 Mon Sep 17 00:00:00 2001 From: sid597 Date: Tue, 3 Mar 2026 19:31:36 +0530 Subject: [PATCH 4/7] ENG-1478: extract emitter key constants, fix stale TODO --- apps/roam/src/components/LeftSidebarView.tsx | 18 ++++++++++++------ .../components/settings/utils/pullWatchers.ts | 8 ++++---- .../settings/utils/settingsEmitter.ts | 6 ++++++ apps/roam/src/index.ts | 7 +++++-- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/apps/roam/src/components/LeftSidebarView.tsx b/apps/roam/src/components/LeftSidebarView.tsx index 46b2dbb81..da00355fe 100644 --- a/apps/roam/src/components/LeftSidebarView.tsx +++ b/apps/roam/src/components/LeftSidebarView.tsx @@ -22,7 +22,10 @@ import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTit import openBlockInSidebar from "roamjs-components/writes/openBlockInSidebar"; import extractRef from "roamjs-components/util/extractRef"; import { getFormattedConfigTree, notify } from "~/utils/discourseConfigRef"; -import { onSettingChange } from "~/components/settings/utils/settingsEmitter"; +import { + onSettingChange, + settingKeys, +} from "~/components/settings/utils/settingsEmitter"; import { type LeftSidebarConfig, type LeftSidebarPersonalSectionConfig, @@ -343,9 +346,12 @@ export const useConfig = () => { refreshConfigTree(); setConfig(buildConfig()); }; - const unsubGlobal = onSettingChange("global:Left sidebar", handleUpdate); + const unsubGlobal = onSettingChange( + settingKeys.globalLeftSidebar, + handleUpdate, + ); const unsubPersonal = onSettingChange( - "personal:Left sidebar", + settingKeys.personalLeftSidebar, handleUpdate, ); return () => { @@ -356,9 +362,9 @@ export const useConfig = () => { return { config, setConfig }; }; -// TODO(ENG-1471): refreshAndNotify still needed for other subscribe() consumers -// (PanelManager, Canvas). Left sidebar reactivity now uses emitter, but other -// components still depend on notify(). Remove when all consumers migrate to emitter. +// TODO(ENG-1471): refreshAndNotify still needed by settings panels +// (LeftSidebarGlobalSettings, LeftSidebarPersonalSettings) for old-system CRUD. +// Remove when settings panels also read via accessors + emitter. export const refreshAndNotify = () => { refreshConfigTree(); notify(); diff --git a/apps/roam/src/components/settings/utils/pullWatchers.ts b/apps/roam/src/components/settings/utils/pullWatchers.ts index 992dc26c0..e3d7e2979 100644 --- a/apps/roam/src/components/settings/utils/pullWatchers.ts +++ b/apps/roam/src/components/settings/utils/pullWatchers.ts @@ -12,7 +12,7 @@ import { type PersonalSettings, type DiscourseNodeSettings, } from "./zodSchema"; -import { emitSettingChange } from "./settingsEmitter"; +import { emitSettingChange, settingKeys } from "./settingsEmitter"; type PullWatchCallback = Parameters[2]; @@ -93,7 +93,7 @@ export const featureFlagHandlers: Partial< > = { /* eslint-disable @typescript-eslint/naming-convention */ "Enable left sidebar": (newValue, oldValue) => { - emitSettingChange("Enable left sidebar", newValue, oldValue); + emitSettingChange(settingKeys.leftSidebarFlag, newValue, oldValue); }, /* eslint-enable @typescript-eslint/naming-convention */ }; @@ -109,7 +109,7 @@ type GlobalSettingsHandlers = { export const globalSettingsHandlers: GlobalSettingsHandlers = { /* eslint-disable @typescript-eslint/naming-convention */ "Left sidebar": (newValue, oldValue) => { - emitSettingChange("global:Left sidebar", newValue, oldValue); + emitSettingChange(settingKeys.globalLeftSidebar, newValue, oldValue); }, /* eslint-enable @typescript-eslint/naming-convention */ }; @@ -125,7 +125,7 @@ type PersonalSettingsHandlers = { export const personalSettingsHandlers: PersonalSettingsHandlers = { /* eslint-disable @typescript-eslint/naming-convention */ "Left sidebar": (newValue, oldValue) => { - emitSettingChange("personal:Left sidebar", newValue, oldValue); + emitSettingChange(settingKeys.personalLeftSidebar, newValue, oldValue); }, /* eslint-enable @typescript-eslint/naming-convention */ }; diff --git a/apps/roam/src/components/settings/utils/settingsEmitter.ts b/apps/roam/src/components/settings/utils/settingsEmitter.ts index ad9029bd1..9a72b5155 100644 --- a/apps/roam/src/components/settings/utils/settingsEmitter.ts +++ b/apps/roam/src/components/settings/utils/settingsEmitter.ts @@ -1,5 +1,11 @@ type SettingChangeCallback = (newValue: unknown, oldValue: unknown) => void; +export const settingKeys = { + leftSidebarFlag: "Enable left sidebar", + globalLeftSidebar: "global:Left sidebar", + personalLeftSidebar: "personal:Left sidebar", +} as const; + const listeners = new Map>(); export const onSettingChange = ( diff --git a/apps/roam/src/index.ts b/apps/roam/src/index.ts index 176c095c4..84ecb6ff4 100644 --- a/apps/roam/src/index.ts +++ b/apps/roam/src/index.ts @@ -39,7 +39,10 @@ import { getPersonalSetting, } from "./components/settings/utils/accessors"; import { setupPullWatchOnSettingsPage } from "./components/settings/utils/pullWatchers"; -import { onSettingChange } from "./components/settings/utils/settingsEmitter"; +import { + onSettingChange, + settingKeys, +} from "./components/settings/utils/settingsEmitter"; import { mountLeftSidebar } from "./components/LeftSidebarView"; export const DEFAULT_CANVAS_PAGE_FORMAT = "Canvas/*"; @@ -154,7 +157,7 @@ export default runExtension(async (onloadArgs) => { } const unsubLeftSidebarFlag = onSettingChange( - "Enable left sidebar", + settingKeys.leftSidebarFlag, (newValue) => { const enabled = Boolean(newValue); const wrapper = document.querySelector( From 5925be205557de14ebc5b28fa39925c360582e7d Mon Sep 17 00:00:00 2001 From: sid597 Date: Tue, 3 Mar 2026 21:17:46 +0530 Subject: [PATCH 5/7] =?UTF-8?q?ENG-1478:=20fix=20legacy=20adapter=20child.?= =?UTF-8?q?uid=20=E2=86=92=20child.text,=20fix=20settingKeys=20on=20person?= =?UTF-8?q?al=20panels?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/components/settings/LeftSidebarPersonalSettings.tsx | 4 ++-- apps/roam/src/components/settings/utils/accessors.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx b/apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx index e897f2762..eef335d2b 100644 --- a/apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx +++ b/apps/roam/src/components/settings/LeftSidebarPersonalSettings.tsx @@ -512,7 +512,7 @@ const SectionItem = memo( { return settings.leftSidebar.personal.sections.map((section) => ({ name: section.text, Children: (section.children || []).map((child) => ({ - uid: child.uid, + uid: child.text, Alias: child.alias?.value || "", })), Settings: { From ee8bf564c5b329c0f5939b6c278af5ac2c55f307 Mon Sep 17 00:00:00 2001 From: sid597 Date: Tue, 3 Mar 2026 21:47:20 +0530 Subject: [PATCH 6/7] ENG-1478: skip reload prompt when new settings store is active --- apps/roam/src/components/settings/GeneralSettings.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/roam/src/components/settings/GeneralSettings.tsx b/apps/roam/src/components/settings/GeneralSettings.tsx index c3f5a6bfc..875015700 100644 --- a/apps/roam/src/components/settings/GeneralSettings.tsx +++ b/apps/roam/src/components/settings/GeneralSettings.tsx @@ -6,6 +6,7 @@ import { GlobalTextPanel, FeatureFlagPanel, } from "./components/BlockPropSettingPanels"; +import { isNewSettingsStoreEnabled } from "./utils/accessors"; import posthog from "posthog-js"; const DiscourseGraphHome = () => { @@ -43,7 +44,7 @@ const DiscourseGraphHome = () => { uid={settings.leftSidebarEnabled.uid} parentUid={settings.settingsUid} onAfterChange={(checked: boolean) => { - if (checked) { + if (checked && !isNewSettingsStoreEnabled()) { setIsAlertOpen(true); } posthog.capture("General Settings: Left Sidebar Toggled", { From ade239b14111607d88ea4a026181d492a50bd01b Mon Sep 17 00:00:00 2001 From: sid597 Date: Tue, 3 Mar 2026 23:04:14 +0530 Subject: [PATCH 7/7] ENG-1478: guard getGlobalSetting/getPersonalSetting against empty keys --- apps/roam/src/components/settings/utils/accessors.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/roam/src/components/settings/utils/accessors.ts b/apps/roam/src/components/settings/utils/accessors.ts index 50f221bbf..ade9c16f7 100644 --- a/apps/roam/src/components/settings/utils/accessors.ts +++ b/apps/roam/src/components/settings/utils/accessors.ts @@ -724,6 +724,8 @@ export const getGlobalSettings = (): GlobalSettings => { export const getGlobalSetting = ( keys: string[], ): T | undefined => { + if (keys.length === 0) return undefined; + if (!isNewSettingsStoreEnabled()) { return getLegacyGlobalSetting(keys) as T | undefined; } @@ -788,6 +790,8 @@ export const getPersonalSettings = (): PersonalSettings => { export const getPersonalSetting = ( keys: string[], ): T | undefined => { + if (keys.length === 0) return undefined; + if (!isNewSettingsStoreEnabled()) { return getLegacyPersonalSetting(keys) as T | undefined; }