Skip to content

Commit 2ed2994

Browse files
authored
improvement: detect and warn about duplicate shortcuts (#7412)
Closes #7395 <img width="877" height="593" alt="Screenshot 2025-12-05 at 10 10 05 AM" src="https://github.com/user-attachments/assets/220dc1bb-c711-4e45-b855-7003aa9985ef" />
1 parent 3214447 commit 2ed2994

File tree

9 files changed

+679
-11
lines changed

9 files changed

+679
-11
lines changed
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/* Copyright 2024 Marimo. All rights reserved. */
2+
3+
import { AlertTriangleIcon } from "lucide-react";
4+
import { KeyboardHotkeys } from "@/components/shortcuts/renderShortcut";
5+
import { Alert, AlertDescription, AlertTitle } from "@/components/ui/alert";
6+
import type { DuplicateGroup } from "@/hooks/useDuplicateShortcuts";
7+
8+
interface DuplicateShortcutBannerProps {
9+
duplicates: DuplicateGroup[];
10+
}
11+
12+
/**
13+
* Banner component that warns about duplicate keyboard shortcuts.
14+
* Displays a warning when multiple actions share the same key binding.
15+
*/
16+
export const DuplicateShortcutBanner: React.FC<
17+
DuplicateShortcutBannerProps
18+
> = ({ duplicates }) => {
19+
// Don't render if no duplicates
20+
if (duplicates.length === 0) {
21+
return null;
22+
}
23+
24+
return (
25+
<Alert variant="warning" className="mb-4">
26+
<AlertTriangleIcon className="h-4 w-4" />
27+
<AlertTitle>Duplicate shortcuts</AlertTitle>
28+
<AlertDescription>
29+
<p className="mb-2">
30+
Multiple actions are assigned to the same keyboard shortcut:
31+
</p>
32+
<ul className="space-y-2">
33+
{duplicates.map(({ key, actions }) => (
34+
<li key={key} className="text-xs">
35+
<div className="flex items-center gap-2 mb-1">
36+
<KeyboardHotkeys shortcut={key} />
37+
<span className="font-semibold">is used by:</span>
38+
</div>
39+
<ul className="ml-6 list-disc">
40+
{actions.map(({ action, name }) => (
41+
<li key={action}>{name}</li>
42+
))}
43+
</ul>
44+
</li>
45+
))}
46+
</ul>
47+
</AlertDescription>
48+
</Alert>
49+
);
50+
};

frontend/src/components/editor/controls/keyboard-shortcuts.tsx

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* Copyright 2024 Marimo. All rights reserved. */
22

33
import { atom, useAtom, useAtomValue } from "jotai";
4-
import { EditIcon, XIcon } from "lucide-react";
4+
import { AlertTriangleIcon, EditIcon, XIcon } from "lucide-react";
55
import { useState } from "react";
66
import { Button } from "@/components/ui/button";
77
import { Input } from "@/components/ui/input";
@@ -15,6 +15,7 @@ import {
1515
} from "@/core/hotkeys/hotkeys";
1616
import { isPlatformMac } from "@/core/hotkeys/shortcuts";
1717
import { useRequestClient } from "@/core/network/requests";
18+
import { useDuplicateShortcuts } from "../../../hooks/useDuplicateShortcuts";
1819
import { useHotkey } from "../../../hooks/useHotkey";
1920
import { KeyboardHotkeys } from "../../shortcuts/renderShortcut";
2021
import {
@@ -25,6 +26,7 @@ import {
2526
DialogPortal,
2627
DialogTitle,
2728
} from "../../ui/dialog";
29+
import { DuplicateShortcutBanner } from "./duplicate-shortcut-banner";
2830

2931
export const keyboardShortcutsAtom = atom(false);
3032

@@ -37,6 +39,10 @@ export const KeyboardShortcuts: React.FC = () => {
3739
const [config, setConfig] = useResolvedMarimoConfig();
3840
const hotkeys = useAtomValue(hotkeysAtom);
3941
const { saveUserConfig } = useRequestClient();
42+
const { duplicates, hasDuplicate, getDuplicatesFor } = useDuplicateShortcuts(
43+
hotkeys,
44+
"Markdown",
45+
);
4046

4147
useHotkey("global.showHelp", () => setIsOpen((v) => !v));
4248

@@ -214,6 +220,9 @@ export const KeyboardShortcuts: React.FC = () => {
214220
);
215221
}
216222

223+
const isDuplicate = hasDuplicate(action);
224+
const duplicateActions = isDuplicate ? getDuplicatesFor(action) : [];
225+
217226
return (
218227
<div
219228
key={action}
@@ -231,7 +240,20 @@ export const KeyboardShortcuts: React.FC = () => {
231240
<div className="w-3 h-3" />
232241
)}
233242
<KeyboardHotkeys className="justify-end" shortcut={hotkey.key} />
234-
<span>{hotkey.name.toLowerCase()}</span>
243+
<div className="flex items-center gap-1">
244+
<span>{hotkey.name.toLowerCase()}</span>
245+
{isDuplicate && (
246+
<div className="group relative inline-flex">
247+
<AlertTriangleIcon className="w-3 h-3 text-(--yellow-11)" />
248+
<div className="invisible group-hover:visible absolute left-0 top-5 z-10 w-max max-w-xs rounded-md bg-(--yellow-2) border border-(--yellow-7) p-2 text-xs text-(--yellow-11) shadow-md">
249+
Also used by:{" "}
250+
{duplicateActions
251+
.map((a) => hotkeys.getHotkey(a).name.toLowerCase())
252+
.join(", ")}
253+
</div>
254+
</div>
255+
)}
256+
</div>
235257
</div>
236258
);
237259
};
@@ -279,6 +301,7 @@ export const KeyboardShortcuts: React.FC = () => {
279301
<DialogHeader>
280302
<DialogTitle>Shortcuts</DialogTitle>
281303
</DialogHeader>
304+
<DuplicateShortcutBanner duplicates={duplicates} />
282305
<div className="flex flex-row gap-3">
283306
<div className="w-1/2">
284307
{renderGroup("Editing")}

frontend/src/components/editor/output/__tests__/ansi-reduce.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -534,13 +534,13 @@ describe("AnsiReducer streaming with append()", () => {
534534
describe("AnsiReducer color preservation", () => {
535535
const CASES = [
536536
// SGR sequences
537-
"\u001b[34mBlue text\u001b[m normal text\u001b[31mRed text\u001b[0m",
537+
"\u001B[34mBlue text\u001B[m normal text\u001B[31mRed text\u001B[0m",
538538
// Complex SGR with parameters
539-
"\u001b[1;31mBold Red\u001b[0m \u001b[48;5;240mGray bg\u001b[0m",
539+
"\u001B[1;31mBold Red\u001B[0m \u001B[48;5;240mGray bg\u001B[0m",
540540
// Character set selection
541-
"Text\u001b(BMore\u001b(0Graphics\u001b(B",
541+
"Text\u001B(BMore\u001B(0Graphics\u001B(B",
542542
// Complex case
543-
"\u001b[34m[D 251201 15:32:24 cell_runner:695]\u001b(B\u001b[m Running post_execution hooks in context\n\u001b[34m[D 251201 15:32:24 hooks_post_execution:65]\u001b(B\u001b[m Acquiring graph lock to update cell import workspace\n\u001b[34m[D 251201 15:32:24 hooks_post_execution:67]\u001b(B\u001b[m Acquired graph lock to update import workspace.\n",
543+
"\u001B[34m[D 251201 15:32:24 cell_runner:695]\u001B(B\u001B[m Running post_execution hooks in context\n\u001B[34m[D 251201 15:32:24 hooks_post_execution:65]\u001B(B\u001B[m Acquiring graph lock to update cell import workspace\n\u001B[34m[D 251201 15:32:24 hooks_post_execution:67]\u001B(B\u001B[m Acquired graph lock to update import workspace.\n",
544544
];
545545

546546
test.each(CASES)("preserves ANSI color codes", (input) => {
@@ -554,12 +554,12 @@ describe("AnsiReducer color preservation", () => {
554554
// Test that color codes work alongside cursor movements
555555
// Note: when cursor moves up, lines below are discarded (tqdm behavior)
556556
const result = reducer.reduce(
557-
"Line1\n\u001b[31mRed\u001b[0m\u001b[1A\u001b[32mGreen\u001b[0m",
557+
"Line1\n\u001B[31mRed\u001B[0m\u001B[1A\u001B[32mGreen\u001B[0m",
558558
);
559559
// After moving up from row 1 to row 0, row 1 is discarded
560560
// Green is written at the end of row 0
561561
expect(result).toMatchInlineSnapshot(
562-
`"Line1 \u001b[32mGreen\u001b[0m"`,
562+
`"Line1 \u001B[32mGreen\u001B[0m"`,
563563
);
564564
});
565565
});

frontend/src/core/state/__mocks__/mocks.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* Copyright 2024 Marimo. All rights reserved. */
12
import type { Observable } from "../observable";
23

34
export function createMockObservable<T>(

0 commit comments

Comments
 (0)