Skip to content

Commit 630e536

Browse files
authored
feat(ui): shortcut field on ui.commands.register (SD-2936) (#3149)
* feat(ui): shortcut field on ui.commands.register (SD-2936) * fix(ui): tighten shortcut parsing and dispatch (SD-2936)
1 parent 17505df commit 630e536

6 files changed

Lines changed: 486 additions & 1 deletion

File tree

packages/super-editor/src/ui/create-super-doc-ui.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type {
1616
} from '@superdoc/document-api';
1717
import { collectEntityHitsFromChain } from './entity-at.js';
1818
import { shallowEqual } from './equality.js';
19+
import { shortcutFromEvent } from './keyboard-shortcuts.js';
1920
import { scrollRangeIntoView } from './scroll-into-view.js';
2021
import { getSelectionAnchorRect, getSelectionRects } from './selection-rects.js';
2122
import { restoreSelection } from './selection-restore.js';
@@ -1021,6 +1022,60 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI {
10211022
customCommandsRegistry.destroy();
10221023
});
10231024

1025+
// Keyboard shortcut dispatch for custom commands registered with a
1026+
// `shortcut` field. Two important shapes:
1027+
//
1028+
// - Bubble phase. ProseMirror's keymap plugin is bubble-phase too
1029+
// and `eventBelongsToView` bails on `event.defaultPrevented`. A
1030+
// capture-phase listener that calls preventDefault would silently
1031+
// suppress every built-in editor keymap (Bold, Enter, Backspace),
1032+
// contradicting the documented "fires alongside built-ins"
1033+
// contract. Running at bubble lets the editor's own keymap
1034+
// process the event first; we dispatch the custom command after.
1035+
//
1036+
// - Scope expanded to the editor's hidden ProseMirror DOM in
1037+
// addition to the painted host. Once the user clicks the document,
1038+
// native focus moves to the hidden contenteditable that PM owns,
1039+
// which lives outside `visibleHost`. Filtering only on
1040+
// `host.contains(target)` would drop every keystroke from the
1041+
// normal editing path.
1042+
if (typeof globalThis !== 'undefined' && (globalThis as { document?: Document }).document) {
1043+
const dom = (globalThis as { document: Document }).document;
1044+
const onKeyDown = (event: Event) => {
1045+
const ke = event as KeyboardEvent;
1046+
// Re-resolve every event because the editor mount can happen
1047+
// after `createSuperDocUI` runs; caching a missing host at
1048+
// construction time would never recover.
1049+
const editor = resolveRoutedEditor(superdoc) as
1050+
| (SuperDocEditorLike & {
1051+
view?: { dom?: HTMLElement };
1052+
presentationEditor?: { visibleHost?: HTMLElement };
1053+
})
1054+
| null;
1055+
if (!editor) return;
1056+
const target = ke.target as Node | null;
1057+
if (!target) return;
1058+
const inHost = editor.presentationEditor?.visibleHost?.contains(target) === true;
1059+
const inPmDom = editor.view?.dom?.contains(target) === true;
1060+
if (!inHost && !inPmDom) return;
1061+
const combo = shortcutFromEvent(ke);
1062+
if (!combo) return;
1063+
const id = customCommandsRegistry.resolveShortcut(combo);
1064+
if (!id) return;
1065+
// Dispatch through the same path `ui.commands.get(id).execute()`
1066+
// uses. preventDefault runs AFTER dispatch so PM's keymap (which
1067+
// already ran in this bubble pass) isn't suppressed by an
1068+
// earlier defaultPrevented check; the call still blocks browser
1069+
// defaults that haven't run yet (the URL-bar shortcut, etc.).
1070+
customCommandsRegistry.execute(id);
1071+
ke.preventDefault();
1072+
};
1073+
dom.addEventListener('keydown', onKeyDown);
1074+
teardown.push(() => {
1075+
dom.removeEventListener('keydown', onKeyDown);
1076+
});
1077+
}
1078+
10241079
/**
10251080
* Single dispatch path for every `execute`-shaped surface on the
10261081
* controller (`ui.toolbar.execute(id)`, `ui.commands.bold.execute()`,

packages/super-editor/src/ui/custom-commands.test.ts

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,3 +1146,151 @@ describe('ui.commands.getContextMenuItems', () => {
11461146
ui.destroy();
11471147
});
11481148
});
1149+
1150+
describe('ui.commands.register — shortcut field', () => {
1151+
function makeStubsWithHost() {
1152+
const stubs = makeStubs();
1153+
const host = document.createElement('div');
1154+
document.body.appendChild(host);
1155+
// Preserve any existing presentationEditor surface the toolbar
1156+
// resolver expects (`getActiveEditor`) and add the `visibleHost`
1157+
// that the keydown listener scopes to.
1158+
const existing =
1159+
(stubs.editor as unknown as { presentationEditor?: Record<string, unknown> }).presentationEditor ?? {};
1160+
(stubs.editor as unknown as { presentationEditor: Record<string, unknown> }).presentationEditor = {
1161+
getActiveEditor: () => stubs.editor,
1162+
...existing,
1163+
visibleHost: host,
1164+
};
1165+
return { ...stubs, host };
1166+
}
1167+
1168+
function fireKey(target: Node, init: Partial<KeyboardEventInit> & { key: string }) {
1169+
const ev = new KeyboardEvent('keydown', { ...init, bubbles: true, cancelable: true });
1170+
target.dispatchEvent(ev);
1171+
return ev;
1172+
}
1173+
1174+
it('dispatches the registered command when the matching combo fires inside the host', () => {
1175+
const { superdoc, host } = makeStubsWithHost();
1176+
const ui = createSuperDocUI({ superdoc });
1177+
1178+
const execute = vi.fn(() => true);
1179+
ui.commands.register({ id: 'company.insertClause', execute, shortcut: 'Mod-Shift-C' });
1180+
1181+
fireKey(host, { key: 'c', ctrlKey: true, shiftKey: true });
1182+
1183+
expect(execute).toHaveBeenCalledTimes(1);
1184+
host.remove();
1185+
ui.destroy();
1186+
});
1187+
1188+
it("dispatches when focus is in the routed editor's hidden PM DOM (the normal editing path)", () => {
1189+
const { superdoc, editor, host } = makeStubsWithHost();
1190+
// Mount the hidden ProseMirror DOM directly under document.body
1191+
// (mirroring how PresentationEditor appends the hidden host outside
1192+
// the visible host) so a click-into-document keypress lands here.
1193+
const pmDom = document.createElement('div');
1194+
document.body.appendChild(pmDom);
1195+
(editor as unknown as { view: { dom: HTMLElement } }).view = { dom: pmDom };
1196+
const ui = createSuperDocUI({ superdoc });
1197+
1198+
const execute = vi.fn(() => true);
1199+
ui.commands.register({ id: 'company.action', execute, shortcut: 'Mod-K' });
1200+
1201+
fireKey(pmDom, { key: 'k', ctrlKey: true });
1202+
1203+
expect(execute).toHaveBeenCalledTimes(1);
1204+
pmDom.remove();
1205+
host.remove();
1206+
ui.destroy();
1207+
});
1208+
1209+
it('does not dispatch when focus is outside the painted host', () => {
1210+
const { superdoc, host } = makeStubsWithHost();
1211+
const ui = createSuperDocUI({ superdoc });
1212+
1213+
const execute = vi.fn(() => true);
1214+
ui.commands.register({ id: 'company.insertClause', execute, shortcut: 'Mod-Shift-C' });
1215+
1216+
const outside = document.createElement('input');
1217+
document.body.appendChild(outside);
1218+
fireKey(outside, { key: 'c', ctrlKey: true, shiftKey: true });
1219+
1220+
expect(execute).not.toHaveBeenCalled();
1221+
outside.remove();
1222+
host.remove();
1223+
ui.destroy();
1224+
});
1225+
1226+
it('warns and replaces when two registrations claim the same shortcut', () => {
1227+
const { superdoc, host } = makeStubsWithHost();
1228+
const ui = createSuperDocUI({ superdoc });
1229+
1230+
const firstExecute = vi.fn(() => true);
1231+
const secondExecute = vi.fn(() => true);
1232+
ui.commands.register({ id: 'company.first', execute: firstExecute, shortcut: 'Mod-K' });
1233+
ui.commands.register({ id: 'company.second', execute: secondExecute, shortcut: 'Mod-K' });
1234+
1235+
expect(warnSpy).toHaveBeenCalled();
1236+
1237+
fireKey(host, { key: 'k', ctrlKey: true });
1238+
expect(firstExecute).not.toHaveBeenCalled();
1239+
expect(secondExecute).toHaveBeenCalledTimes(1);
1240+
1241+
host.remove();
1242+
ui.destroy();
1243+
});
1244+
1245+
it('drops the shortcut on unregister so later keypresses are no-ops', () => {
1246+
const { superdoc, host } = makeStubsWithHost();
1247+
const ui = createSuperDocUI({ superdoc });
1248+
1249+
const execute = vi.fn(() => true);
1250+
const reg = ui.commands.register({ id: 'company.toggle', execute, shortcut: 'Mod-J' });
1251+
1252+
fireKey(host, { key: 'j', ctrlKey: true });
1253+
expect(execute).toHaveBeenCalledTimes(1);
1254+
1255+
reg.unregister();
1256+
fireKey(host, { key: 'j', ctrlKey: true });
1257+
expect(execute).toHaveBeenCalledTimes(1);
1258+
1259+
host.remove();
1260+
ui.destroy();
1261+
});
1262+
1263+
it('accepts a string[] for multiple shortcuts on the same command', () => {
1264+
const { superdoc, host } = makeStubsWithHost();
1265+
const ui = createSuperDocUI({ superdoc });
1266+
1267+
const execute = vi.fn(() => true);
1268+
ui.commands.register({
1269+
id: 'company.action',
1270+
execute,
1271+
shortcut: ['Mod-1', 'Mod-Shift-1'],
1272+
});
1273+
1274+
fireKey(host, { key: '1', ctrlKey: true });
1275+
fireKey(host, { key: '1', ctrlKey: true, shiftKey: true });
1276+
1277+
expect(execute).toHaveBeenCalledTimes(2);
1278+
host.remove();
1279+
ui.destroy();
1280+
});
1281+
1282+
it('warns on a malformed shortcut and ignores it', () => {
1283+
const { superdoc, host } = makeStubsWithHost();
1284+
const ui = createSuperDocUI({ superdoc });
1285+
1286+
const execute = vi.fn(() => true);
1287+
ui.commands.register({ id: 'company.bad', execute, shortcut: 'Mod-Shift' });
1288+
1289+
expect(warnSpy).toHaveBeenCalled();
1290+
fireKey(host, { key: 'Shift', ctrlKey: true });
1291+
expect(execute).not.toHaveBeenCalled();
1292+
1293+
host.remove();
1294+
ui.destroy();
1295+
});
1296+
});

packages/super-editor/src/ui/custom-commands.ts

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { normalizeShortcut } from './keyboard-shortcuts.js';
12
import type {
23
ContextMenuContribution,
34
ContextMenuItem,
@@ -13,6 +14,12 @@ import type {
1314
ViewportEntityHit,
1415
} from './types.js';
1516

17+
const DEFAULT_SHORTCUT_COLLISION_MESSAGE = (shortcut: string, oldId: string, newId: string) =>
18+
`[superdoc/ui] ui.commands.register(): shortcut '${shortcut}' was already bound to '${oldId}'. Replacing with '${newId}'.`;
19+
20+
const DEFAULT_INVALID_SHORTCUT_MESSAGE = (id: string, raw: string) =>
21+
`[superdoc/ui] ui.commands.register(): id '${id}' carries an invalid shortcut '${raw}' — ignored. Use a string like 'Mod-Shift-K'.`;
22+
1623
/**
1724
* Built-in group ids in the order they render in the context menu.
1825
* Custom groups land after these, ranked by the smallest registration
@@ -71,6 +78,12 @@ interface InternalCustomEntry {
7178
execute: CustomCommandRegistration['execute'];
7279
getState: CustomCommandRegistration['getState'];
7380
override: boolean;
81+
/**
82+
* Normalized shortcut strings claimed by this registration.
83+
* Tracked so unregister/replacement can drop them from the
84+
* shortcut → id index in one pass.
85+
*/
86+
shortcuts: string[];
7487
contextMenu: ContextMenuContribution | null;
7588
/**
7689
* Monotonic counter at registration time; ties in `(group, order)`
@@ -128,6 +141,14 @@ export interface CustomCommandsRegistry {
128141
*/
129142
getContextMenuItems(state: SuperDocUIState, entities: ViewportEntityHit[]): ContextMenuItem[];
130143

