Skip to content

[ENG-299] Trigger for DG summoning menu #160

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

Closed
trangdoan982 wants to merge 2 commits into from
Closed

[ENG-299] Trigger for DG summoning menu #160

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
57c7454
curr progress
trangdoan982 May 16, 2025
846022c
rm logs
trangdoan982 May 16, 2025
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
8 changes: 4 additions & 4 deletions apps/roam/src/components/DiscourseNodeMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,18 +197,18 @@ export const render = (props: Props) => {
};

// node_modules\@blueprintjs\core\lib\esm\components\hotkeys\hotkeyParser.js
const isMac = () => {
export const isMac = () => {
const platform =
typeof navigator !== "undefined" ? navigator.platform : undefined;
return platform == null ? false : /Mac|iPod|iPhone|iPad/.test(platform);
};
const MODIFIER_BIT_MASKS = {
export const MODIFIER_BIT_MASKS = {
alt: 1,
ctrl: 2,
meta: 4,
shift: 8,
};
const ALIASES: { [key: string]: string } = {
export const ALIASES: { [key: string]: string } = {
cmd: "meta",
command: "meta",
escape: "esc",
Expand All @@ -219,7 +219,7 @@ const ALIASES: { [key: string]: string } = {
return: "enter",
win: "meta",
};
const normalizeKeyCombo = (combo: string) => {
export const normalizeKeyCombo = (combo: string) => {
const keys = combo.replace(/\s/g, "").split("+");
return keys.map(function (key) {
const keyName = ALIASES[key] != null ? ALIASES[key] : key;
Expand Down
123 changes: 114 additions & 9 deletions apps/roam/src/components/DiscourseNodeSearchMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import {
Position,
Checkbox,
Button,
InputGroup,
IKeyCombo,
getKeyCombo,
} from "@blueprintjs/core";
import ReactDOM from "react-dom";
import getUids from "roamjs-components/dom/getUids";
Expand All @@ -23,11 +26,14 @@ import getDiscourseNodes, { DiscourseNode } from "~/utils/getDiscourseNodes";
import getDiscourseNodeFormatExpression from "~/utils/getDiscourseNodeFormatExpression";
import { escapeCljString } from "~/utils/formatUtils";
import { Result } from "~/utils/types";
import { OnloadArgs } from "roamjs-components/types";
import { getModifiersFromCombo, normalizeKeyCombo } from "./DiscourseNodeMenu";

type Props = {
textarea: HTMLTextAreaElement;
triggerPosition: number;
onClose: () => void;
extensionAPI: OnloadArgs["extensionAPI"];
};

const waitForBlock = (
Expand All @@ -51,6 +57,7 @@ const NodeSearchMenu = ({
onClose,
textarea,
triggerPosition,
extensionAPI,
}: { onClose: () => void } & Props) => {
const [activeIndex, setActiveIndex] = useState(0);
const [searchTerm, setSearchTerm] = useState("");
Expand Down Expand Up @@ -236,19 +243,15 @@ const NodeSearchMenu = ({
);

const handleTextAreaInput = useCallback(() => {
const atTriggerRegex = /@(.*)$/;
const textBeforeCursor = textarea.value.substring(
const textAfterTrigger = textarea.value.substring(
triggerPosition,
textarea.selectionStart,
);
const match = atTriggerRegex.exec(textBeforeCursor);
if (match) {
debouncedSearchTerm(match[1]);
} else {
onClose();
return;

if (textAfterTrigger.length > 0) {
debouncedSearchTerm(textAfterTrigger);
}
}, [textarea, onClose, debouncedSearchTerm, triggerPosition]);
}, [textarea, debouncedSearchTerm, triggerPosition]);
Comment on lines +246 to +254
Copy link
Contributor

@coderabbitai coderabbitai bot May 16, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Empty query leaves menu open – UI feels “stuck”

handleTextAreaInput no longer closes the menu when the user backspaces past the trigger.
That means the pop-over lingers with an empty result list, which can feel broken.

-    if (textAfterTrigger.length > 0) {
-      debouncedSearchTerm(textAfterTrigger);
-    }
+    if (textAfterTrigger.length > 0) {
+      debouncedSearchTerm(textAfterTrigger);
+    } else {
+      onClose();          // hide menu when no query is present
+    }

Alternatively, keep it open but call debouncedSearchTerm("") so the UI refreshes to an explicit “No matches” state.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const textAfterTrigger = textarea.value.substring(
triggerPosition,
textarea.selectionStart,
);
const match = atTriggerRegex.exec(textBeforeCursor);
if (match) {
debouncedSearchTerm(match[1]);
} else {
onClose();
return;
if (textAfterTrigger.length > 0) {
debouncedSearchTerm(textAfterTrigger);
}
}, [textarea, onClose, debouncedSearchTerm, triggerPosition]);
}, [textarea, debouncedSearchTerm, triggerPosition]);
const textAfterTrigger = textarea.value.substring(
triggerPosition,
textarea.selectionStart,
);
if (textAfterTrigger.length > 0) {
debouncedSearchTerm(textAfterTrigger);
} else {
onClose(); // hide menu when no query is present
}
}, [textarea, debouncedSearchTerm, triggerPosition]);
🤖 Prompt for AI Agents 
In apps/roam/src/components/DiscourseNodeSearchMenu.tsx around lines 246 to 254,
the current code does not close the menu or refresh the UI when the query is
empty after backspacing past the trigger, causing the menu to remain open with
no results. Modify the logic so that if textAfterTrigger is empty, either close
the menu explicitly or call debouncedSearchTerm with an empty string to refresh
the UI and show a "No matches" state, ensuring the menu does not feel stuck.


const keydownListener = useCallback(
(e: KeyboardEvent) => {
Expand Down Expand Up @@ -520,4 +523,106 @@ export const renderDiscourseNodeSearchMenu = (props: Props) => {
);
};

export const NodeSearchMenuTriggerComponent = ({
extensionAPI,
}: {
extensionAPI: OnloadArgs["extensionAPI"];
}) => {
const inputRef = useRef<HTMLInputElement>(null);
const [isActive, setIsActive] = useState(false);
const [comboKey, setComboKey] = useState<IKeyCombo>(
() =>
(extensionAPI.settings.get(
"discourse-node-search-trigger",
) as IKeyCombo) || { modifiers: 0, key: "@" },
);

const handleKeyDown = useCallback(
(e: React.KeyboardEvent) => {
e.stopPropagation();
e.preventDefault();
const comboObj = getKeyCombo(e.nativeEvent);
if (!comboObj.key) return;
const specialCharMap = {
"~": { key: "`", modifiers: 8 },
"!": { key: "1", modifiers: 8 },
"@": { key: "2", modifiers: 8 },
"#": { key: "3", modifiers: 8 },
$: { key: "4", modifiers: 8 },
"%": { key: "5", modifiers: 8 },
"^": { key: "6", modifiers: 8 },
"&": { key: "7", modifiers: 8 },
"*": { key: "8", modifiers: 8 },
"(": { key: "9", modifiers: 8 },
")": { key: "0", modifiers: 8 },
_: { key: "-", modifiers: 8 },
"+": { key: "=", modifiers: 8 },
"{": { key: "[", modifiers: 8 },
"}": { key: "]", modifiers: 8 },
"|": { key: "\\", modifiers: 8 },
":": { key: ";", modifiers: 8 },
'"': { key: "'", modifiers: 8 },
"<": { key: ",", modifiers: 8 },
">": { key: ".", modifiers: 8 },
"?": { key: "/", modifiers: 8 },
};

for (const [specialChar, mapping] of Object.entries(specialCharMap)) {
if (
comboObj.key === mapping.key &&
comboObj.modifiers === mapping.modifiers
) {
setComboKey({ modifiers: 0, key: specialChar });
extensionAPI.settings.set("discourse-node-search-trigger", {
modifiers: 0,
key: specialChar,
});
return;
}
}

setComboKey(comboObj);
extensionAPI.settings.set("discourse-node-search-trigger", comboObj);
},
[extensionAPI],
);

const shortcut = useMemo(() => {
if (!comboKey.key) return "";

const modifiers = getModifiersFromCombo(comboKey);
const comboString = [...modifiers, comboKey.key].join("+");
return normalizeKeyCombo(comboString).join("+");
}, [comboKey]);

return (
<InputGroup
inputRef={inputRef}
placeholder={
isActive ? "Press keys ..." : "Click to set trigger (default: @)"
}
value={shortcut}
onKeyDown={handleKeyDown}
onFocus={() => setIsActive(true)}
onBlur={() => setIsActive(false)}
rightElement={
<Button
hidden={
!comboKey.key || (comboKey.key === "@" && comboKey.modifiers === 0)
}
icon={"remove"}
onClick={() => {
setComboKey({ modifiers: 0, key: "@" });
extensionAPI.settings.set("discourse-node-search-trigger", {
modifiers: 0,
key: "@",
});
}}
minimal
/>
}
/>
);
};

export default NodeSearchMenu;
12 changes: 12 additions & 0 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 @@ -3,6 +3,7 @@ import { OnloadArgs } from "roamjs-components/types";
import { Label, Checkbox } from "@blueprintjs/core";
import Description from "roamjs-components/components/Description";
import { NodeMenuTriggerComponent } from "~/components/DiscourseNodeMenu";
import { NodeSearchMenuTriggerComponent } from "~/components/DiscourseNodeSearchMenu";
import {
getOverlayHandler,
onPageRefObserverChange,
Expand All @@ -28,6 +29,17 @@ const HomePersonalSettings = ({ onloadArgs }: { onloadArgs: OnloadArgs }) => {
/>
<NodeMenuTriggerComponent extensionAPI={extensionAPI} />
</Label>

<Label>
Node Search Menu Trigger
<Description
description={
"Customize the trigger key for the Discourse Node Search Menu (default: @). Must refresh after editing."
}
/>
<NodeSearchMenuTriggerComponent extensionAPI={extensionAPI} />
</Label>

<Checkbox
defaultChecked={
extensionAPI.settings.get("discourse-context-overlay") as boolean
Expand Down
94 changes: 71 additions & 23 deletions apps/roam/src/utils/initializeObserversAndListeners.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import {
createButtonObserver,
getPageTitleValueByHtmlElement,
} from "roamjs-components/dom";
import { createBlock } from "roamjs-components/writes";
import { createBlock, updateBlock } from "roamjs-components/writes";
import getUids from "roamjs-components/dom/getUids";
import { renderLinkedReferenceAdditions } from "~/utils/renderLinkedReferenceAdditions";
import { createConfigObserver } from "roamjs-components/components/ConfigPage";
import { renderTldrawCanvas } from "~/components/canvas/Tldraw";
Expand Down Expand Up @@ -148,6 +149,11 @@ export const initObservers = async ({
) as IKeyCombo) || undefined;
const personalTrigger = personalTriggerCombo?.key;
const personalModifiers = getModifiersFromCombo(personalTriggerCombo);

const nodeSearchTriggerCombo =
(onloadArgs.extensionAPI.settings.get(
"discourse-node-search-trigger",
) as IKeyCombo) || undefined;
const handleNodeMenuRender = (target: HTMLElement, evt: KeyboardEvent) => {
if (
target.tagName === "TEXTAREA" &&
Expand All @@ -162,6 +168,50 @@ export const initObservers = async ({
}
};

const handleNodeSearchRender = (target: HTMLElement, evt: KeyboardEvent) => {
if (
target.tagName === "TEXTAREA" &&
target.classList.contains("rm-block-input")
) {
const textarea = target as HTMLTextAreaElement;
const location = window.roamAlphaAPI.ui.getFocusedBlock();
if (!location) return;

const cursorPos = textarea.selectionStart;
const isBeginningOrAfterSpace =
cursorPos === 0 ||
textarea.value.charAt(cursorPos - 1) === " " ||
textarea.value.charAt(cursorPos - 1) === "\n";

if (isBeginningOrAfterSpace) {
const triggerChar = nodeSearchTriggerCombo?.key || "@";
let triggerPosition = cursorPos;

if (!evt.isComposing && evt.key !== triggerChar) {
const text = textarea.value;
const newText =
text.slice(0, cursorPos) + triggerChar + text.slice(cursorPos);

const blockUid = getUids(textarea).blockUid;
if (blockUid) {
updateBlock({ uid: blockUid, text: newText });
}
triggerPosition = cursorPos;
}
Comment on lines +190 to +200
Copy link
Contributor

@coderabbitai coderabbitai bot May 16, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Asynchronous write may race with UI state and cursor management
updateBlock is asynchronous but the surrounding function is not async, and its returned promise is ignored.
• If the Roam API lags, renderDiscourseNodeSearchMenu can pop up before the block text actually includes the trigger character, causing a mismatch between what the menu thinks is in the block and the real DOM.
• The textarea value is never updated locally, so the user keeps seeing the pre-update text until Roam re-renders, which feels glitchy, and the caret position will jump.

-          updateBlock({ uid: blockUid, text: newText });
+          await updateBlock({ uid: blockUid, text: newText });
+          // Keep the local textarea in-sync and restore cursor
+          textarea.value = newText;
+          textarea.setSelectionRange(triggerPosition + 1, triggerPosition + 1);

Turning handleNodeSearchRender into an async function (and awaiting it from the listener) prevents the race and preserves UX.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In apps/roam/src/utils/initializeObserversAndListeners.ts around lines 190 to
200, the updateBlock call is asynchronous but not awaited, causing potential
race conditions with UI updates and cursor management. To fix this, make the
surrounding function async and await the updateBlock call to ensure the block
text is updated before continuing. Also, update the textarea value locally to
reflect the new text immediately and manage the caret position to prevent jumps,
preserving a smooth user experience.


renderDiscourseNodeSearchMenu({
onClose: () => {},
textarea: textarea,
triggerPosition: triggerPosition,
extensionAPI: onloadArgs.extensionAPI,
});

evt.preventDefault();
evt.stopPropagation();
}
}
};

const nodeMenuTriggerListener = (e: Event) => {
const evt = e as KeyboardEvent;
const target = evt.target as HTMLElement;
Expand Down Expand Up @@ -191,28 +241,26 @@ export const initObservers = async ({
const target = evt.target as HTMLElement;
if (document.querySelector(".discourse-node-search-menu")) return;

if (evt.key === "@" || (evt.key === "2" && evt.shiftKey)) {
if (
target.tagName === "TEXTAREA" &&
target.classList.contains("rm-block-input")
) {
const textarea = target as HTMLTextAreaElement;
const location = window.roamAlphaAPI.ui.getFocusedBlock();
if (!location) return;

const cursorPos = textarea.selectionStart;
const isBeginningOrAfterSpace =
cursorPos === 0 ||
textarea.value.charAt(cursorPos - 1) === " " ||
textarea.value.charAt(cursorPos - 1) === "\n";
if (isBeginningOrAfterSpace) {
renderDiscourseNodeSearchMenu({
onClose: () => {},
textarea: textarea,
triggerPosition: cursorPos,
});
}
}
// If no personal trigger is set or key is empty, the feature is disabled
if (!nodeSearchTriggerCombo?.key) return;

const personalTrigger = nodeSearchTriggerCombo.key;
const personalModifiers = getModifiersFromCombo(nodeSearchTriggerCombo);

let triggerMatched = false;

console.log("evt.key", evt.key);
console.log("personal trigger", personalTrigger, nodeSearchTriggerCombo);
if (evt.key === personalTrigger) {
triggerMatched =
(!personalModifiers.includes("ctrl") || evt.ctrlKey) &&
(!personalModifiers.includes("shift") || evt.shiftKey) &&
(!personalModifiers.includes("alt") || evt.altKey) &&
(!personalModifiers.includes("meta") || evt.metaKey);
}
Comment on lines +254 to +260
Copy link
Contributor

@coderabbitai coderabbitai bot May 16, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Trigger matching is fragile – make it case-insensitive and reuse normalizeKeyCombo
evt.key differs between "d" and "D" depending on Shift, so Shift-modified combos can fail to match even when modifiers are correct. Compare keys after normalisation:

-    if (evt.key === personalTrigger) {
+    if (evt.key.toLowerCase() === personalTrigger.toLowerCase()) {

Even better, call the already-exported normalizeKeyCombo helper and compare the fully-normalised combo objects to avoid hand-rolled logic.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (evt.key === personalTrigger) {
triggerMatched =
(!personalModifiers.includes("ctrl") || evt.ctrlKey) &&
(!personalModifiers.includes("shift") || evt.shiftKey) &&
(!personalModifiers.includes("alt") || evt.altKey) &&
(!personalModifiers.includes("meta") || evt.metaKey);
}
if (evt.key.toLowerCase() === personalTrigger.toLowerCase()) {
triggerMatched =
(!personalModifiers.includes("ctrl") || evt.ctrlKey) &&
(!personalModifiers.includes("shift") || evt.shiftKey) &&
(!personalModifiers.includes("alt") || evt.altKey) &&
(!personalModifiers.includes("meta") || evt.metaKey);
}
🤖 Prompt for AI Agents 
In apps/roam/src/utils/initializeObserversAndListeners.ts around lines 254 to
260, the current trigger matching logic compares evt.key directly, which is
case-sensitive and can fail for Shift-modified keys. To fix this, replace the
manual key and modifier checks by using the existing normalizeKeyCombo helper to
normalize both the event key combo and the personalTrigger combo, then compare
these normalized objects for equality. This ensures case-insensitive and
consistent matching without hand-rolled logic.


if (triggerMatched) {
handleNodeSearchRender(target, evt);
}
};

Expand Down