Skip to content

Commit ac1d9be

Browse files
Cristhianzlerichare
andcommitted
fix(frontend): reset modifier-only saved shortcuts to default (#14245)
* fix: reset modifier-only shortcuts to default * fix(frontend): replace explicit any casts in shortcuts store tests Biome's noExplicitAny rule fires on three pre-existing `as any` casts in shortcuts.test.ts. The lint job checks every file a PR touches, so editing this test file surfaced them. Replace the casts with a typed helper that reads dynamic store keys through Record<string, unknown>. --------- Co-authored-by: Eric Hare <ericrhare@gmail.com>
1 parent e702963 commit ac1d9be

4 files changed

Lines changed: 162 additions & 36 deletions

File tree

src/frontend/src/pages/SettingsPage/pages/ShortcutsPage/EditShortcutButton/helpers.ts

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -47,29 +47,7 @@ export function checkForKeys(keys: string, keyToCompare: string): boolean {
4747
);
4848
}
4949

50-
/** Keys that only modify another key and cannot stand alone as a shortcut. */
51-
const MODIFIER_KEYS = new Set([
52-
"cmd",
53-
"ctrl",
54-
"control",
55-
"alt",
56-
"option",
57-
"shift",
58-
"meta",
59-
"mod",
60-
]);
61-
62-
/**
63-
* True when the recorded combination has no non-modifier key (e.g. "Cmd" or
64-
* "Ctrl + Shift"). Such a combination can never fire, so it must be rejected.
65-
*/
66-
export function isModifierOnlyCombination(recorded: string): boolean {
67-
const parts = recorded
68-
.split("+")
69-
.map((part) => part.trim().toLowerCase())
70-
.filter(Boolean);
71-
return parts.length === 0 || parts.every((part) => MODIFIER_KEYS.has(part));
72-
}
50+
export { isModifierOnlyCombination } from "@/utils/shortcuts";
7351

7452
export function normalizeRecordedCombination(recorded: string): string {
7553
const parts = recorded.split(" ");

src/frontend/src/stores/__tests__/shortcuts.test.ts

Lines changed: 89 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ const mockShortcuts = [
2828
},
2929
];
3030

