Skip to content

Commit 262cdd8

Browse files
trangdoan982claude
andauthored
[ENG-1545] Switch positions of node type and node title fields (#907)
* [ENG-1545] Switch positions of node type and node title fields Move Content input above Node Type selector so users can immediately start typing. Support empty Node Type (queries all node types), auto-detect type on selection, and lock type when a node is selected. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * lint and remove unnecessary change * small fixes * address pr comment * address PR comments * address PR comments * address PR comments about non-null assertions * override global style to restore focus-ring --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 73f4dd7 commit 262cdd8

4 files changed

Lines changed: 101 additions & 54 deletions

File tree

apps/roam/src/components/ModifyNodeDialog.tsx

Lines changed: 82 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ import renderOverlay, {
2222
import fireQuery from "~/utils/fireQuery";
2323
import getDiscourseNodes, {
2424
excludeDefaultNodes,
25+
type DiscourseNode,
2526
} from "~/utils/getDiscourseNodes";
27+
import { getAllDiscourseNodesSince } from "~/utils/getAllDiscourseNodesSince";
2628
import FuzzySelectInput from "./FuzzySelectInput";
2729
import { createBlock, updateBlock } from "roamjs-components/writes";
2830
import {
@@ -40,7 +42,7 @@ import posthog from "posthog-js";
4042
export type ModifyNodeDialogMode = "create" | "edit";
4143
export type ModifyNodeDialogProps = {
4244
mode: ModifyNodeDialogMode;
43-
nodeType: string;
45+
nodeType?: string;
4446
initialValue: { text: string; uid: string };
4547
initialReferencedNode?: { text: string; uid: string };
4648
sourceBlockUid?: string; //the block that we started modifying from
@@ -109,13 +111,15 @@ const ModifyNodeDialog = ({
109111
: allNodes.filter(excludeDefaultNodes);
110112
}, [includeDefaultNodes]);
111113

112-
const [selectedNodeType, setSelectedNodeType] = useState(() => {
114+
const [selectedNodeType, setSelectedNodeType] = useState<
115+
DiscourseNode | undefined
116+
>(() => {
113117
const node = discourseNodes.find((n) => n.type === nodeType);
114-
return node || discourseNodes[0];
118+
return node;
115119
});
116120

117121
const nodeFormat = useMemo(() => {
118-
return selectedNodeType.format || "";
122+
return selectedNodeType?.format || "";
119123
}, [selectedNodeType]);
120124

121125
const referencedNode = useMemo(() => {
@@ -160,6 +164,19 @@ const ModifyNodeDialog = ({
160164
if (contentRequestIdRef.current === req && alive) {
161165
setOptions((prev) => ({ ...prev, content: results }));
162166
}
167+
} else {
168+
const rawResults = await getAllDiscourseNodesSince(
169+
undefined,
170+
discourseNodes,
171+
);
172+
const results = rawResults.map((r) => ({
173+
text: r.text,
174+
uid: r.source_local_id,
175+
discourseNodeType: r.type,
176+
}));
177+
if (contentRequestIdRef.current === req && alive) {
178+
setOptions((prev) => ({ ...prev, content: results }));
179+
}
163180
}
164181
} catch (error) {
165182
if (contentRequestIdRef.current === req && alive) {
@@ -224,11 +241,24 @@ const ModifyNodeDialog = ({
224241
alive = false;
225242
refAlive = false;
226243
};
227-
}, [selectedNodeType, referencedNode]);
228-
229-
const setValue = useCallback((r: Result) => {
230-
setContent(r);
231-
}, []);
244+
}, [selectedNodeType, referencedNode, discourseNodes]);
245+
246+
const setValue = useCallback(
247+
(r: Result) => {
248+
setContent(r);
249+
if (!selectedNodeType && r.uid) {
250+
const detectedType = r.discourseNodeType;
251+
if (detectedType) {
252+
const nt = discourseNodes.find((n) => n.type === detectedType);
253+
if (nt) {
254+
setSelectedNodeType(nt);
255+
setError("");
256+
}
257+
}
258+
}
259+
},
260+
[selectedNodeType, discourseNodes],
261+
);
232262

233263
const setReferencedNodeValueCallback = useCallback((r: Result) => {
234264
setReferencedNodeValue(r);
@@ -304,9 +334,13 @@ const ModifyNodeDialog = ({
304334

305335
const onSubmit = async () => {
306336
if (!content.text.trim()) return;
337+
if (!selectedNodeType && !isContentLocked) {
338+
setError("Please select a node type");
339+
return;
340+
}
307341
posthog.capture("Modify Node Dialog: Submit Triggered", {
308342
mode,
309-
nodeType: selectedNodeType.type,
343+
nodeType: selectedNodeType?.type,
310344
});
311345
try {
312346
if (mode === "create") {
@@ -326,7 +360,7 @@ const ModifyNodeDialog = ({
326360
await addImageToPage({
327361
pageUid,
328362
imageUrl,
329-
configPageUid: selectedNodeType.type,
363+
configPageUid: selectedNodeType?.type || "",
330364
extensionAPI,
331365
});
332366
}
@@ -371,6 +405,7 @@ const ModifyNodeDialog = ({
371405
},
372406
);
373407
} else {
408+
if (!selectedNodeType) return;
374409
formattedTitle = await getNewDiscourseNodeText({
375410
text: content.text.trim(),
376411
nodeType: selectedNodeType.type,
@@ -384,7 +419,7 @@ const ModifyNodeDialog = ({
384419
// Create new discourse node
385420
const newPageUid = await createDiscourseNode({
386421
text: formattedTitle,
387-
configPageUid: selectedNodeType.type,
422+
configPageUid: selectedNodeType?.type ?? "",
388423
extensionAPI,
389424
imageUrl,
390425
});
@@ -505,56 +540,62 @@ const ModifyNodeDialog = ({
505540
style={{ pointerEvents: "all" }}
506541
>
507542
<div className={`${Classes.DIALOG_BODY} flex flex-col gap-4`}>
543+
{/* Content Input */}
544+
<Label className="w-full">
545+
Content
546+
<FuzzySelectInput
547+
value={content}
548+
setValue={setValue}
549+
options={options.content}
550+
placeholder={
551+
loading
552+
? "..."
553+
: selectedNodeType
554+
? `Enter a ${selectedNodeType.text.toLowerCase()} ...`
555+
: "Search all nodes..."
556+
}
557+
mode={mode}
558+
isLocked={isContentLocked}
559+
autoFocus={!isContentLocked}
560+
/>
561+
</Label>
562+
508563
{/* Node Type Selector */}
509564
<div className="flex w-full">
510-
<Label autoFocus={false}>
565+
<Label autoFocus={false} className="w-full">
511566
Node Type
512567
<MenuItemSelect
513568
items={discourseNodes.map((n) => n.type)}
514569
transformItem={(t) =>
515570
discourseNodes.find((n) => n.type === t)?.text || t
516571
}
517-
activeItem={selectedNodeType.type}
572+
activeItem={selectedNodeType?.type ?? null}
518573
onItemSelect={(t) => {
519574
const nt = discourseNodes.find((n) => n.type === t);
520575
if (nt) {
521576
setSelectedNodeType(nt);
522577
setReferencedNodeValue({ text: "", uid: "" });
578+
setError("");
523579
}
524580
}}
525-
disabled={mode === "edit" || disableNodeTypeChange}
526-
popoverProps={{ openOnTargetFocus: false }}
527-
className={
528-
mode === "edit" || disableNodeTypeChange
529-
? "cursor-not-allowed opacity-50"
530-
: ""
581+
disabled={
582+
mode === "edit" || disableNodeTypeChange || isContentLocked
531583
}
584+
popoverProps={{ openOnTargetFocus: false }}
585+
ButtonProps={{
586+
className:
587+
mode === "edit" || disableNodeTypeChange || isContentLocked
588+
? "cursor-not-allowed opacity-50"
589+
: "",
590+
}}
532591
/>
533592
</Label>
534593
</div>
535594

536-
{/* Content Input */}
537-
<div className="w-full">
538-
<Label>Content</Label>
539-
<FuzzySelectInput
540-
value={content}
541-
setValue={setValue}
542-
options={options.content}
543-
placeholder={
544-
loading
545-
? "..."
546-
: `Enter a ${selectedNodeType.text.toLowerCase()} ...`
547-
}
548-
mode={mode}
549-
isLocked={isContentLocked}
550-
autoFocus={!isContentLocked}
551-
/>
552-
</div>
553-
554595
{/* Referenced Node Input */}
555596
{referencedNode && !isContentLocked && mode === "create" && (
556-
<div className="w-full">
557-
<Label>{referencedNode.name}</Label>
597+
<Label className="w-full">
598+
{referencedNode.name}
558599
<FuzzySelectInput
559600
value={referencedNodeValue}
560601
setValue={setReferencedNodeValueCallback}
@@ -564,7 +605,7 @@ const ModifyNodeDialog = ({
564605
isLocked={isReferencedNodeLocked}
565606
autoFocus={false}
566607
/>
567-
</div>
608+
</Label>
568609
)}
569610
</div>
570611
{/* Submit Button */}

apps/roam/src/styles/discourseGraphStyles.css

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,22 @@ div.roamjs-discourse-drawer div.bp3-drawer {
171171
min-height: 96px;
172172
}
173173

174+
/*
175+
Override button focus inside roamjs-canvas-dialog.
176+
177+
Reason:
178+
- Blueprint defines a global `:focus` outline, but does NOT provide a
179+
button-specific focus rule with higher specificity.
180+
- If a global `outline: none` (or similar override) is applied elsewhere,
181+
buttons can lose their visible focus state.
182+
- This restores a proper focus outline for accessibility, scoped locally,
183+
matching Blueprint’s default focus styling intent.
184+
*/
185+
.roamjs-canvas-dialog .bp3-button:focus-visible {
186+
outline: 2px auto rgba(19, 124, 189, 0.6);
187+
outline-offset: 2px;
188+
}
189+
174190
.bp3-tabs.bp3-vertical > .bp3-tab-panel {
175191
flex-grow: 1;
176192
}

apps/roam/src/utils/getAllDiscourseNodesSince.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,10 @@ export const getDiscourseNodeTypeWithSettingsBlockNodes = async (
6464
};
6565

6666
export const getAllDiscourseNodesSince = async (
67-
since: ISODateString,
67+
since: ISODateString | undefined,
6868
nodeTypes: DiscourseNode[],
6969
): Promise<RoamDiscourseNodeData[]> => {
70-
const sinceMs = new Date(since).getTime();
70+
const sinceMs = since ? new Date(since).getTime() : 0;
7171
if (!nodeTypes.length) {
7272
return [];
7373
}

apps/roam/src/utils/registerCommandPaletteCommands.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -189,19 +189,9 @@ export const registerCommandPaletteCommands = (onloadArgs: OnloadArgs) => {
189189

190190
const selectionStart = uid ? getSelectionStartForBlock(uid) : 0;
191191

192-
const defaultNodeType =
193-
getDiscourseNodes().filter(excludeDefaultNodes)[0]?.type;
194-
if (!defaultNodeType) {
195-
renderToast({
196-
id: "create-discourse-node-command-no-types",
197-
content: "No discourse node types found in settings.",
198-
});
199-
return;
200-
}
201-
202192
renderModifyNodeDialog({
203193
mode: "create",
204-
nodeType: defaultNodeType,
194+
nodeType: "",
205195
initialValue: { text: "", uid: "" },
206196
extensionAPI,
207197
onSuccess: async (result) => {

0 commit comments

Comments
 (0)