Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { useState, useMemo, useEffect, useRef } from "react";
import { Button, Intent } from "@blueprintjs/core";
import BlocksPanel from "roamjs-components/components/ConfigPanels/BlocksPanel";
import DualWriteBlocksPanel from "./components/EphemeralBlocksPanel";
import getSubTree from "roamjs-components/util/getSubTree";
import { DiscourseNode } from "~/utils/getDiscourseNodes";
import extractRef from "roamjs-components/util/extractRef";
Expand All @@ -12,6 +12,8 @@ import {
DiscourseNodeTextPanel,
} from "./components/BlockPropSettingPanels";

const TEMPLATE_SETTING_KEYS = ["template"];

const BlockRenderer = ({ uid }: { uid: string }) => {
const containerRef = useRef<HTMLDivElement>(null);

Expand Down Expand Up @@ -81,13 +83,12 @@ const DiscourseNodeSuggestiveRules = ({

return (
<div className="flex flex-col gap-4 p-4">
<BlocksPanel
<DualWriteBlocksPanel
nodeType={node.type}
title="Template"
description={`The template that auto fills ${node.text} page when generated.`}
order={0}
parentUid={nodeUid}
settingKeys={TEMPLATE_SETTING_KEYS}
uid={templateUid}
defaultValue={node.template}
/>

<DiscourseNodeTextPanel
Expand Down
11 changes: 6 additions & 5 deletions apps/roam/src/components/settings/NodeConfig.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import React, {
} from "react";
import { DiscourseNode } from "~/utils/getDiscourseNodes";
import SelectPanel from "roamjs-components/components/ConfigPanels/SelectPanel";
import BlocksPanel from "roamjs-components/components/ConfigPanels/BlocksPanel";
import DualWriteBlocksPanel from "./components/EphemeralBlocksPanel";
import { getSubTree } from "roamjs-components/util";
import Description from "roamjs-components/components/Description";
import { Label, Tabs, Tab, TabId, InputGroup } from "@blueprintjs/core";
Expand All @@ -27,6 +27,8 @@ import {
DiscourseNodeFlagPanel,
} from "./components/BlockPropSettingPanels";

const TEMPLATE_SETTING_KEYS = ["template"];

export const getCleanTagText = (tag: string): string => {
return tag.replace(/^#+/, "").trim().toUpperCase();
};
Expand Down Expand Up @@ -342,13 +344,12 @@ const NodeConfig = ({
title="Template"
panel={
<div className="flex flex-col gap-4 p-1">
<BlocksPanel
<DualWriteBlocksPanel
nodeType={node.type}
title="Template"
description={`The template that auto fills ${node.text} page when generated.`}
order={0}
parentUid={node.type}
settingKeys={TEMPLATE_SETTING_KEYS}
uid={templateUid}
defaultValue={node.template}
/>
</div>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ const createDiscourseNodeSetter =
(keys: string[], value: json): void =>
setDiscourseNodeSetting(nodeType, keys, value);

type DiscourseNodeBaseProps = {
export type DiscourseNodeBaseProps = {
nodeType: string;
title: string;
description: string;
Expand Down
109 changes: 109 additions & 0 deletions apps/roam/src/components/settings/components/EphemeralBlocksPanel.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import React, { useRef, useEffect, useCallback } from "react";
import { Label } from "@blueprintjs/core";
import Description from "roamjs-components/components/Description";
import createBlock from "roamjs-components/writes/createBlock";
import getFullTreeByParentUid from "roamjs-components/queries/getFullTreeByParentUid";
import getFirstChildUidByBlockUid from "roamjs-components/queries/getFirstChildUidByBlockUid";
import type { TreeNode } from "roamjs-components/types";
import type { RoamNodeType } from "~/components/settings/utils/zodSchema";
import { setDiscourseNodeSetting } from "~/components/settings/utils/accessors";
import type { DiscourseNodeBaseProps } from "./BlockPropSettingPanels";

const DEBOUNCE_MS = 250;

type DualWriteBlocksPanelProps = DiscourseNodeBaseProps & {
uid: string;
};

const serializeBlockTree = (children: TreeNode[]): RoamNodeType[] =>
children
.sort((a, b) => a.order - b.order)
.map((child) => ({
text: child.text,
...(child.heading && { heading: child.heading as 0 | 1 | 2 | 3 }),
...(child.open === false && { open: false }),
...(child.children.length > 0 && {
children: serializeBlockTree(child.children),
}),
}));

const DualWriteBlocksPanel = ({
nodeType,
settingKeys,
title,
description,
uid,
}: DualWriteBlocksPanelProps) => {
const containerRef = useRef<HTMLDivElement>(null);
const debounceRef = useRef(0);
const pullWatchArgsRef = useRef<
[string, string, (before: unknown, after: unknown) => void] | null
>(null);

const handleChange = useCallback(() => {
window.clearTimeout(debounceRef.current);
debounceRef.current = window.setTimeout(() => {
const tree = getFullTreeByParentUid(uid);
const serialized = serializeBlockTree(tree.children);
setDiscourseNodeSetting(nodeType, settingKeys, serialized);
}, DEBOUNCE_MS);
}, [uid, nodeType, settingKeys]);

useEffect(() => {
const el = containerRef.current;
if (!el || !uid) return;

const pattern = "[:block/string :block/order {:block/children ...}]";
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 PullWatch pattern omits :block/heading and :block/open, so changes to those attributes never trigger a sync

The DualWriteBlocksPanel registers a Roam pull watch with pattern [:block/string :block/order {:block/children ...}] (EphemeralBlocksPanel.tsx:56), but the serializeBlockTree function at EphemeralBlocksPanel.tsx:18-28 reads and serializes child.heading and child.open into the output.

Root cause and impact

The pull watch pattern determines which Datomic attribute changes trigger the callback. Since :block/heading and :block/open are absent from the pattern, changing a block's heading level (e.g. normal → H1) or toggling its open/collapsed state will not fire the callback, so setDiscourseNodeSetting is never called for those edits.

However, serializeBlockTree does include these fields:

...(child.heading && { heading: child.heading as 0 | 1 | 2 | 3 }),
...(child.open === false && { open: false }),

This means heading/open changes made in isolation are silently dropped from the block-props settings store. They would only be picked up incidentally if the user also edits text or reorders blocks in the same session. The fix is to add :block/heading and :block/open to the pull pattern:

[:block/string :block/order :block/heading :block/open {:block/children ...}]
Suggested change
const pattern = "[:block/string :block/order {:block/children ...}]";
const pattern = "[:block/string :block/order :block/heading :block/open {:block/children ...}]";
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

const entityId = `[:block/uid "${uid}"]`;
const callback = () => handleChange();

const registerPullWatch = () => {
pullWatchArgsRef.current = [pattern, entityId, callback];
window.roamAlphaAPI.data.addPullWatch(pattern, entityId, callback);
};

if (!getFirstChildUidByBlockUid(uid)) {
void createBlock({ node: { text: " " }, parentUid: uid }).then(() => {
el.innerHTML = "";
void window.roamAlphaAPI.ui.components.renderBlock({ uid, el });
registerPullWatch();
});
} else {
el.innerHTML = "";
void window.roamAlphaAPI.ui.components.renderBlock({ uid, el });
registerPullWatch();
}

return () => {
window.clearTimeout(debounceRef.current);
if (pullWatchArgsRef.current) {
window.roamAlphaAPI.data.removePullWatch(...pullWatchArgsRef.current);
pullWatchArgsRef.current = null;
}
};
}, [uid, handleChange]);

return (
<>
<Label>
{title}
<Description description={description} />
</Label>
<style>{`.dg-dualwrite-blocks > div > .rm-block-main {
display: none;
}
.dg-dualwrite-blocks > div > .rm-block-children > .rm-multibar {
display: none;
}
.dg-dualwrite-blocks > div > .rm-block-children {
margin-left: -4px;
}`}</style>
<div
ref={containerRef}
className="dg-dualwrite-blocks rounded border border-gray-200 py-2"
/>
</>
);
};

export default DualWriteBlocksPanel;
4 changes: 0 additions & 4 deletions apps/roam/src/components/settings/utils/zodSchema.example.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ const canvasSettings: CanvasSettings = {
};

const suggestiveRules: SuggestiveRules = {
template: [
{ text: "Summary::", heading: 2 },
{ text: "Key Points::", heading: 2, children: [{ text: "" }] },
],
embeddingRef: "((block-uid-123))",
isFirstChild: true,
};
Expand Down
1 change: 0 additions & 1 deletion apps/roam/src/components/settings/utils/zodSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ export const RoamNodeSchema: z.ZodType<RoamNode> = z.lazy(() =>
);

export const SuggestiveRulesSchema = z.object({
template: z.array(RoamNodeSchema).default([]),
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this being removed? is it being replaced with something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't have to store the template structure explicitly for suggestiveRulesSchema because

  • firstly suggestive mode shows the template not to edit it but to let the user copy a block uid from the template
  • and even if the user edits it we should be doing it in one place because there is no use for a different version of template, we use the canonical template from the nodes

embeddingRef: z.string().default(""),
isFirstChild: z.boolean().default(false),
});
Expand Down
Loading