Skip to content

ENG-1456: Migrate personal boolean flag consumers (getSetting → getPersonalSetting) #814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sid597 merged 2 commits into from
Mar 2, 2026
Merged

ENG-1456: Migrate personal boolean flag consumers (getSetting → getPersonalSetting) #814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bcb6e27
ENG-1472: Refactor BlockPropSettingPanels to add accessor-backed defa…
sid597 Mar 1, 2026
185006d
ENG-1456: Migrate personal boolean flag consumers (getSetting → getPe…
sid597 Feb 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion apps/roam/src/components/DiscourseFloatingMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import { FeedbackWidget } from "./BirdEatsBugs";
import { render as renderSettings } from "~/components/settings/Settings";
import posthog from "posthog-js";
import { getPersonalSetting } from "./settings/utils/accessors";

type DiscourseFloatingMenuProps = {
// CSS placement class
Expand Down Expand Up @@ -128,7 +129,7 @@ export const installDiscourseFloatingMenu = (
floatingMenuAnchor.id = ANCHOR_ID;
document.getElementById("app")?.appendChild(floatingMenuAnchor);
}
if (onLoadArgs.extensionAPI.settings.get("hide-feedback-button") as boolean) {
if (getPersonalSetting<boolean>(["Hide feedback button"])) {
floatingMenuAnchor.classList.add("hidden");
}
Comment thread
sid597 marked this conversation as resolved.
ReactDOM.render(
Expand Down
15 changes: 5 additions & 10 deletions apps/roam/src/components/canvas/DiscourseNodeUtil.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,7 @@ import calcCanvasNodeSizeAndImg from "~/utils/calcCanvasNodeSizeAndImg";
import { createTextJsxFromSpans } from "./DiscourseRelationShape/helpers";
import { loadImage } from "~/utils/loadImage";
import { getRelationColor } from "./DiscourseRelationShape/DiscourseRelationUtil";
import {
AUTO_CANVAS_RELATIONS_KEY,
DISCOURSE_CONTEXT_OVERLAY_IN_CANVAS_KEY,
} from "~/data/userSettings";
import { getSetting } from "~/utils/extensionSettings";
import { getPersonalSetting } from "~/components/settings/utils/accessors";
import DiscourseContextOverlay from "~/components/DiscourseContextOverlay";
import { getDiscourseNodeColors } from "~/utils/getDiscourseNodeColors";
import { render as renderToast } from "roamjs-components/components/Toast";
Expand Down Expand Up @@ -435,7 +431,7 @@ export class BaseDiscourseNodeUtil extends BaseBoxShapeUtil<DiscourseNodeShape>
} = discourseContext.nodes[shape.type] || {};
// eslint-disable-next-line react-hooks/rules-of-hooks
const isOverlayEnabled = useMemo(
() => getSetting(DISCOURSE_CONTEXT_OVERLAY_IN_CANVAS_KEY, false),
() => getPersonalSetting<boolean>(["Overlay in canvas"]),
Copy link
Copy Markdown
Member

@mdroidian mdroidian Feb 28, 2026

Choose a reason for hiding this comment

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

Could we promote this to a shared constant or typed key? That would give us autocomplete and eliminate the risk of subtle string mismatches.

Ideally that's something we look into doing before merging to main.

Copy link
Copy Markdown
Collaborator Author

@sid597 sid597 Mar 1, 2026

Choose a reason for hiding this comment

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

We can, it depends on what the end goal is. I am coming from the pov that this is intermediate code that is not going to be merged to main, we are merging all this to a staging branch, testing it thoroughly internally then removing the intermediate code i.e both the dual-write and dual-read.

So if this is intermediate then we should not update this code, if the end goal is that we will merge this main and then do real world testing and monitoring, and at some point in future cleanup the intermediate code, then we should do this.

Copy link
Copy Markdown
Member

@mdroidian mdroidian Mar 1, 2026

Choose a reason for hiding this comment

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

It can be for the future, as long as it is tracked somewhere. Ideally a linear ticket.

Copy link
Copy Markdown
Collaborator Author

@sid597 sid597 Mar 2, 2026

Choose a reason for hiding this comment

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

ENG-1499: Convert legacy-blockprop key pairs to shared constant or typed key

[],
);
Comment thread
sid597 marked this conversation as resolved.

Expand Down Expand Up @@ -516,10 +512,9 @@ export class BaseDiscourseNodeUtil extends BaseBoxShapeUtil<DiscourseNodeShape>
uid,
});

const autoCanvasRelations = getSetting<boolean>(
AUTO_CANVAS_RELATIONS_KEY,
false,
);
const autoCanvasRelations = getPersonalSetting<boolean>([
"Auto canvas relations",
]);
if (autoCanvasRelations) {
try {
Comment thread
sid597 marked this conversation as resolved.
const relationIds = getRelationIds();
Expand Down
10 changes: 4 additions & 6 deletions apps/roam/src/components/canvas/Tldraw.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,14 @@ import ToastListener, { dispatchToastEvent } from "./ToastListener";
import { CanvasDrawerPanel } from "./CanvasDrawer";
import { ClipboardPanel, ClipboardProvider } from "./Clipboard";
import internalError from "~/utils/internalError";
import { AUTO_CANVAS_RELATIONS_KEY } from "~/data/userSettings";
import { getSetting } from "~/utils/extensionSettings";
import { isPluginTimerReady, waitForPluginTimer } from "~/utils/pluginTimer";
import { HistoryEntry } from "@tldraw/store";
import { TLRecord } from "@tldraw/tlschema";
import { WHITE_LOGO_SVG } from "~/icons";
import { BLOCK_REF_REGEX } from "roamjs-components/dom";
import { defaultHandleExternalTextContent } from "./defaultHandleExternalTextContent";
import posthog from "posthog-js";
import { getPersonalSetting } from "~/components/settings/utils/accessors";

declare global {
// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
Expand Down Expand Up @@ -1132,10 +1131,9 @@ const InsideEditorAndUiContext = ({
editor.sideEffects.registerAfterCreateHandler("shape", (shape) => {
const util = editor.getShapeUtil(shape);
if (util instanceof BaseDiscourseNodeUtil) {
const autoCanvasRelations = getSetting<boolean>(
AUTO_CANVAS_RELATIONS_KEY,
false,
);
const autoCanvasRelations = getPersonalSetting<boolean>([
"Auto canvas relations",
]);
if (autoCanvasRelations) {
void util.createExistingRelations({
shape: shape as DiscourseNodeShape,
Expand Down
9 changes: 5 additions & 4 deletions apps/roam/src/components/canvas/uiOverrides.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ import calcCanvasNodeSizeAndImg from "~/utils/calcCanvasNodeSizeAndImg";
import { AddReferencedNodeType } from "./DiscourseRelationShape/DiscourseRelationTool";
import { getRelationColor } from "./DiscourseRelationShape/DiscourseRelationUtil";
import DiscourseGraphPanel from "./DiscourseToolPanel";
import { DISCOURSE_TOOL_SHORTCUT_KEY } from "~/data/userSettings";
import { getSetting } from "~/utils/extensionSettings";
import { CustomDefaultToolbar } from "./CustomDefaultToolbar";
import { renderModifyNodeDialog } from "~/components/ModifyNodeDialog";
import { getPersonalSetting } from "~/components/settings/utils/accessors";

export const getOnSelectForShape = ({
shape,
Expand Down Expand Up @@ -271,10 +270,12 @@ export const createUiOverrides = ({
}): TLUiOverrides => ({
tools: (editor, tools) => {
// Get the custom keyboard shortcut for the discourse tool
const discourseToolCombo = getSetting(DISCOURSE_TOOL_SHORTCUT_KEY, {
const discourseToolCombo = getPersonalSetting<IKeyCombo>([
"Discourse tool shortcut",
]) || {
key: "",
modifiers: 0,
}) as IKeyCombo;
};

// For discourse tool, just use the key directly since we don't allow modifiers
const discourseToolShortcut = discourseToolCombo?.key?.toUpperCase() || "";
Comment thread
sid597 marked this conversation as resolved.
Expand Down
2 changes: 0 additions & 2 deletions apps/roam/src/components/settings/AdminPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import { USE_REIFIED_RELATIONS } from "~/data/userSettings";
import posthog from "posthog-js";
import { setFeatureFlag } from "~/components/settings/utils/accessors";
import { FeatureFlagPanel } from "./components/BlockPropSettingPanels";
import { getFeatureFlag } from "./utils/accessors";

const NodeRow = ({ node }: { node: PConceptFull }) => {
return (
Expand Down Expand Up @@ -473,7 +472,6 @@ const FeatureFlagsTab = (): React.ReactElement => {
title="Use new settings store"
description="When enabled, accessor getters read from block props instead of the old system. Surfaces dual-write gaps during development."
featureKey="Use new settings store"
initialValue={getFeatureFlag("Use new settings store")}
/>

<Button
Expand Down
1 change: 0 additions & 1 deletion apps/roam/src/components/settings/GeneralSettings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ const DiscourseGraphHome = () => {
<FeatureFlagPanel
title="(BETA) Left Sidebar"
description="Whether or not to enable the left sidebar."
initialValue={settings.leftSidebarEnabled.value}
featureKey="Enable left sidebar"
order={2}
uid={settings.leftSidebarEnabled.uid}
Expand Down
15 changes: 1 addition & 14 deletions apps/roam/src/components/settings/HomePersonalSettings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
STREAMLINE_STYLING_KEY,
DISALLOW_DIAGNOSTICS,
} from "~/data/userSettings";
import { getSetting, setSetting } from "~/utils/extensionSettings";
import { setSetting } from "~/utils/extensionSettings";
import { enablePostHog, disablePostHog } from "~/utils/posthog";
import KeyboardShortcutInput from "./KeyboardShortcutInput";
import streamlineStyling from "~/styles/streamlineStyling";
Expand Down Expand Up @@ -67,7 +67,6 @@ const HomePersonalSettings = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
title="Overlay"
description="Whether or not to overlay discourse context information over discourse node references."
settingKeys={["Discourse context overlay"]}
initialValue={getSetting<boolean>("discourse-context-overlay", false)}
Copy link
Copy Markdown
Member

@mdroidian mdroidian Feb 28, 2026

Choose a reason for hiding this comment

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

What is initialValue being replaced with?

Copy link
Copy Markdown
Collaborator Author

@sid597 sid597 Mar 1, 2026

Choose a reason for hiding this comment

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

Before: Callers read the value manually and passed it as initialValue:

  <PersonalFlagPanel                                                 
    settingKeys={["Discourse context overlay"]}                      
    initialValue={getSetting<boolean>("discourse-context-overlay",   
  false)}             
  />

After: PersonalFlagPanel reads it internally — same lookup, just moved inside:

  <PersonalFlagPanel
    settingKeys={["Discourse context overlay"]}
  />

The wrapper does getPersonalSetting(settingKeys) which calls the
same getSetting(legacyKey, zodDefault) under the hood. The legacy
key mapping ("Discourse context overlay" → "discourse-context-overlay") and default (false) come from the
accessor layer now instead of the call site.

This was set up in ENG-1472 — PersonalFlagPanel (#813) already self-reads,
so the initialValue props were redundant.

onChange={(checked) => {
void setSetting("discourse-context-overlay", checked);
onPageRefObserverChange(overlayHandler)(checked);
Expand All @@ -81,7 +80,6 @@ const HomePersonalSettings = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
title="Suggestive mode overlay"
description="Whether or not to overlay suggestive mode button over discourse node references."
settingKeys={["Suggestive mode overlay"]}
initialValue={getSetting<boolean>("suggestive-mode-overlay", false)}
onChange={(checked) => {
void setSetting("suggestive-mode-overlay", checked);
onPageRefObserverChange(getSuggestiveOverlayHandler(onloadArgs))(
Expand All @@ -94,7 +92,6 @@ const HomePersonalSettings = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
title="Text selection popup"
description="Whether or not to show the discourse node menu when selecting text."
settingKeys={["Text selection popup"]}
initialValue={getSetting("text-selection-popup", true)}
onChange={(checked) => {
void setSetting("text-selection-popup", checked);
}}
Expand All @@ -103,7 +100,6 @@ const HomePersonalSettings = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
title="Disable sidebar open"
description="Disable opening new nodes in the sidebar when created"
settingKeys={["Disable sidebar open"]}
initialValue={getSetting<boolean>("disable-sidebar-open", false)}
onChange={(checked) => {
void setSetting("disable-sidebar-open", checked);
}}
Expand All @@ -112,7 +108,6 @@ const HomePersonalSettings = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
title="Page preview"
description="Whether or not to display page previews when hovering over page refs"
settingKeys={["Page preview"]}
initialValue={getSetting<boolean>("page-preview", false)}
onChange={(checked) => {
void setSetting("page-preview", checked);
onPageRefObserverChange(previewPageRefHandler)(checked);
Expand All @@ -122,7 +117,6 @@ const HomePersonalSettings = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
title="Hide feedback button"
description="Hide the 'Send feedback' button at the bottom right of the screen."
settingKeys={["Hide feedback button"]}
initialValue={getSetting<boolean>("hide-feedback-button", false)}
onChange={(checked) => {
void setSetting("hide-feedback-button", checked);
if (checked) {
Expand All @@ -136,7 +130,6 @@ const HomePersonalSettings = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
title="Auto canvas relations"
description="Automatically add discourse relations to canvas when a node is added"
settingKeys={["Auto canvas relations"]}
initialValue={getSetting<boolean>(AUTO_CANVAS_RELATIONS_KEY, false)}
onChange={(checked) => {
void setSetting(AUTO_CANVAS_RELATIONS_KEY, checked);
}}
Expand All @@ -146,10 +139,6 @@ const HomePersonalSettings = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
title="(BETA) Overlay in canvas"
description="Whether or not to overlay discourse context information over canvas nodes."
settingKeys={["Overlay in canvas"]}
initialValue={getSetting<boolean>(
DISCOURSE_CONTEXT_OVERLAY_IN_CANVAS_KEY,
false,
)}
onChange={(checked) => {
void setSetting(DISCOURSE_CONTEXT_OVERLAY_IN_CANVAS_KEY, checked);
}}
Expand All @@ -158,7 +147,6 @@ const HomePersonalSettings = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
title="Streamline styling"
description="Apply streamlined styling to your personal graph for a cleaner appearance."
settingKeys={["Streamline styling"]}
initialValue={getSetting<boolean>(STREAMLINE_STYLING_KEY, false)}
onChange={(checked) => {
void setSetting(STREAMLINE_STYLING_KEY, checked);
const existingStyleElement =
Expand All @@ -176,7 +164,6 @@ const HomePersonalSettings = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
title="Disable product diagnostics"
description="Disable sending usage signals and error reports that help us improve the product."
settingKeys={["Disable product diagnostics"]}
initialValue={getSetting<boolean>(DISALLOW_DIAGNOSTICS, false)}
onChange={(checked) => {
void setSetting(DISALLOW_DIAGNOSTICS, checked);
if (checked) {
Expand Down
3 changes: 0 additions & 3 deletions apps/roam/src/components/settings/QuerySettings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ const QuerySettings = ({
title="Hide query metadata"
description="Hide the Roam blocks that are used to power each query"
settingKeys={["Query", "Hide query metadata"]}
initialValue={
(extensionAPI.settings.get(HIDE_METADATA_KEY) as boolean) ?? true
}
onChange={(checked) => {
void extensionAPI.settings.set(HIDE_METADATA_KEY, checked);
posthog.capture("Query Settings: Hide Metadata Toggled", {
Expand Down
Loading