Skip to content

Commit f4ae3f0

Browse files
trangdoan982trangdoan982
committed
address all PR comments
1 parent 1a07243 commit f4ae3f0

File tree

2 files changed

+144
-85
lines changed

2 files changed

+144
-85
lines changed

‎apps/roam/src/components/ModifyNodeDialog.tsx‎

Lines changed: 116 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,16 @@ import getDiscourseNodes, {
2525
} from "~/utils/getDiscourseNodes";
2626
import FuzzySelectInput from "./FuzzySelectInput";
2727
import { createBlock, updateBlock } from "roamjs-components/writes";
28-
import { getNewDiscourseNodeText } from "~/utils/formatUtils";
28+
import {
29+
getNewDiscourseNodeText,
30+
getReferencedNodeInFormat,
31+
} from "~/utils/formatUtils";
2932
import createDiscourseNode from "~/utils/createDiscourseNode";
3033
import { OnloadArgs } from "roamjs-components/types";
3134
import { render as renderToast } from "roamjs-components/components/Toast";
3235
import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle";
36+
import resolveQueryBuilderRef from "~/utils/resolveQueryBuilderRef";
37+
import runQuery from "~/utils/runQuery";
3338

3439
export type ModifyNodeDialogMode = "create" | "edit";
3540
export type ModifyNodeDialogProps = {
@@ -45,7 +50,6 @@ export type ModifyNodeDialogProps = {
4550
text: string;
4651
uid: string;
4752
action: string;
48-
newPageUid?: string;
4953
}) => Promise<void>;
5054
onClose: () => void;
5155
};
@@ -93,10 +97,7 @@ const ModifyNodeDialog = ({
9397
referencedNode: Result[];
9498
}>({ content: [], referencedNode: [] });
9599

96-
const [loading, setLoading] = useState<{
97-
content: boolean;
98-
referencedNode: boolean;
99-
}>({ content: false, referencedNode: false });
100+
const [loading, setLoading] = useState(false);
100101

101102
const contentRequestIdRef = useRef(0);
102103
const referencedNodeRequestIdRef = useRef(0);
@@ -119,35 +120,20 @@ const ModifyNodeDialog = ({
119120
}, [selectedNodeType]);
120121

121122
const referencedNode = useMemo(() => {
122-
const regex = /{([\w\d-]*)}/g;
123-
const matches = [...nodeFormat.matchAll(regex)];
124-
125-
for (const match of matches) {
126-
const val = match[1];
127-
if (val.toLowerCase() === "content") continue;
128-
if (val.toLowerCase() === "context") continue;
129-
130-
const allNodes = includeDefaultNodes
131-
? getDiscourseNodes()
132-
: getDiscourseNodes().filter(excludeDefaultNodes);
133-
134-
const refNode = allNodes.find(({ text }) =>
135-
new RegExp(text, "i").test(val),
136-
);
123+
const refNode = getReferencedNodeInFormat({
124+
format: nodeFormat,
125+
});
137126

138-
if (refNode) {
139-
return {
140-
name: refNode.text,
141-
nodeType: refNode.type,
142-
};
143-
}
144-
}
127+
if (!refNode) return null;
145128

146-
return null;
147-
}, [nodeFormat, includeDefaultNodes]);
129+
return {
130+
name: refNode.text,
131+
nodeType: refNode.type,
132+
};
133+
}, [nodeFormat]);
148134

149135
useEffect(() => {
150-
setLoading({ content: true, referencedNode: Boolean(referencedNode) });
136+
setLoading(true);
151137

152138
let alive = true;
153139
const req = ++contentRequestIdRef.current;
@@ -186,10 +172,6 @@ const ModifyNodeDialog = ({
186172
),
187173
});
188174
}
189-
} finally {
190-
if (contentRequestIdRef.current === req && alive) {
191-
setLoading((prev) => ({ ...prev, content: false }));
192-
}
193175
}
194176
};
195177

@@ -215,17 +197,30 @@ const ModifyNodeDialog = ({
215197
}
216198
} catch (error) {
217199
if (referencedNodeRequestIdRef.current === refReq && refAlive) {
218-
console.error("Error fetching referenced node options:", error);
219-
}
220-
} finally {
221-
if (referencedNodeRequestIdRef.current === refReq && refAlive) {
222-
setLoading((prev) => ({ ...prev, referencedNode: false }));
200+
renderToast({
201+
id: `discourse-node-error-${Date.now()}`,
202+
intent: "danger",
203+
content: (
204+
<span>
205+
Error fetching referenced node options: {String(error)}
206+
</span>
207+
),
208+
});
223209
}
224210
}
225211
};
226212