31+
// Reads a shortcut key that is not part of the store type (tests intentionally
32+
// set arbitrary names to exercise the dynamic-key behavior of the store).
33+
const getDynamicShortcut = (store: unknown, name: string): unknown =>
34+
(store as Record<string, unknown>)[name];
35+
3136
describe("useShortcutsStore", () => {
3237
beforeEach(() => {
3338
localStorage.clear();
@@ -163,7 +168,9 @@ describe("useShortcutsStore", () => {
163168
result.current.updateUniqueShortcut("customShortcut", "mod+custom");
164169
});
165170

166-
expect((result.current as any).customShortcut).toBe("mod+custom");
171+
expect(getDynamicShortcut(result.current, "customShortcut")).toBe(
172+
"mod+custom",
173+
);
167174
});
168175

169176
it("should not affect shortcuts array when updating individual shortcuts", () => {
@@ -190,6 +197,85 @@ describe("useShortcutsStore", () => {
190197

191198
expect(result.current.shortcuts).toEqual(originalShortcuts);
192199
});
200+
201+
it("should keep valid custom shortcuts untouched", () => {
202+
const saved = [{ name: "Test", display_name: "Test", shortcut: "mod+t" }];
203+
localStorage.setItem("langflow-shortcuts", JSON.stringify(saved));
204+
const { result } = renderHook(() => useShortcutsStore());
205+
206+
act(() => {
207+
result.current.getShortcutsFromStorage();
208+
});
209+
210+
expect((result.current as unknown as Record<string, string>).Test).toBe(
211+
"mod+t",
212+
);
213+
expect(result.current.shortcuts).toEqual(saved);
214+
expect(JSON.parse(localStorage.getItem("langflow-shortcuts")!)).toEqual(
215+
saved,
216+
);
217+
});
218+
219+
it("should replace a modifier-only saved shortcut with the default for that action", () => {
220+
localStorage.setItem(
221+
"langflow-shortcuts",
222+
JSON.stringify([
223+
{ name: "Test", display_name: "Test", shortcut: "mod" },
224+
]),
225+
);
226+
const { result } = renderHook(() => useShortcutsStore());
227+
228+
act(() => {
229+
result.current.getShortcutsFromStorage();
230+
});
231+
232+
expect((result.current as unknown as Record<string, string>).Test).toBe(
233+
"t",
234+
);
235+
expect(result.current.shortcuts).toEqual([
236+
{ name: "Test", display_name: "Test", shortcut: "t" },
237+
]);
238+
});
239+
240+
it("should persist the sanitized combination back to localStorage", () => {
241+
localStorage.setItem(
242+
"langflow-shortcuts",
243+
JSON.stringify([
244+
{ name: "Test", display_name: "Test", shortcut: "ctrl+shift" },
245+
]),
246+
);
247+
const { result } = renderHook(() => useShortcutsStore());
248+
249+
act(() => {
250+
result.current.getShortcutsFromStorage();
251+
});
252+
253+
expect(JSON.parse(localStorage.getItem("langflow-shortcuts")!)).toEqual([
254+
{ name: "Test", display_name: "Test", shortcut: "t" },
255+
]);
256+
});
257+
258+
it("should drop a modifier-only shortcut that has no default", () => {
259+
localStorage.setItem(
260+
"langflow-shortcuts",
261+
JSON.stringify([
262+
{ name: "Ghost", display_name: "Ghost", shortcut: "mod" },
263+
]),
264+
);
265+
const { result } = renderHook(() => useShortcutsStore());
266+
267+
act(() => {
268+
result.current.getShortcutsFromStorage();
269+
});
270+
271+
expect(
272+
(result.current as unknown as Record<string, string>).Ghost,
273+
).toBeUndefined();
274+
expect(result.current.shortcuts).toEqual([]);
275+
expect(JSON.parse(localStorage.getItem("langflow-shortcuts")!)).toEqual(
276+
[],
277+
);
278+
});
193279
});
194280

195281
describe("state management", () => {
@@ -228,7 +314,7 @@ describe("useShortcutsStore", () => {
228314
result.current.updateUniqueShortcut("special", "mod+shift+~");
229315
});
230316

231-
expect((result.current as any).special).toBe("mod+shift+~");
317+
expect(getDynamicShortcut(result.current, "special")).toBe("mod+shift+~");
232318
});
233319

234320
it("should handle empty string shortcuts", () => {
@@ -238,7 +324,7 @@ describe("useShortcutsStore", () => {
238324
result.current.updateUniqueShortcut("empty", "");
239325
});
240326

241-
expect((result.current as any).empty).toBe("");
327+
expect(getDynamicShortcut(result.current, "empty")).toBe("");
242328
});
243329

244330
it("should handle shortcuts array with duplicate names", () => {

src/frontend/src/stores/shortcuts.ts

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,42 @@
11
import { create } from "zustand";
2+
import { isModifierOnlyCombination } from "@/utils/shortcuts";
23
import { toCamelCase } from "@/utils/utils";
34
import { defaultShortcuts } from "../constants/constants";
45
import type { shortcutsStoreType } from "../types/store";
56

7+
type SavedShortcut = {
8+
name: string;
9+
display_name: string;
10+
shortcut: string;
11+
};
12+
13+
/**
14+
* Older builds allowed recording modifier-only combinations (e.g. just "mod"),
15+
* which can never fire. Restore the default combination for those entries and
16+
* drop the ones that no longer map to a known action.
17+
*/
18+
function sanitizeSavedShortcuts(saved: SavedShortcut[]): {
19+
sanitized: SavedShortcut[];
20+
changed: boolean;
21+
} {
22+
let changed = false;
23+
const sanitized: SavedShortcut[] = [];
24+
saved.forEach((item) => {
25+
if (!isModifierOnlyCombination(item.shortcut)) {
26+
sanitized.push(item);
27+
return;
28+
}
29+
changed = true;
30+
const fallback = defaultShortcuts.find(
31+
(defaultItem) => toCamelCase(defaultItem.name) === toCamelCase(item.name),
32+
);
33+
if (fallback) {
34+
sanitized.push({ ...item, shortcut: fallback.shortcut });
35+
}
36+
});
37+
return { sanitized, changed };
38+
}
39+
640
export const useShortcutsStore = create<shortcutsStoreType>((set, get) => ({
741
shortcuts: defaultShortcuts,
842
setShortcuts: (newShortcuts) => {
@@ -42,17 +76,22 @@ export const useShortcutsStore = create<shortcutsStoreType>((set, get) => ({
4276
});
4377
},
4478
getShortcutsFromStorage: () => {
45-
if (localStorage.getItem("langflow-shortcuts")) {
46-
const savedShortcuts = localStorage.getItem("langflow-shortcuts");
47-
const savedArr = JSON.parse(savedShortcuts!);
48-
savedArr.forEach(({ name, shortcut }) => {
49-
const shortcutName = toCamelCase(name);
50-
set({
51-
[shortcutName]: shortcut,
52-
});
53-
});
54-
get().setShortcuts(savedArr);
79+
const savedShortcuts = localStorage.getItem("langflow-shortcuts");
80+
if (!savedShortcuts) {
81+
return;
82+
}
83+
const savedArr: SavedShortcut[] = JSON.parse(savedShortcuts);
84+
const { sanitized, changed } = sanitizeSavedShortcuts(savedArr);
85+
if (changed) {
86+
localStorage.setItem("langflow-shortcuts", JSON.stringify(sanitized));
5587
}
88+
sanitized.forEach(({ name, shortcut }) => {
89+
const shortcutName = toCamelCase(name);
90+
set({
91+
[shortcutName]: shortcut,
92+
});
93+
});
94+
get().setShortcuts(sanitized);
5695
},
5796
}));
5897

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/** Keys that only modify another key and cannot stand alone as a shortcut. */
2+
const MODIFIER_KEYS = new Set([
3+
"cmd",
4+
"ctrl",
5+
"control",
6+
"alt",
7+
"option",
8+
"shift",
9+
"meta",
10+
"mod",
11+
]);
12+
13+
/**
14+
* True when the recorded combination has no non-modifier key (e.g. "Cmd" or
15+
* "Ctrl + Shift"). Such a combination can never fire, so it must be rejected.
16+
*/
17+
export function isModifierOnlyCombination(recorded: string): boolean {
18+
const parts = recorded
19+
.split("+")
20+
.map((part) => part.trim().toLowerCase())
21+
.filter(Boolean);
22+
return parts.length === 0 || parts.every((part) => MODIFIER_KEYS.has(part));
23+
}

0 commit comments

Comments
 (0)