From 9ffa9821958e7b15fc5639eac58b871ad527ccf7 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Tue, 8 Sep 2026 13:04:03 -0400 Subject: [PATCH 1/3] =?UTF-8?q?ENG-2186=20Grammar=20=E2=80=BA=20Nodes=20dr?= =?UTF-8?q?ill-down=20and=20settings=20navigation=20primitive?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the per-node-type rail tabs with a single Grammar › Nodes entry that drills from the node list into a node type's page, with a back button and a breadcrumb. The route state is a path reducer (select-tab / push / pop / truncate) exposed through SettingsNavContext; an incoming id that is not a known tab is treated as a node type uid so the old per-node deep links keep resolving. Also carries the write plumbing the drill-down made necessary. Panels debounce their writes and used to cancel the pending one on unmount; a back button puts an unmount one keystroke from every input, so a commit now runs exactly once by timer, flush, or unmount (useDeferredWrite). Legacy block writes go through an awaitable hook registered with the same in-flight tracker, and every commit re-reads the config tree only after its write resolves, so a reader that flushes first (the export) sees the value it was just given. The node type page itself keeps its inner tabs here; ENG-2188 rebuilds it. Co-Authored-By: Claude Fable 5.1 --- .../settings/DiscourseNodeConfigPanel.tsx | 28 +-- .../components/settings/GrammarNodesRoute.tsx | 59 ++++++ .../src/components/settings/NodeConfig.tsx | 10 + .../roam/src/components/settings/Settings.tsx | 73 ++++---- .../components/BlockPropSettingPanels.tsx | 172 ++++++++++++------ .../components/EphemeralBlocksPanel.tsx | 59 ++++-- .../navigation/SettingsNavContext.tsx | 53 ++++++ .../navigation/SettingsPageHeader.tsx | 75 ++++++++ .../settings/utils/settingsNavigation.ts | 83 +++++++++ .../components/settings/utils/settingsTabs.ts | 12 +- apps/roam/src/styles/settingsStyles.css | 54 ++++++ apps/roam/src/utils/pendingSettingWrites.ts | 26 +++ apps/roam/src/utils/setBlockProps.ts | 24 ++- 13 files changed, 592 insertions(+), 136 deletions(-) create mode 100644 apps/roam/src/components/settings/GrammarNodesRoute.tsx create mode 100644 apps/roam/src/components/settings/navigation/SettingsNavContext.tsx create mode 100644 apps/roam/src/components/settings/navigation/SettingsPageHeader.tsx create mode 100644 apps/roam/src/components/settings/utils/settingsNavigation.ts create mode 100644 apps/roam/src/utils/pendingSettingWrites.ts diff --git a/apps/roam/src/components/settings/DiscourseNodeConfigPanel.tsx b/apps/roam/src/components/settings/DiscourseNodeConfigPanel.tsx index e543966c3..b1b3ae4fa 100644 --- a/apps/roam/src/components/settings/DiscourseNodeConfigPanel.tsx +++ b/apps/roam/src/components/settings/DiscourseNodeConfigPanel.tsx @@ -8,9 +8,10 @@ import { Tooltip, } from "@blueprintjs/core"; import React, { useState } from "react"; -import getDiscourseNodes from "~/utils/getDiscourseNodes"; +import getDiscourseNodes, { + excludeDefaultNodes, +} from "~/utils/getDiscourseNodes"; import refreshConfigTree from "~/utils/refreshConfigTree"; -import type { CustomField } from "roamjs-components/components/ConfigPanels/types"; import posthog from "posthog-js"; import getDiscourseRelations, { type DiscourseRelation, @@ -24,20 +25,12 @@ import { } from "./utils/accessors"; import { GLOBAL_KEYS } from "./utils/settingKeys"; import { invalidateDiscourseNodeTypeCaches } from "~/utils/discourseNodeTypeCache"; +import { useSettingsNav } from "./navigation/SettingsNavContext"; -type DiscourseNodeConfigPanelProps = React.ComponentProps< - CustomField["options"]["component"] -> & { - isPopup?: boolean; - setSelectedTabId: (id: string) => void; -}; - -const DiscourseNodeConfigPanel: React.FC = ({ - isPopup, - setSelectedTabId, -}) => { +const DiscourseNodeConfigPanel: React.FC = () => { + const { push } = useSettingsNav(); const [nodes, setNodes] = useState(() => - getDiscourseNodes().filter((n) => n.backedBy === "user"), + getDiscourseNodes().filter(excludeDefaultNodes), ); const [label, setLabel] = useState(""); const [isCreating, setIsCreating] = useState(false); @@ -52,11 +45,8 @@ const DiscourseNodeConfigPanel: React.FC = ({ >([]); const [nodeTypeIdToDelete, setNodeTypeIdToDelete] = useState(""); const navigateToNode = (uid: string) => { - if (isPopup) { - setSelectedTabId(uid); - } else { - window.roamAlphaAPI.ui.mainWindow.openPage({ page: { uid } }); - } + push(uid); + posthog.capture("Settings: Node Type Opened", { nodeTypeUid: uid }); }; const createNodeType = async (): Promise => { diff --git a/apps/roam/src/components/settings/GrammarNodesRoute.tsx b/apps/roam/src/components/settings/GrammarNodesRoute.tsx new file mode 100644 index 000000000..5662c5ece --- /dev/null +++ b/apps/roam/src/components/settings/GrammarNodesRoute.tsx @@ -0,0 +1,59 @@ +import React, { useEffect } from "react"; +import { OnloadArgs } from "roamjs-components/types"; +import getDiscourseNodes, { + excludeDefaultNodes, +} from "~/utils/getDiscourseNodes"; +import { formatHexColor } from "./DiscourseNodeCanvasSettings"; +import { useSettingsNav } from "./navigation/SettingsNavContext"; +import SettingsPageHeader from "./navigation/SettingsPageHeader"; +import DiscourseNodeConfigPanel from "./DiscourseNodeConfigPanel"; +import NodeConfig from "./NodeConfig"; + +const NODES_ANCESTOR_LABELS = ["Grammar"] as const; + +const GrammarNodesRoute = ({ + onloadArgs, +}: { + onloadArgs: OnloadArgs; +}): JSX.Element => { + const { segments, goToDepth } = useSettingsNav(); + const nodes = getDiscourseNodes().filter(excludeDefaultNodes); + + const [nodeTypeUid] = segments; + const node = nodeTypeUid + ? nodes.find((n) => n.type === nodeTypeUid) + : undefined; + + // A deleted node type or stale deep link resolves to nothing; return to the list. + const isStalePath = Boolean(nodeTypeUid) && !node; + useEffect(() => { + if (isStalePath) goToDepth(0); + }, [isStalePath, goToDepth]); + + const resolveLabel = (segment: string): string => + nodes.find((n) => n.type === segment)?.text ?? segment; + + return ( +
+ +
+ {node ? ( + + ) : ( +
+ +
+ )} +
+
+ ); +}; + +export default GrammarNodesRoute; diff --git a/apps/roam/src/components/settings/NodeConfig.tsx b/apps/roam/src/components/settings/NodeConfig.tsx index 92e68d5dd..b4114b63b 100644 --- a/apps/roam/src/components/settings/NodeConfig.tsx +++ b/apps/roam/src/components/settings/NodeConfig.tsx @@ -81,11 +81,18 @@ const DiscourseNodeColorSetting = ({ [canvasUid, nodeType], ); + // Navigating away unmounts mid-debounce, so the pending colour is written rather than dropped. + const pendingColorRef = useRef(null); + const persistColorValueRef = useRef(persistColorValue); + persistColorValueRef.current = persistColorValue; useEffect(() => { return () => { if (!colorWriteTimeoutRef.current) return; window.clearTimeout(colorWriteTimeoutRef.current); + const pending = pendingColorRef.current; + pendingColorRef.current = null; + if (pending !== null) persistColorValueRef.current(pending); }; }, []); @@ -94,8 +101,10 @@ const DiscourseNodeColorSetting = ({ window.clearTimeout(colorWriteTimeoutRef.current); colorWriteTimeoutRef.current = null; } + pendingColorRef.current = colorValue; colorWriteTimeoutRef.current = window.setTimeout(() => { persistColorValue(colorValue); + pendingColorRef.current = null; colorWriteTimeoutRef.current = null; }, COLOR_WRITE_DEBOUNCE_MS); }; @@ -134,6 +143,7 @@ const DiscourseNodeColorSetting = ({ window.clearTimeout(colorWriteTimeoutRef.current); colorWriteTimeoutRef.current = null; } + pendingColorRef.current = null; setColor(""); persistColorValue(""); }} diff --git a/apps/roam/src/components/settings/Settings.tsx b/apps/roam/src/components/settings/Settings.tsx index e2f0e7cba..fcba1a38e 100644 --- a/apps/roam/src/components/settings/Settings.tsx +++ b/apps/roam/src/components/settings/Settings.tsx @@ -1,4 +1,11 @@ -import React, { useEffect, useMemo, useState } from "react"; +import React, { + useCallback, + useEffect, + useMemo, + useReducer, + useRef, + useState, +} from "react"; import { OnloadArgs } from "roamjs-components/types"; import { Classes, @@ -16,11 +23,6 @@ import discourseConfigRef from "~/utils/discourseConfigRef"; import DiscourseGraphExport from "./ExportSettings"; import QuerySettings from "./QuerySettings"; import AdminPanel from "./AdminPanel"; -import DiscourseNodeConfigPanel from "./DiscourseNodeConfigPanel"; -import getDiscourseNodes, { - excludeDefaultNodes, -} from "~/utils/getDiscourseNodes"; -import NodeConfig from "./NodeConfig"; import PreferencesGeneral from "./PreferencesGeneral"; import PreferencesStyling from "./PreferencesStyling"; import LeftSidebarSettings from "./LeftSidebarSettings"; @@ -32,7 +34,14 @@ import { getVersionWithDate } from "~/utils/getVersion"; import posthog from "posthog-js"; import { bulkReadSettings } from "./utils/accessors"; import { onSettingChange, settingKeys } from "./utils/settingsEmitter"; -import { SETTINGS_TAB_IDS, resolveSettingsTabId } from "./utils/settingsTabs"; +import { SETTINGS_TAB_IDS } from "./utils/settingsTabs"; +import { + resolveInitialSettingsPath, + settingsNavReducer, + tabIdOf, +} from "./utils/settingsNavigation"; +import { SettingsNavProvider } from "./navigation/SettingsNavContext"; +import GrammarNodesRoute from "./GrammarNodesRoute"; const SectionHeader = ({ children }: { children: React.ReactNode }) => (
@@ -77,10 +86,15 @@ export const SettingsDialog = ({ const relationsNode = grammarNode?.children.find( (node) => node.text === "relations", ); - const nodesNode = grammarNode?.children.find((node) => node.text === "nodes"); - const nodes = getDiscourseNodes().filter(excludeDefaultNodes); - const [activeTabId, setActiveTabId] = useState(() => - resolveSettingsTabId(selectedTabId), + const [path, dispatch] = useReducer( + settingsNavReducer, + selectedTabId, + resolveInitialSettingsPath, + ); + const activeTabId = tabIdOf(path); + const selectTab = useCallback( + (tabId: string) => dispatch({ type: "select-tab", tabId }), + [], ); // eslint-disable-next-line react-hooks/exhaustive-deps const settings = useMemo(() => bulkReadSettings(), [activeTabId]); @@ -98,15 +112,14 @@ export const SettingsDialog = ({ const { versionStamp } = getVersionWithDate(); const openAdminPanel = (): void => { setShowAdminPanel(true); - setActiveTabId(SETTINGS_TAB_IDS.admin); + selectTab(SETTINGS_TAB_IDS.admin); posthog.capture("Settings: Admin Panel Opened from Footer"); }; + const initialTabId = useRef(activeTabId).current; useEffect(() => { - posthog.capture("Settings: Dialog Opened", { - initialTabId: String(resolveSettingsTabId(selectedTabId)), - }); - }, [selectedTabId]); + posthog.capture("Settings: Dialog Opened", { initialTabId }); + }, [initialTabId]); useEffect(() => { const handleKeyPress = (e: KeyboardEvent) => { @@ -114,14 +127,14 @@ export const SettingsDialog = ({ e.stopPropagation(); e.preventDefault(); setShowAdminPanel(true); - setActiveTabId(SETTINGS_TAB_IDS.admin); + selectTab(SETTINGS_TAB_IDS.admin); posthog.capture("Settings: Admin Panel Opened via Shortcut"); } }; window.addEventListener("keydown", handleKeyPress); return () => window.removeEventListener("keydown", handleKeyPress); - }, []); + }, [selectTab]); return ( { - setActiveTabId(id); + selectTab(String(id)); posthog.capture("Settings: Tab Opened", { tabId: String(id), }); @@ -238,16 +251,10 @@ export const SettingsDialog = ({ + + + } /> } /> - Node types - {nodes.map((n) => ( - } - /> - ))} Advanced void | Promise; + +type DeferredWrite = { + schedule: (commit: Commit, delayMs: number) => void; +}; + +type LegacyBlockSync = (text: string) => Promise; + +// The legacy config tree is what readers see while `Use new settings store` is +// off, so a commit must be able to await this write before re-reading the tree. +const useLegacyBlockSync = ({ + title, + parentUid, + order, + uid, +}: { + title: string; + parentUid?: string; + order?: number; + uid?: string; +}): LegacyBlockSync | undefined => { + const uidRef = useRef(uid); + const valueUidRef = useRef(uid ? getFirstChildUidByBlockUid(uid) : ""); + const enabled = parentUid !== undefined && order !== undefined; + const sync = useCallback( + async (text: string): Promise => { + if (valueUidRef.current) { + await trackRoamWrite(updateBlock({ uid: valueUidRef.current, text })); + return; + } + if (!uidRef.current) { + uidRef.current = await trackRoamWrite( + createBlock({ + node: { text: title }, + parentUid: parentUid ?? "", + order: order ?? 0, + }), + ); + } + valueUidRef.current = await trackRoamWrite( + createBlock({ node: { text }, parentUid: uidRef.current, order: 0 }), + ); + }, + [title, parentUid, order], + ); + return enabled ? sync : undefined; +}; + +// One timer and one registry entry per panel: a commit runs exactly once, by timer, +// flush, or unmount. Unmount commits rather than cancels. +const useDeferredWrite = (): DeferredWrite => { + const timeoutRef = useRef(0); + const commitRef = useRef(null); + + const forget = useCallback(() => { + window.clearTimeout(timeoutRef.current); + if (commitRef.current) { + removePendingSettingWrite(commitRef.current); + commitRef.current = null; + } + }, []); + + const schedule = useCallback( + (commit: Commit, delayMs: number) => { + forget(); + const runOnce = (): void | Promise => { + forget(); + return commit(); + }; + commitRef.current = runOnce; + addPendingSettingWrite(runOnce); + timeoutRef.current = window.setTimeout(() => void runOnce(), delayMs); + }, + [forget], + ); + + useEffect(() => () => void commitRef.current?.(), []); + + return { schedule }; +}; const BaseTextPanel = ({ title, @@ -135,22 +224,13 @@ const BaseTextPanel = ({ const [value, setValue] = useState(() => initialValue ?? ""); const errorRef = useRef(error); errorRef.current = error; - const debounceRef = useRef(0); - const hasBlockSync = parentUid !== undefined && order !== undefined; - const { onChange: rawSyncToBlock } = useSingleChildValue({ + const { schedule } = useDeferredWrite(); + const syncToBlock = useLegacyBlockSync({ title: blockKey ?? title, - parentUid: parentUid ?? "", - order: order ?? 0, + parentUid, + order, uid, - defaultValue: initialValue ?? "", - transform: (s: string) => s, - toStr: (s: string) => s, }); - const syncToBlock = hasBlockSync ? rawSyncToBlock : undefined; - - useEffect(() => { - return () => window.clearTimeout(debounceRef.current); - }, []); const handleChange = ( e: ChangeEvent, @@ -159,15 +239,11 @@ const BaseTextPanel = ({ setValue(newValue); onChange?.(newValue); - window.clearTimeout(debounceRef.current); - debounceRef.current = window.setTimeout(() => { + schedule(async () => { if (errorRef.current) return; - syncToBlock?.(newValue); - debounceRef.current = window.setTimeout(() => { - if (errorRef.current) return; - refreshConfigTree(); - setter(settingKeys, newValue); - }, 100); + await syncToBlock?.(newValue); + setter(settingKeys, newValue); + refreshConfigTree(); }, DEBOUNCE_MS); }; @@ -283,33 +359,23 @@ const BaseNumberPanel = ({ blockKey, }: BaseNumberPanelProps) => { const [value, setValue] = useState(() => initialValue ?? 0); - const hasBlockSync = parentUid !== undefined && order !== undefined; - const { onChange: rawSyncToBlock } = useSingleChildValue({ + const syncToBlock = useLegacyBlockSync({ title: blockKey ?? title, - parentUid: parentUid ?? "", - order: order ?? 0, + parentUid, + order, uid, - defaultValue: initialValue ?? 0, - transform: (s: string) => parseInt(s, 10), - toStr: (v: number) => `${v}`, }); - const syncToBlock = hasBlockSync ? rawSyncToBlock : undefined; - const refreshTimeoutRef = useRef(0); - - useEffect(() => { - return () => window.clearTimeout(refreshTimeoutRef.current); - }, []); + const { schedule } = useDeferredWrite(); const handleChange = (valueAsNumber: number) => { if (Number.isNaN(valueAsNumber)) return; setValue(valueAsNumber); - syncToBlock?.(valueAsNumber); - window.clearTimeout(refreshTimeoutRef.current); - refreshTimeoutRef.current = window.setTimeout(() => { - refreshConfigTree(); + schedule(async () => { + await syncToBlock?.(`${valueAsNumber}`); setter(settingKeys, valueAsNumber); + refreshConfigTree(); onChange?.(valueAsNumber); - }, 100); + }, SHORT_DEBOUNCE_MS); }; return ( @@ -339,32 +405,22 @@ const BaseSelectPanel = ({ blockKey, }: BaseSelectPanelProps) => { const [value, setValue] = useState(() => initialValue ?? options[0]); - const hasBlockSync = parentUid !== undefined && order !== undefined; - const { onChange: rawSyncToBlock } = useSingleChildValue({ + const syncToBlock = useLegacyBlockSync({ title: blockKey ?? title, - parentUid: parentUid ?? "", - order: order ?? 0, + parentUid, + order, uid, - defaultValue: initialValue ?? options[0] ?? "", - transform: (s: string) => s, - toStr: (s: string) => s, }); - const syncToBlock = hasBlockSync ? rawSyncToBlock : undefined; - const refreshTimeoutRef = useRef(0); - - useEffect(() => { - return () => window.clearTimeout(refreshTimeoutRef.current); - }, []); + const { schedule } = useDeferredWrite(); const handleChange = (e: ChangeEvent) => { const newValue = e.target.value; setValue(newValue); - syncToBlock?.(newValue); - window.clearTimeout(refreshTimeoutRef.current); - refreshTimeoutRef.current = window.setTimeout(() => { - refreshConfigTree(); + schedule(async () => { + await syncToBlock?.(newValue); setter(settingKeys, newValue); - }, 100); + refreshConfigTree(); + }, SHORT_DEBOUNCE_MS); }; return ( diff --git a/apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx b/apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx index 3e0fd8aa6..25cfa7ee0 100644 --- a/apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx +++ b/apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx @@ -16,7 +16,8 @@ import type { DiscourseNodeBaseProps } from "./BlockPropSettingPanels"; const DEBOUNCE_MS = 250; const TEMPLATE_BUFFER_TEXT = "Template"; -type DualWriteBlocksPanelProps = DiscourseNodeBaseProps & { +type DualWriteBlocksPanelProps = Omit & { + title?: string; uid: string; defaultValue?: InputTextNode[]; }; @@ -115,7 +116,7 @@ const DualWriteBlocksPanel = ({ const newUid = window.roamAlphaAPI.util.generateUID(); const dv = defaultValueRef.current; const seed: InputTextNode[] = dv && dv.length > 0 ? dv : [{ text: " " }]; - void createBlock({ + const created = createBlock({ node: { text: TEMPLATE_BUFFER_TEXT, uid: newUid, children: seed }, parentUid: nodeType, order: "last", @@ -125,23 +126,44 @@ const DualWriteBlocksPanel = ({ return () => { cancelled = true; setBufferUid(null); - void deleteBlock(newUid); + // Deleting a uid whose createBlock is still in flight orphans the buffer block. + void created.then( + () => deleteBlock(newUid), + () => undefined, + ); }; }, [isNewStore, nodeType]); + const writeChanges = useCallback(() => { + if (!renderUid) return; + const tree = getFullTreeByParentUid(renderUid); + const serialized = serializeBlockTree(tree.children); + setDiscourseNodeSetting(nodeType, settingKeys, serialized); + if (isNewStore && renderUid !== uid) { + const legacyTree = getFullTreeByParentUid(uid); + mirrorBufferToLegacyChildren(tree.children, legacyTree.children, uid); + } + }, [renderUid, uid, isNewStore, nodeType, settingKeys]); + + // In a ref so unmount cleanup can flush without re-running on every identity change. + const writeChangesRef = useRef(writeChanges); + writeChangesRef.current = writeChanges; + + const flushPendingChanges = useCallback(() => { + if (!debounceRef.current) return; + window.clearTimeout(debounceRef.current); + debounceRef.current = 0; + writeChangesRef.current(); + }, []); + const handleChange = useCallback(() => { if (!renderUid) return; window.clearTimeout(debounceRef.current); debounceRef.current = window.setTimeout(() => { - const tree = getFullTreeByParentUid(renderUid); - const serialized = serializeBlockTree(tree.children); - setDiscourseNodeSetting(nodeType, settingKeys, serialized); - if (isNewStore && renderUid !== uid) { - const legacyTree = getFullTreeByParentUid(uid); - mirrorBufferToLegacyChildren(tree.children, legacyTree.children, uid); - } + debounceRef.current = 0; + writeChangesRef.current(); }, DEBOUNCE_MS); - }, [renderUid, uid, isNewStore, nodeType, settingKeys]); + }, [renderUid]); useEffect(() => { const el = containerRef.current; @@ -181,20 +203,23 @@ const DualWriteBlocksPanel = ({ return () => { cancelled = true; - window.clearTimeout(debounceRef.current); + // Navigating away lands right after a keystroke, and the buffer block is deleted next. + flushPendingChanges(); if (pullWatchArgsRef.current) { window.roamAlphaAPI.data.removePullWatch(...pullWatchArgsRef.current); pullWatchArgsRef.current = null; } }; - }, [renderUid, handleChange]); + }, [renderUid, handleChange, flushPendingChanges]); return ( <> - + {title ? ( + + ) : null}