diff --git a/specifyweb/frontend/js_src/lib/components/Keyboard/__tests__/UserDefinitions.test.ts b/specifyweb/frontend/js_src/lib/components/Keyboard/__tests__/UserDefinitions.test.ts index 19d10f05283..a83c1a6d5bd 100644 --- a/specifyweb/frontend/js_src/lib/components/Keyboard/__tests__/UserDefinitions.test.ts +++ b/specifyweb/frontend/js_src/lib/components/Keyboard/__tests__/UserDefinitions.test.ts @@ -1,9 +1,10 @@ -import type { KeyboardShortcuts, ModifierKey } from '../context'; -import { exportsForTests, keyJoinSymbol } from '../context'; import type { GenericPreferences } from '../../Preferences/types'; import { userPreferenceDefinitions } from '../../Preferences/UserDefinitions'; +import type { KeyboardShortcuts, ModifierKey } from '../config'; +import { keyLocalizations, modifierKeys, specialKeyboardKeys } from '../config'; +import { exportsForTests, keyJoinSymbol } from '../context'; -const { keysToString, modifierKeys, specialKeys } = exportsForTests; +const { keysToString } = exportsForTests; test('Validate default keyboard shortcuts in userPreferenceDefinitions', () => { Object.entries(userPreferenceDefinitions as GenericPreferences).forEach( @@ -51,9 +52,6 @@ function validateShortcut( const parts = shortcut.split(keyJoinSymbol); if (parts.length === 0) return 'unexpected empty shortcut'; - /* - * FIXME: add test against default preferences containing non-existing shortcuts - */ const shortcutModifierKeys = parts .map((part) => part as ModifierKey) .filter((part) => modifierKeys.includes(part)) @@ -69,7 +67,9 @@ function validateShortcut( return `shortcut is not normalized: ${normalizedShortcut} (expected keys to be sorted, with modifier keys before non-modifiers)`; if (shortcutModifierKeys.length === 0) { - const specialKey = nonModifierKeys.find((key) => specialKeys.has(key)); + const specialKey = nonModifierKeys.find((key) => + specialKeyboardKeys.has(key) + ); if (typeof specialKey === 'string') return `can't use special reserved keys as default shortcuts, unless prefixed with a modifier key. Found ${specialKey}`; } @@ -80,6 +80,10 @@ function validateShortcut( if (nonModifierKeys.length === 0) return 'shortcut must contain at least one non-modifier key'; + const disallowedKey = nonModifierKeys.find((key) => !allowed.has(key)); + if (typeof disallowedKey === 'string') + return `found an unknown key code: ${disallowedKey}. Make sure you are using key code rather than key name (e.g. use "KeyA" instead of "a"). To test, see what "keydown" event gives you for event.code (rather than event.key). If you are sure you have the key code, then you can extend the list of allowed keys in this test`; + return undefined; } @@ -93,41 +97,18 @@ function validateShortcut( */ const allowed = new Set([ // Not including modifier keys as the above check will only check non-modifiers - ...Array.from(specialKeys), - 'ArrowDown', - 'ArrowLeft', - 'ArrowRight', - 'ArrowUp', - 'End', - 'Home', - 'PageDown', - 'PageUp', - 'Insert', - 'F1', - 'F2', - 'F3', - 'F4', - 'F5', - 'F6', - 'F7', - 'F8', - 'F9', - 'F10', - 'F11', - 'F12', + ...Array.from(specialKeyboardKeys), + ...Object.keys(keyLocalizations), 'BrowserBack', 'BrowserForward', 'BrowserHome', 'BrowserRefresh', - 'BrowserrSearch', + 'BrowserSearch', 'Add', 'Multiply', 'Subtract', 'Decimal', 'Divide', - ...'01234567890-=qwertyuiop[]asdfghjkl;\'zxcvbnm,./\\`~!@#$%^&*()_+QWERTYUIOP{}|ASDFGHJKL:"ZXCVBNM<>?'.split( - '' - ), /* * This list doesn't include more obscure keys, but we probably shouldn't be * using those in key shortcuts anyway diff --git a/specifyweb/frontend/js_src/lib/components/Keyboard/config.ts b/specifyweb/frontend/js_src/lib/components/Keyboard/config.ts index 5799341bf94..98e3b8556d2 100644 --- a/specifyweb/frontend/js_src/lib/components/Keyboard/config.ts +++ b/specifyweb/frontend/js_src/lib/components/Keyboard/config.ts @@ -1,5 +1,5 @@ import { preferencesText } from '../../localization/preferences'; -import type { RA, RR } from '../../utils/types'; +import type { IR, RA, RR } from '../../utils/types'; /** * Because operating systems, browsers and browser extensions define many @@ -29,12 +29,18 @@ export const keyboardPlatform: KeyboardPlatform = ? 'windows' : 'other'; -const modifierKeys = ['Alt', 'Ctrl', 'Meta', 'Shift'] as const; +export const modifierKeys = ['Alt', 'Ctrl', 'Meta', 'Shift'] as const; export type ModifierKey = typeof modifierKeys[number]; export const allModifierKeys = new Set([ ...modifierKeys, 'AltGraph', 'CapsLock', + 'MetaLeft', + 'MetaRight', + 'ShiftLeft', + 'ShiftRight', + 'AltLeft', + 'AltRight', ]); export const keyboardModifierLocalization: RR = { @@ -66,7 +72,78 @@ export const keyboardModifierLocalization: RR = { export const specialKeyboardKeys = new Set([ 'Enter', 'Tab', - ' ', + 'Space', 'Escape', 'Backspace', ]); + +/** + * Because we are listening to key codes that correspond to US English letters, + * we should show keys in the UI in US English to avoid confusion. + * (otherwise Cmd+O is ambiguous as it's not clear if it refers to English O or + * local language О). + * See https://github.com/specify/specify7/issues/1746#issuecomment-2227113839 + * + * For some keys, it is less confusing to see a symbol (like arrow keys), rather + * than 'ArrowUp', thus symbols are used for those keys. + * See http://xahlee.info/comp/unicode_computing_symbols.html + * + * Try not to define keyboard shortcuts for keys that may be in a different + * place in other keyboard layouts (the positions of special symbols wary a lot) + */ +export const keyLocalizations: IR = { + ArrowDown: '↓', + ArrowLeft: '←', + ArrowRight: '→', + ArrowUp: '↑', + Backquote: '`', + Backslash: '\\', + Backspace: '⌫', + BracketLeft: '[', + BracketRight: ']', + Comma: ',', + Digit0: '0', + Digit1: '1', + Digit2: '2', + Digit3: '3', + Digit4: '4', + Digit5: '5', + Digit6: '6', + Digit7: '7', + Digit8: '8', + Digit9: '9', + // "return" key is used over Enter on macOS keyboards + Enter: keyboardPlatform === 'mac' ? '↵' : 'Enter', + Equal: '=', + KeyA: 'a', + KeyB: 'b', + KeyC: 'c', + KeyD: 'd', + KeyE: 'e', + KeyF: 'f', + KeyG: 'g', + KeyH: 'h', + KeyI: 'i', + KeyJ: 'j', + KeyK: 'k', + KeyL: 'l', + KeyM: 'm', + KeyN: 'n', + KeyO: 'o', + KeyP: 'p', + KeyQ: 'q', + KeyR: 'r', + KeyS: 's', + KeyT: 't', + KeyU: 'u', + KeyV: 'v', + KeyW: 'w', + KeyX: 'x', + KeyY: 'y', + KeyZ: 'z', + Minus: '-', + Period: '.', + Quote: "'", + Semicolon: ';', + Slash: '/', +}; diff --git a/specifyweb/frontend/js_src/lib/components/Keyboard/context.tsx b/specifyweb/frontend/js_src/lib/components/Keyboard/context.tsx index 2501aef8a95..e3705ff51d2 100644 --- a/specifyweb/frontend/js_src/lib/components/Keyboard/context.tsx +++ b/specifyweb/frontend/js_src/lib/components/Keyboard/context.tsx @@ -62,8 +62,8 @@ const pressedKeys: string[] = []; document.addEventListener('keydown', (event) => { if (shouldIgnoreKeyPress(event)) return; - if (!pressedKeys.includes(event.key)) { - pressedKeys.push(event.key); + if (!pressedKeys.includes(event.code)) { + pressedKeys.push(event.code); pressedKeys.sort(); } @@ -73,7 +73,7 @@ document.addEventListener('keydown', (event) => { // Ignore shortcuts that result in printed characters when in an input field const ignore = isPrintable && isEntering; if (ignore) return; - if (modifiers.length === 0 && specialKeyboardKeys.has(event.key)) return; + if (modifiers.length === 0 && specialKeyboardKeys.has(event.code)) return; const keyString = keysToString(modifiers, pressedKeys); const handler = interceptor ?? listeners.get(keyString); @@ -96,7 +96,7 @@ function shouldIgnoreKeyPress(event: KeyboardEvent): boolean { // See https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key#value if (key === 'Dead' || key === 'Unidentified') return true; - // Do not allow binding a key shortcut to a modifier key only + // Do not allow binding a key shortcut directly to a modifier key const isModifier = allModifierKeys.has(event.key); return !isModifier; diff --git a/specifyweb/frontend/js_src/lib/components/Keyboard/shortcuts.tsx b/specifyweb/frontend/js_src/lib/components/Keyboard/shortcuts.tsx index 887d9b0f552..abb47bfb107 100644 --- a/specifyweb/frontend/js_src/lib/components/Keyboard/shortcuts.tsx +++ b/specifyweb/frontend/js_src/lib/components/Keyboard/shortcuts.tsx @@ -27,10 +27,6 @@ import { localizeKeyboardShortcut, resolvePlatformShortcuts } from './utils'; * those in the UI if present on the page * * FIXME: open key shortcut viewer on cmd+/ - * - * FIXME: localize some key shortcuts (arrow keys, home, etc) - * - * FIXME: add a mapping of allowed default keyboard shortcuts and use that in tests */ export function KeyboardShortcutPreferenceItem({ diff --git a/specifyweb/frontend/js_src/lib/components/Keyboard/utils.ts b/specifyweb/frontend/js_src/lib/components/Keyboard/utils.ts index 5de0fbdb7f5..e79d1370f86 100644 --- a/specifyweb/frontend/js_src/lib/components/Keyboard/utils.ts +++ b/specifyweb/frontend/js_src/lib/components/Keyboard/utils.ts @@ -3,7 +3,11 @@ import type { LocalizedString } from 'typesafe-i18n'; import type { RA } from '../../utils/types'; import { localized } from '../../utils/types'; import type { KeyboardShortcuts, ModifierKey } from './config'; -import { keyboardModifierLocalization, keyboardPlatform } from './config'; +import { + keyboardModifierLocalization, + keyboardPlatform, + keyLocalizations, +} from './config'; import { keyJoinSymbol } from './context'; const localizedKeyJoinSymbol = ' + '; @@ -11,7 +15,12 @@ export const localizeKeyboardShortcut = (shortcut: string): LocalizedString => localized( shortcut .split(keyJoinSymbol) - .map((key) => keyboardModifierLocalization[key as ModifierKey] ?? key) + .map( + (key) => + keyboardModifierLocalization[key as ModifierKey] ?? + keyLocalizations[key] ?? + key + ) .join(localizedKeyJoinSymbol) ); @@ -41,11 +50,11 @@ export function resolvePlatformShortcuts( const replaceCtrlWithMeta = (shortcut: string): string => shortcut .split(keyJoinSymbol) - .map((key) => (key === 'ctrl' ? 'meta' : key)) + .map((key) => (key === 'Ctrl' ? 'Meta' : key)) .join(keyJoinSymbol); const replaceMetaWithCtrl = (shortcut: string): string => shortcut .split(keyJoinSymbol) - .map((key) => (key === 'meta' ? 'ctrl' : key)) + .map((key) => (key === 'Meta' ? 'Ctrl' : key)) .join(keyJoinSymbol); diff --git a/specifyweb/frontend/js_src/lib/components/Preferences/BasePreferences.tsx b/specifyweb/frontend/js_src/lib/components/Preferences/BasePreferences.tsx index e08bca63b92..8075806cdad 100644 --- a/specifyweb/frontend/js_src/lib/components/Preferences/BasePreferences.tsx +++ b/specifyweb/frontend/js_src/lib/components/Preferences/BasePreferences.tsx @@ -18,11 +18,11 @@ import { contextUnlockedPromise, foreverFetch, } from '../InitialContext'; +import { bindKeyboardShortcut } from '../Keyboard/context'; import { - bindKeyboardShortcut, + localizeKeyboardShortcut, resolvePlatformShortcuts, -} from '../Keyboard/context'; -import { localizeKeyboardShortcut } from '../Keyboard/shortcuts'; +} from '../Keyboard/utils'; import { formatUrl } from '../Router/queryString'; import type { GenericPreferences, PreferenceItem } from './types'; diff --git a/specifyweb/frontend/js_src/lib/components/Preferences/UserDefinitions.tsx b/specifyweb/frontend/js_src/lib/components/Preferences/UserDefinitions.tsx index 5fc49bf2461..71aa7b1f751 100644 --- a/specifyweb/frontend/js_src/lib/components/Preferences/UserDefinitions.tsx +++ b/specifyweb/frontend/js_src/lib/components/Preferences/UserDefinitions.tsx @@ -37,7 +37,7 @@ import type { TableFields } from '../DataModel/helperTypes'; import { genericTables } from '../DataModel/tables'; import type { Collection, Tables } from '../DataModel/types'; import { error, softError } from '../Errors/assert'; -import type { KeyboardShortcuts } from '../Keyboard/context'; +import type { KeyboardShortcuts } from '../Keyboard/config'; import { KeyboardShortcutPreferenceItem } from '../Keyboard/shortcuts'; import type { StatLayout } from '../Statistics/types'; import { @@ -82,6 +82,14 @@ const tableLabel = (tableName: keyof Tables): LocalizedString => const defineKeyboardShortcut = ( title: LocalizedString, + /** + * If defined a keyboard shortcut for one platform, it will be automatically + * transformed (`ctrl -> cmd`) for the other platforms. + * + * Thus, you should define keyboard shortcuts for the "other" platform only, + * unless you actually want to use different keyboard shortcuts on different + * systems. + */ defaultValue: KeyboardShortcuts | string ): PreferenceItem => definePref({ diff --git a/specifyweb/frontend/js_src/lib/utils/types.ts b/specifyweb/frontend/js_src/lib/utils/types.ts index 90cef29f30b..d13540b53e5 100644 --- a/specifyweb/frontend/js_src/lib/utils/types.ts +++ b/specifyweb/frontend/js_src/lib/utils/types.ts @@ -215,11 +215,13 @@ export const ensure = * empty string is used. However, empty string is not a LocalizedString, so * this hack is needed * - * @example Valid use case + * @example + * // Valid use case * localized('') * - * @example Invalid use case + * @example * ```ts + * // Invalid use case * // Wrong: * localized(table.name) * // Should use table label instead (unless there is a good reason to use