Skip to content
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.

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

fix(core): Fix keyboard shortcuts for non-ansi layouts #12672

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions packages/editor-ui/src/components/canvas/Canvas.vue
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,9 @@ const keyMap = computed(() => ({
ctrl_c: emitWithSelectedNodes((ids) => emit('copy:nodes', ids)),
enter: emitWithLastSelectedNode((id) => onSetNodeActive(id)),
ctrl_a: () => addSelectedNodes(graphNodes.value),
'shift_+|+|=': async () => await onZoomIn(),
'shift+_|-|_': async () => await onZoomOut(),
// Support both key and code for zooming in and out
'shift_+|+|=|shift_Equal|Equal': async () => await onZoomIn(),
'shift+_|-|_|shift_Minus|Minus': async () => await onZoomOut(),
0: async () => await onResetZoom(),
1: async () => await onFitView(),
ArrowUp: emitWithLastSelectedNode(selectUpperSiblingNode),
Expand Down
27 changes: 27 additions & 0 deletions packages/editor-ui/src/composables/useKeybindings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,31 @@ describe('useKeybindings', () => {
document.dispatchEvent(eventB);
expect(handler).toHaveBeenCalledTimes(2);
});

it("should prefer the 'key' over 'code' for dvorak to work correctly", () => {
const cHandler = vi.fn();
const iHandler = vi.fn();
const keymap = ref({
'ctrl+c': cHandler,
'ctrl+i': iHandler,
});

useKeybindings(keymap);

const event = new KeyboardEvent('keydown', { key: 'c', code: 'KeyI', ctrlKey: true });
document.dispatchEvent(event);
expect(cHandler).toHaveBeenCalled();
expect(iHandler).not.toHaveBeenCalled();
});

it("should fallback to 'code' for non-ansi layouts", () => {
const handler = vi.fn();
const keymap = ref({ 'ctrl+c': handler });

useKeybindings(keymap);

const event = new KeyboardEvent('keydown', { key: 'ב', code: 'KeyC', ctrlKey: true });
document.dispatchEvent(event);
expect(handler).toHaveBeenCalled();
});
});
63 changes: 54 additions & 9 deletions packages/editor-ui/src/composables/useKeybindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@ import { computed, unref } from 'vue';

type KeyMap = Record<string, (event: KeyboardEvent) => void>;

/**
* Binds a `keydown` event to `document` and calls the approriate
* handlers based on the given `keymap`. The keymap is a map from
* shortcut strings to handlers. The shortcut strings can contain
* multiple shortcuts separated by `|`.
*
* @example
* ```ts
* {
* 'ctrl+a': () => console.log('ctrl+a'),
* 'ctrl+b|ctrl+c': () => console.log('ctrl+b or ctrl+c'),
* }
* ```
*/
export const useKeybindings = (
keymap: Ref<KeyMap>,
options?: {
Expand All @@ -29,12 +43,10 @@ export const useKeybindings = (

const normalizedKeymap = computed(() =>
Object.fromEntries(
Object.entries(keymap.value)
.map(([shortcut, handler]) => {
const shortcuts = shortcut.split('|');
return shortcuts.map((s) => [normalizeShortcutString(s), handler]);
})
.flat(),
Object.entries(keymap.value).flatMap(([shortcut, handler]) => {
const shortcuts = shortcut.split('|');
return shortcuts.map((s) => [normalizeShortcutString(s), handler]);
}),
),
);

Expand Down Expand Up @@ -62,10 +74,36 @@ export const useKeybindings = (
return shortcutPartsToString(shortcut.split(new RegExp(`[${splitCharsRegEx}]`)));
}

/**
* Converts a keyboard event code to a key string.
*
* @example
* keyboardEventCodeToKey('Digit0') -> '0'
* keyboardEventCodeToKey('KeyA') -> 'a'
*/
function keyboardEventCodeToKey(code: string) {
if (code.startsWith('Digit')) {
return code.replace('Digit', '').toLowerCase();
} else if (code.startsWith('Key')) {
return code.replace('Key', '').toLowerCase();
}

return code.toLowerCase();
}

/**
* Converts a keyboard event to a shortcut string for both
* `key` and `code`.
*
* @example
* keyboardEventToShortcutString({ key: 'a', code: 'KeyA', ctrlKey: true })
* // --> { byKey: 'ctrl+a', byCode: 'ctrl+a' }
*/
function toShortcutString(event: KeyboardEvent) {
const { shiftKey, altKey } = event;
const ctrlKey = isCtrlKeyPressed(event);
const keys = [event.key];
const codes = [keyboardEventCodeToKey(event.code)];
const modifiers: string[] = [];

if (shiftKey) {
Expand All @@ -80,15 +118,22 @@ export const useKeybindings = (
modifiers.push('alt');
}

return shortcutPartsToString([...modifiers, ...keys]);
return {
byKey: shortcutPartsToString([...modifiers, ...keys]),
byCode: shortcutPartsToString([...modifiers, ...codes]),
};
}

function onKeyDown(event: KeyboardEvent) {
if (ignoreKeyPresses.value || isDisabled.value) return;

const shortcutString = toShortcutString(event);
const { byKey, byCode } = toShortcutString(event);

const handler = normalizedKeymap.value[shortcutString];
// Prefer `byKey` over `byCode` so that:
// - ANSI layouts work correctly
// - Dvorak works correctly
// - Non-ansi layouts work correctly
const handler = normalizedKeymap.value[byKey] ?? normalizedKeymap.value[byCode];

if (handler) {
event.preventDefault();
Expand Down
Loading