144+
/**
145+
* Look up the custom command id (if any) bound to a normalized
146+
* shortcut string. Used by the controller's keydown listener to
147+
* dispatch matched shortcuts. Returns `undefined` when nothing is
148+
* registered for that combo.
149+
*/
150+
resolveShortcut(shortcut: string): string | undefined;
151+
131152
/** Drop every registration and tear down per-command Subscribables. */
132153
destroy(): void;
133154
}
@@ -179,6 +200,37 @@ export function createCustomCommandsRegistry(deps: CustomCommandsRegistryDeps):
179200
// are broken by registration time. Stable across snapshots, doesn't
180201
// reuse values when entries are unregistered + re-registered.
181202
let nextRegistrationSeq = 0;
203+
// Normalized shortcut string → command id. Replacement / unregister
204+
// mutate this through `releaseShortcuts` / `claimShortcuts` so a
205+
// stale registration's shortcuts can never dispatch into a removed
206+
// entry.
207+
const shortcutIndex = new Map<string, string>();
208+
209+
const releaseShortcuts = (entry: InternalCustomEntry) => {
210+
for (const sc of entry.shortcuts) {
211+
if (shortcutIndex.get(sc) === entry.id) shortcutIndex.delete(sc);
212+
}
213+
};
214+
215+
const claimShortcuts = (id: string, raw: string | string[] | undefined): string[] => {
216+
if (raw === undefined) return [];
217+
const list = Array.isArray(raw) ? raw : [raw];
218+
const claimed: string[] = [];
219+
for (const item of list) {
220+
const normalized = normalizeShortcut(item);
221+
if (!normalized) {
222+
console.warn(DEFAULT_INVALID_SHORTCUT_MESSAGE(id, item));
223+
continue;
224+
}
225+
const prior = shortcutIndex.get(normalized);
226+
if (prior && prior !== id) {
227+
console.warn(DEFAULT_SHORTCUT_COLLISION_MESSAGE(normalized, prior, id));
228+
}
229+
shortcutIndex.set(normalized, id);
230+
claimed.push(normalized);
231+
}
232+
return claimed;
233+
};
182234
// Active observer disposers per command id. Lets `unregister` (and
183235
// replacement) actively tear down inner subscriptions instead of
184236
// waiting for the observer wrapper's lazy `!entries.has(id)` check
@@ -335,9 +387,14 @@ export function createCustomCommandsRegistry(deps: CustomCommandsRegistryDeps):
335387
// Existing observers attached to the prior registration must be
336388
// told their command is gone before we install the new one — the
337389
// observer's `entries.has(id)` short-circuit will then detach.
338-
if (entries.has(id)) {
390+
const priorEntry = entries.get(id);
391+
if (priorEntry) {
339392
console.warn(DEFAULT_REPLACEMENT_MESSAGE(id));
340393
disposeAllObservers(id);
394+
// Drop the prior registration's shortcuts before claiming the
395+
// new ones so a re-registration that drops a binding doesn't
396+
// leave a stale shortcut → id mapping.
397+
releaseShortcuts(priorEntry);
341398
}
342399

343400
// Capture the entry by reference so this registration's
@@ -351,6 +408,7 @@ export function createCustomCommandsRegistry(deps: CustomCommandsRegistryDeps):
351408
execute: execute as InternalCustomEntry['execute'],
352409
getState: getState as InternalCustomEntry['getState'],
353410
override,
411+
shortcuts: claimShortcuts(id, registration.shortcut),
354412
contextMenu: registration.contextMenu ?? null,
355413
registrationSeq: nextRegistrationSeq++,
356414
lastErrorMessage: null,
@@ -387,6 +445,7 @@ export function createCustomCommandsRegistry(deps: CustomCommandsRegistryDeps):
387445
entries.delete(id);
388446
handleCache.delete(id);
389447
subscribableCache.delete(id);
448+
releaseShortcuts(ownEntry);
390449
// Actively detach every active observer for this id so they
391450
// stop holding the inner Subscribable. The observer wrapper's
392451
// lazy `!entries.has(id)` check would otherwise leave the
@@ -552,6 +611,10 @@ export function createCustomCommandsRegistry(deps: CustomCommandsRegistryDeps):
552611
return items;
553612
},
554613

614+
resolveShortcut(shortcut) {
615+
return shortcutIndex.get(shortcut);
616+
},
617+
555618
destroy() {
556619
// Dispose every active observer before clearing maps so the
557620
// inner Subscribables release their selector subscriptions; just
@@ -561,6 +624,7 @@ export function createCustomCommandsRegistry(deps: CustomCommandsRegistryDeps):
561624
entries.clear();
562625
handleCache.clear();
563626
subscribableCache.clear();
627+
shortcutIndex.clear();
564628
},
565629
};
566630

0 commit comments

Comments
 (0)