Skip to content

Commit 30dc59c

Browse files
trangdoan982trangdoan982
committed
address all PR comments
1 parent 86a94e0 commit 30dc59c

File tree

5 files changed

+99
-88
lines changed

5 files changed

+99
-88
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,9 @@ const FuzzySelectInput = <T extends Result = Result>({
7777
const handleClear = useCallback(() => {
7878
setIsLocked(false);
7979
setQuery("");
80-
setValue({ text: "", uid: "" } as T);
80+
setValue({ ...value, text: "", uid: "" } as T);
8181
onLockedChange?.(false);
82-
}, [setValue, onLockedChange]);
82+
}, [value, setValue, onLockedChange]);
8383

8484
// Handle keyboard navigation
8585
const handleKeyDown = useCallback(
@@ -161,7 +161,7 @@ const FuzzySelectInput = <T extends Result = Result>({
161161
}
162162

163163
// Create mode: locked value display
164-
if (isLocked || initialIsLocked) {
164+
if (isLocked) {
165165
return (
166166
<div className="flex w-full items-center gap-2">
167167
<div className="flex flex-1 items-center gap-2 rounded border border-gray-300 bg-gray-100 px-3 py-2 dark:border-gray-600 dark:bg-gray-800">

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

Lines changed: 85 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export type ModifyNodeDialogProps = {
3939
initialReferencedNode?: { text: string; uid: string };
4040
sourceBlockUid?: string; //the block that we started modifying from
4141
extensionAPI?: OnloadArgs["extensionAPI"];
42-
isFromCanvas?: boolean;
42+
includeDefaultNodes?: boolean; // Include default nodes (Page, Block) in node type selector
4343
imageUrl?: string; // For image conversion from canvas
4444
onSuccess: (result: {
4545
text: string;
@@ -58,38 +58,56 @@ const ModifyNodeDialog = ({
5858
initialReferencedNode,
5959
sourceBlockUid,
6060
extensionAPI,
61-
isFromCanvas = false,
61+
includeDefaultNodes = false,
6262
imageUrl,
6363
onSuccess,
6464
onClose,
6565
}: RoamOverlayProps<ModifyNodeDialogProps>) => {
66-
const [contentText, setContentText] = useState(initialValue.text);
67-
const [contentUid, setContentUid] = useState(initialValue.uid);
68-
const [referencedNodeText, setReferencedNodeText] = useState(
69-
initialReferencedNode?.text || "",
70-
);
71-
const [referencedNodeUid, setReferencedNodeUid] = useState(
72-
initialReferencedNode?.uid || "",
73-
);
74-
const [isContentLocked, setIsContentLocked] = useState(false);
75-
const [isReferencedNodeLocked, setIsReferencedNodeLocked] = useState(
76-
Boolean(initialReferencedNode?.uid),
66+
const [content, setContent] = useState<Result>({
67+
text: initialValue.text,
68+
uid: initialValue.uid,
69+
});
70+
const [referencedNodeValue, setReferencedNodeValue] = useState<Result>({
71+
text: initialReferencedNode?.text || "",
72+
uid: initialReferencedNode?.uid || "",
73+
});
74+
75+
const isContentLocked = useMemo(
76+
() =>
77+
Boolean(
78+
content.uid && content.uid !== initialValue.uid && mode === "create",
79+
),
80+
[content.uid, initialValue.uid, mode],
7781
);
78-
const [contentOptions, setContentOptions] = useState<Result[]>([]);
79-
const [referencedNodeOptions, setReferencedNodeOptions] = useState<Result[]>(
80-
[],
82+
const isReferencedNodeLocked = useMemo(
83+
() =>
84+
Boolean(
85+
referencedNodeValue.uid &&
86+
referencedNodeValue.uid !== initialReferencedNode?.uid,
87+
),
88+
[referencedNodeValue.uid, initialReferencedNode?.uid],
8189
);
82-
const [contentLoading, setContentLoading] = useState(false);
83-
const [referencedNodeLoading, setReferencedNodeLoading] = useState(false);
90+
91+
const [options, setOptions] = useState<{
92+
content: Result[];
93+
referencedNode: Result[];
94+
}>({ content: [], referencedNode: [] });
95+
96+
const [loading, setLoading] = useState<{
97+
content: boolean;
98+
referencedNode: boolean;
99+
}>({ content: false, referencedNode: false });
100+
84101
const contentRequestIdRef = useRef(0);
85102
const referencedNodeRequestIdRef = useRef(0);
86103
const [error, setError] = useState("");
87104

88105
const discourseNodes = useMemo(() => {
89106
const allNodes = getDiscourseNodes();
90-
// Allow default nodes when opened from canvas, exclude them otherwise
91-
return isFromCanvas ? allNodes : allNodes.filter(excludeDefaultNodes);
92-
}, [isFromCanvas]);
107+
return includeDefaultNodes
108+
? allNodes
109+
: allNodes.filter(excludeDefaultNodes);
110+
}, [includeDefaultNodes]);
93111

94112
const [selectedNodeType, setSelectedNodeType] = useState(() => {
95113
const node = discourseNodes.find((n) => n.type === nodeType);
@@ -109,7 +127,7 @@ const ModifyNodeDialog = ({
109127
if (val.toLowerCase() === "content") continue;
110128
if (val.toLowerCase() === "context") continue;
111129

112-
const allNodes = isFromCanvas
130+
const allNodes = includeDefaultNodes
113131
? getDiscourseNodes()
114132
: getDiscourseNodes().filter(excludeDefaultNodes);
115133

@@ -126,10 +144,10 @@ const ModifyNodeDialog = ({
126144
}
127145

128146
return null;
129-
}, [nodeFormat, isFromCanvas]);
147+
}, [nodeFormat, includeDefaultNodes]);
130148

131149
useEffect(() => {
132-
setContentLoading(true);
150+
setLoading({ content: true, referencedNode: Boolean(referencedNode) });
133151

134152
let alive = true;
135153
const req = ++contentRequestIdRef.current;
@@ -154,16 +172,18 @@ const ModifyNodeDialog = ({
154172
},
155173
],
156174
});
157-
if (contentRequestIdRef.current === req && alive)
158-
setContentOptions(results);
175+
if (contentRequestIdRef.current === req && alive) {
176+
setOptions((prev) => ({ ...prev, content: results }));
177+
}
159178
}
160179
} catch (error) {
161180
if (contentRequestIdRef.current === req && alive) {
162181
console.error("Error fetching content options:", error);
163182
}
164183
} finally {
165-
if (contentRequestIdRef.current === req && alive)
166-
setContentLoading(false);
184+
if (contentRequestIdRef.current === req && alive) {
185+
setLoading((prev) => ({ ...prev, content: false }));
186+
}
167187
}
168188
};
169189

@@ -185,15 +205,15 @@ const ModifyNodeDialog = ({
185205
],
186206
});
187207
if (referencedNodeRequestIdRef.current === refReq && refAlive) {
188-
setReferencedNodeOptions(results);
208+
setOptions((prev) => ({ ...prev, referencedNode: results }));
189209
}
190210
} catch (error) {
191211
if (referencedNodeRequestIdRef.current === refReq && refAlive) {
192212
console.error("Error fetching referenced node options:", error);
193213
}
194214
} finally {
195215
if (referencedNodeRequestIdRef.current === refReq && refAlive) {
196-
setReferencedNodeLoading(false);
216+
setLoading((prev) => ({ ...prev, referencedNode: false }));
197217
}
198218
}
199219
};
@@ -207,13 +227,11 @@ const ModifyNodeDialog = ({
207227
}, [selectedNodeType, referencedNode]);
208228

209229
const setValue = useCallback((r: Result) => {
210-
setContentText(r.text);
211-
setContentUid(r.uid);
230+
setContent(r);
212231
}, []);
213232

214-
const setReferencedNodeValue = useCallback((r: Result) => {
215-
setReferencedNodeText(r.text);
216-
setReferencedNodeUid(r.uid);
233+
const setReferencedNodeValueCallback = useCallback((r: Result) => {
234+
setReferencedNodeValue(r);
217235
}, []);
218236

219237
const onCancelClick = useCallback(() => {
@@ -233,29 +251,29 @@ const ModifyNodeDialog = ({
233251
);
234252

235253
const onSubmit = async () => {
236-
if (!contentText.trim()) return;
254+
if (!content.text.trim()) return;
237255
try {
238256
if (mode === "create") {
239257
// If content is locked (user selected existing node), just insert it
240-
if (isContentLocked && contentUid) {
258+
if (isContentLocked && content.uid) {
241259
if (sourceBlockUid) {
242-
const pageRef = `[[${contentText}]]`;
260+
const pageRef = `[[${content.text}]]`;
243261
await updateBlock({
244262
uid: sourceBlockUid,
245263
text: pageRef,
246264
});
247265
}
248266

249267
if (imageUrl) {
250-
const pageUid = contentUid || getPageUidByPageTitle(contentText);
268+
const pageUid = content.uid || getPageUidByPageTitle(content.text);
251269
if (pageUid) {
252270
await addImageToPage(pageUid, imageUrl);
253271
}
254272
}
255273

256274
await onSuccess({
257-
text: contentText,
258-
uid: contentUid,
275+
text: content.text,
276+
uid: content.uid,
259277
action: "create",
260278
});
261279

@@ -265,12 +283,12 @@ const ModifyNodeDialog = ({
265283

266284
// Format content with referenced node if present
267285
let formattedTitle = "";
268-
if (referencedNode && referencedNodeText) {
286+
if (referencedNode && referencedNodeValue.text) {
269287
// Format the referenced node if it's new
270-
let formattedReferencedNodeText = referencedNodeText;
288+
let formattedReferencedNodeText = referencedNodeValue.text;
271289
if (!isReferencedNodeLocked) {
272290
const formattedRefNode = await getNewDiscourseNodeText({
273-
text: referencedNodeText.trim(),
291+
text: referencedNodeValue.text.trim(),
274292
nodeType: referencedNode.nodeType,
275293
blockUid: sourceBlockUid,
276294
});
@@ -283,18 +301,19 @@ const ModifyNodeDialog = ({
283301
formattedTitle = nodeFormat.replace(
284302
/{([\w\d-]*)}/g,
285303
(_, val: string) => {
286-
if (/content/i.test(val)) return contentText.trim();
304+
if (/content/i.test(val)) return content.text.trim();
287305
if (new RegExp(referencedNode.name, "i").test(val))
288306
return `[[${formattedReferencedNodeText}]]`;
289307
return "";
290308
},
291309
);
292310
} else {
293311
formattedTitle = await getNewDiscourseNodeText({
294-
text: contentText.trim(),
312+
text: content.text.trim(),
295313
nodeType: selectedNodeType.type,
296314
blockUid: sourceBlockUid,
297315
});
316+
console.log("formattedTitle", formattedTitle);
298317
}
299318
if (!formattedTitle) {
300319
return;
@@ -360,21 +379,21 @@ const ModifyNodeDialog = ({
360379

361380
await onSuccess({
362381
text: formattedTitle,
363-
uid: contentUid,
382+
uid: newPageUid,
364383
action: "create",
365384
newPageUid,
366385
});
367386
} else {
368387
// Edit mode: update the existing block
369-
let updatedContent = contentText;
388+
let updatedContent = content.text;
370389

371390
// Format with referenced node if present
372-
if (referencedNode && referencedNodeText) {
391+
if (referencedNode && referencedNodeValue.text) {
373392
updatedContent = nodeFormat
374393
.replace(/{([\w\d-]*)}/g, (_, val: string) => {
375-
if (/content/i.test(val)) return contentText.trim();
394+
if (/content/i.test(val)) return content.text.trim();
376395
if (new RegExp(referencedNode.name, "i").test(val))
377-
return `[[${referencedNodeText}]]`;
396+
return `[[${referencedNodeValue.text}]]`;
378397
return "";
379398
})
380399
.trim();
@@ -395,16 +414,13 @@ const ModifyNodeDialog = ({
395414

396415
await onSuccess({
397416
text: updatedContent,
398-
uid: sourceBlockUid || contentUid,
417+
uid: sourceBlockUid || content.uid,
399418
action: "edit",
400419
});
401420
}
402421
onClose();
403422
} catch (error) {
404423
setError((error as Error).message);
405-
} finally {
406-
setContentLoading(false);
407-
setReferencedNodeLoading(false);
408424
}
409425
};
410426

@@ -451,18 +467,17 @@ const ModifyNodeDialog = ({
451467
<div className="w-full">
452468
<Label>Content</Label>
453469
<FuzzySelectInput
454-
value={{ text: contentText, uid: contentUid }}
470+
value={content}
455471
setValue={setValue}
456-
options={contentOptions}
472+
options={options.content}
457473
placeholder={
458-
contentLoading
474+
loading.content
459475
? "..."
460476
: `Enter a ${selectedNodeType.text.toLowerCase()} ...`
461477
}
462-
disabled={contentLoading}
463-
onLockedChange={setIsContentLocked}
478+
disabled={loading.content}
464479
mode={mode}
465-
initialUid={contentUid}
480+
initialUid={content.uid}
466481
/>
467482
</div>
468483

@@ -471,19 +486,15 @@ const ModifyNodeDialog = ({
471486
<div className="w-full">
472487
<Label>{referencedNode.name}</Label>
473488
<FuzzySelectInput
474-
value={{
475-
text: referencedNodeText || "",
476-
uid: referencedNodeUid || "",
477-
}}
478-
setValue={setReferencedNodeValue}
479-
options={referencedNodeOptions}
489+
value={referencedNodeValue}
490+
setValue={setReferencedNodeValueCallback}
491+
options={options.referencedNode}
480492
placeholder={
481-
referencedNodeLoading ? "..." : "Select a referenced node"
493+
loading.referencedNode ? "..." : "Select a referenced node"
482494
}
483-
disabled={referencedNodeLoading}
484-
onLockedChange={setIsReferencedNodeLocked}
495+
disabled={loading.referencedNode}
485496
mode={"create"}
486-
initialUid={referencedNodeUid}
497+
initialUid={referencedNodeValue.uid}
487498
initialIsLocked={isReferencedNodeLocked}
488499
/>
489500
</div>
@@ -498,17 +509,17 @@ const ModifyNodeDialog = ({
498509
text="Confirm"
499510
intent={Intent.PRIMARY}
500511
onClick={() => void onSubmit()}
501-
disabled={contentLoading || !contentText.trim()}
512+
disabled={loading.content || !content.text.trim()}
502513
className="flex-shrink-0"
503514
/>
504515
<Button
505516
text="Cancel"
506517
onClick={onCancelClick}
507-
disabled={contentLoading}
518+
disabled={loading.content}
508519
className="flex-shrink-0"
509520
/>
510521
<span className="flex-grow text-red-800">{error}</span>
511-
{contentLoading && <Spinner size={SpinnerSize.SMALL} />}
522+
{loading.content && <Spinner size={SpinnerSize.SMALL} />}
512523
</div>
513524
</div>
514525
</div>

0 commit comments

Comments
 (0)