Skip to content

Commit 154c7cd

Browse files
ENG-1741 Prevent duplicate keyboard shortcuts for node types (#1020)
* ENG-1741 Prevent duplicate keyboard shortcuts for node types - On node creation, skip auto-assigning the first-letter shortcut if it is already taken by any existing node type; leave empty instead - In node settings, validate the shortcut field against all other node types and surface an inline error when a conflict is detected Co-authored-by: Cursor <cursoragent@cursor.com> * address PR comment --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 8de5853 commit 154c7cd

2 files changed

Lines changed: 51 additions & 2 deletions

File tree

apps/roam/src/components/settings/DiscourseNodeConfigPanel.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,15 @@ const DiscourseNodeConfigPanel: React.FC<DiscourseNodeConfigPanelProps> = ({
8080
className="select-none"
8181
disabled={!label}
8282
onClick={() => {
83-
const shortcut = label.slice(0, 1).toUpperCase();
83+
const candidateShortcut = label.slice(0, 1).toUpperCase();
84+
const existingShortcuts = new Set(
85+
getDiscourseNodes()
86+
.map((n) => n.shortcut.toUpperCase())
87+
.filter(Boolean),
88+
);
89+
const shortcut = existingShortcuts.has(candidateShortcut)
90+
? ""
91+
: candidateShortcut;
8492
const format = `[[${label.slice(0, 3).toUpperCase()}]] - {content}`;
8593
posthog.capture("Discourse Node: Type Created", { label: label });
8694
void createPage({

apps/roam/src/components/settings/NodeConfig.tsx

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React, { useState, useCallback, useEffect } from "react";
2-
import { DiscourseNode } from "~/utils/getDiscourseNodes";
2+
import getDiscourseNodes, { DiscourseNode } from "~/utils/getDiscourseNodes";
33
import DualWriteBlocksPanel from "./components/EphemeralBlocksPanel";
44
import { getSubTree } from "roamjs-components/util";
55
import Description from "roamjs-components/components/Description";
@@ -92,9 +92,11 @@ const NodeConfig = ({
9292
const [selectedTabId, setSelectedTabId] = useState<TabId>("general");
9393
const [tagError, setTagError] = useState("");
9494
const [formatError, setFormatError] = useState("");
95+
const [shortcutError, setShortcutError] = useState("");
9596

9697
const [tagValue, setTagValue] = useState(node.tag || "");
9798
const [formatValue, setFormatValue] = useState(node.format || "");
99+
const [shortcutValue, setShortcutValue] = useState(node.shortcut || "");
98100
const validate = useCallback(
99101
({
100102
tag,
@@ -155,6 +157,43 @@ const NodeConfig = ({
155157
validate({ tag: tagValue, format: formatValue });
156158
}, [tagValue, formatValue, validate]);
157159

160+
const validateShortcut = useCallback(
161+
(value: string): boolean => {
162+
if (!value) {
163+
setShortcutError("");
164+
return true;
165+
}
166+
const normalizedValue = value.toUpperCase();
167+
const taken = getDiscourseNodes()
168+
.filter((n) => n.type !== node.type && n.shortcut)
169+
.map((n) => ({
170+
shortcut: n.shortcut.toUpperCase(),
171+
label: n.text,
172+
}));
173+
const matchingNodes = taken.filter((n) => n.shortcut === normalizedValue);
174+
if (matchingNodes.length) {
175+
setShortcutError(
176+
`Shortcut "${normalizedValue}" is already used by: ${matchingNodes
177+
.map((n) => `"${n.label}"`)
178+
.join(", ")}.`,
179+
);
180+
return false;
181+
}
182+
setShortcutError("");
183+
return true;
184+
},
185+
[node.type],
186+
);
187+
188+
const handleShortcutChange = useCallback(
189+
(value: string) => {
190+
if (validateShortcut(value)) {
191+
setShortcutValue(value);
192+
}
193+
},
194+
[validateShortcut],
195+
);
196+
158197
return (
159198
<>
160199
<Tabs
@@ -184,6 +223,8 @@ const NodeConfig = ({
184223
description={`The trigger to quickly create a ${node.text} page from the node menu.`}
185224
settingKeys={[DISCOURSE_NODE_KEYS.shortcut]}
186225
initialValue={node.shortcut}
226+
error={shortcutError}
227+
onChange={handleShortcutChange}
187228
order={0}
188229
parentUid={node.type}
189230
uid={shortcutUid}

0 commit comments

Comments
 (0)