227-
void fetchOptions();
228-
void fetchReferencedOptions();
213+
void (async () => {
214+
const promises = [fetchOptions()];
215+
if (referencedNode) {
216+
promises.push(fetchReferencedOptions());
217+
}
218+
await Promise.all(promises);
219+
if (contentRequestIdRef.current === req && alive) {
220+
setLoading(false);
221+
}
222+
})();
223+
229224
return () => {
230225
alive = false;
231226
refAlive = false;
@@ -245,15 +240,67 @@ const ModifyNodeDialog = ({
245240
}, [onClose]);
246241

247242
const addImageToPage = useCallback(
248-
async (pageUid: string, imageUrl: string) => {
249-
const imageMarkdown = `![](${imageUrl})`;
250-
await createBlock({
251-
node: { text: imageMarkdown },
252-
order: 0,
253-
parentUid: pageUid,
254-
});
243+
async ({
244+
pageUid,
245+
imageUrl,
246+
configPageUid,
247+
extensionAPI,
248+
}: {
249+
pageUid: string;
250+
imageUrl: string;
251+
configPageUid: string;
252+
extensionAPI?: OnloadArgs["extensionAPI"];
253+
}) => {
254+
const discourseNodes = getDiscourseNodes();
255+
const canvasSettings = Object.fromEntries(
256+
discourseNodes.map((n) => [n.type, { ...n.canvasSettings }]),
257+
);
258+
const {
259+
"query-builder-alias": qbAlias = "",
260+
"key-image": isKeyImage = "",
261+
"key-image-option": keyImageOption = "",
262+
} = canvasSettings[configPageUid] || {};
263+
264+
const createOrUpdateImageBlock = async (imagePlaceholderUid?: string) => {
265+
const imageMarkdown = `![](${imageUrl})`;
266+
if (imagePlaceholderUid) {
267+
await updateBlock({
268+
uid: imagePlaceholderUid,
269+
text: imageMarkdown,
270+
});
271+
} else {
272+
await createBlock({
273+
node: { text: imageMarkdown },
274+
order: 0,
275+
parentUid: pageUid,
276+
});
277+
}
278+
};
279+
280+
if (!isKeyImage || !extensionAPI) {
281+
await createOrUpdateImageBlock();
282+
return;
283+
}
284+
285+
if (keyImageOption === "query-builder") {
286+
const parentUid = resolveQueryBuilderRef({
287+
queryRef: qbAlias,
288+
extensionAPI,
289+
});
290+
const results = await runQuery({
291+
extensionAPI,
292+
parentUid,
293+
// due to query format
294+
// eslint-disable-next-line @typescript-eslint/naming-convention
295+
inputs: { NODETEXT: content.text, NODEUID: pageUid },
296+
});
297+
const imagePlaceholderUid = results.allProcessedResults[0]?.uid;
298+
await createOrUpdateImageBlock(imagePlaceholderUid);
299+
} else {
300+
await createOrUpdateImageBlock();
301+
}
255302
},
256-
[],
303+
[content.text],
257304
);
258305

259306
const onSubmit = async () => {
@@ -273,7 +320,12 @@ const ModifyNodeDialog = ({
273320
if (imageUrl) {
274321
const pageUid = content.uid || getPageUidByPageTitle(content.text);
275322
if (pageUid) {
276-
await addImageToPage(pageUid, imageUrl);
323+
await addImageToPage({
324+
pageUid,
325+
imageUrl,
326+
configPageUid: selectedNodeType.type,
327+
extensionAPI,
328+
});
277329
}
278330
}
279331

@@ -306,6 +358,8 @@ const ModifyNodeDialog = ({
306358

307359
formattedTitle = nodeFormat.replace(
308360
/{([\w\d-]*)}/g,
361+
// unused variable will take _ as name
362+
// eslint-disable-next-line @typescript-eslint/naming-convention
309363
(_, val: string) => {
310364
if (/content/i.test(val)) return content.text.trim();
311365
if (new RegExp(referencedNode.name, "i").test(val))
@@ -364,6 +418,7 @@ const ModifyNodeDialog = ({
364418
await window.roamAlphaAPI.ui.rightSidebar.addWindow({
365419
window: {
366420
// @ts-expect-error TODO: fix this
421+
// eslint-disable-next-line @typescript-eslint/naming-convention
367422
"block-uid": newPageUid,
368423
type: "outline",
369424
},
@@ -386,7 +441,6 @@ const ModifyNodeDialog = ({
386441
text: formattedTitle,
387442
uid: newPageUid,
388443
action: "create",
389-
newPageUid,
390444
});
391445
} else {
392446
// Edit mode: update the existing block
@@ -395,6 +449,8 @@ const ModifyNodeDialog = ({
395449
// Format with referenced node if present
396450
if (referencedNode && referencedNodeValue.text) {
397451
updatedContent = nodeFormat
452+
// unused variable will take _ as name
453+
// eslint-disable-next-line @typescript-eslint/naming-convention
398454
.replace(/{([\w\d-]*)}/g, (_, val: string) => {
399455
if (/content/i.test(val)) return content.text.trim();
400456
if (new RegExp(referencedNode.name, "i").test(val))
@@ -476,11 +532,11 @@ const ModifyNodeDialog = ({
476532
setValue={setValue}
477533
options={options.content}
478534
placeholder={
479-
loading.content
535+
loading
480536
? "..."
481537
: `Enter a ${selectedNodeType.text.toLowerCase()} ...`
482538
}
483-
disabled={loading.content}
539+
disabled={loading}
484540
mode={mode}
485541
initialUid={content.uid}
486542
/>
@@ -494,10 +550,8 @@ const ModifyNodeDialog = ({
494550
value={referencedNodeValue}
495551
setValue={setReferencedNodeValueCallback}
496552
options={options.referencedNode}
497-
placeholder={
498-
loading.referencedNode ? "..." : "Select a referenced node"
499-
}
500-
disabled={loading.referencedNode}
553+
placeholder={loading ? "..." : "Select a referenced node"}
554+
disabled={loading}
501555
mode={"create"}
502556
initialUid={referencedNodeValue.uid}
503557
initialIsLocked={isReferencedNodeLocked}
@@ -514,17 +568,17 @@ const ModifyNodeDialog = ({
514568
text="Confirm"
515569
intent={Intent.PRIMARY}
516570
onClick={() => void onSubmit()}
517-
disabled={loading.content || !content.text.trim()}
571+
disabled={loading || !content.text.trim()}
518572
className="flex-shrink-0"
519573
/>
520574
<Button
521575
text="Cancel"
522576
onClick={onCancelClick}
523-
disabled={loading.content}
577+
disabled={loading}
524578
className="flex-shrink-0"
525579
/>
526580
<span className="flex-grow text-red-800">{error}</span>
527-
{loading.content && <Spinner size={SpinnerSize.SMALL} />}
581+
{loading && <Spinner size={SpinnerSize.SMALL} />}
528582
</div>
529583
</div>
530584
</div>
@@ -534,6 +588,7 @@ const ModifyNodeDialog = ({
534588

535589
export const renderModifyNodeDialog = (props: ModifyNodeDialogProps) =>
536590
renderOverlay({
591+
// eslint-disable-next-line @typescript-eslint/naming-convention
537592
Overlay: ModifyNodeDialog,
538593
props,
539594
});

‎apps/roam/src/utils/renderNodeTagPopup.tsx‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export const renderNodeTagPopupButton = (
4646
const rawBlockText = blockUid ? getTextByBlockUid(blockUid) : "";
4747
const cleanedBlockText = rawBlockText.replace(textContent, "").trim();
4848

49-
const getInitialReferencedNode = () => {
49+
const getInitialReferencedNode = async () => {
5050
if (!blockUid) return { text: "", uid: "" };
5151

5252
const referencedNodeType = getReferencedNodeInFormat({
@@ -56,15 +56,17 @@ export const renderNodeTagPopupButton = (
5656
if (!referencedNodeType) return { text: "", uid: "" };
5757

5858
try {
59-
const referenced = window.roamAlphaAPI.data.fast.q(
60-
`[:find (pull ?r [:node/title :block/string]) :where [?b :block/uid "${blockUid}"] (or-join [?b ?r] (and [?b :block/parents ?p] [?p :block/refs ?r]) (and [?b :block/page ?r])) ${discourseNodeFormatToDatalog(
61-
{
62-
freeVar: "r",
63-
...referencedNodeType,
64-
},
59+
const referenced = (
60+
await window.roamAlphaAPI.data.async.fast.q(
61+
`[:find (pull ?r [:node/title :block/string]) :where [?b :block/uid "${blockUid}"] (or-join [?b ?r] (and [?b :block/parents ?p] [?p :block/refs ?r]) (and [?b :block/page ?r])) ${discourseNodeFormatToDatalog(
62+
{
63+
freeVar: "r",
64+
...referencedNodeType,
65+
},
66+
)
67+
.map((c) => compileDatalog(c, 0))
68+
.join(" ")}]`,
6569
)
66-
.map((c) => compileDatalog(c, 0))
67-
.join(" ")}]`,
6870
)?.[0]?.[0] as PullBlock;
6971

7072
if (referenced) {
@@ -82,27 +84,29 @@ export const renderNodeTagPopupButton = (
8284
return { text: "", uid: "" };
8385
};
8486

87+
const handleClick = async () => {
88+
const initialReferencedNode = await getInitialReferencedNode();
89+
renderModifyNodeDialog({
90+
mode: "create",
91+
nodeType: matchedNode.type,
92+
initialValue: { text: cleanedBlockText, uid: "" },
93+
initialReferencedNode,
94+
onSuccess: async () => {
95+
// Success is handled by the dialog itself
96+
},
97+
onClose: () => {},
98+
sourceBlockUid: blockUid,
99+
extensionAPI,
100+
});
101+
};
102+
85103
ReactDOM.render(
86104
<Popover
87105
content={
88106
<Button
89107
minimal
90108
outlined
91-
onClick={() => {
92-
const initialReferencedNode = getInitialReferencedNode();
93-
renderModifyNodeDialog({
94-
mode: "create",
95-
nodeType: matchedNode.type,
96-
initialValue: { text: cleanedBlockText, uid: "" },
97-
initialReferencedNode,
98-
onSuccess: async () => {
99-
// Success is handled by the dialog itself
100-
},
101-
onClose: () => {},
102-
sourceBlockUid: blockUid,
103-
extensionAPI,
104-
});
105-
}}
109+
onClick={() => void handleClick()}
106110
text={`Create ${matchedNode.text}`}
107111
/>
108112
}

0 commit comments

Comments
 (0)