diff --git a/main/keyboardShortcuts/index.ts b/main/keyboardShortcuts/index.ts index 8118529d9..72018dd14 100644 --- a/main/keyboardShortcuts/index.ts +++ b/main/keyboardShortcuts/index.ts @@ -1,7 +1,6 @@ import { globalShortcut } from 'electron' import log from 'electron-log' -import store from '../store' import { shortcutKeyMap } from '../../resources/keyboard/mappings' import type { Shortcut } from '../store/state/types/shortcuts' @@ -10,9 +9,6 @@ const stringifyShortcut = ({ modifierKeys, shortcutKey }: Shortcut) => ({ accelerator: [...modifierKeys.slice().sort(), shortcutKeyMap[shortcutKey] || shortcutKey].join('+') }) -const equivalentShortcuts = (shortcut1: Shortcut, shortcut2: Shortcut) => - shortcut1.modifierKeys === shortcut2.modifierKeys && shortcut1.shortcutKey === shortcut2.shortcutKey - function unregister(shortcut: Shortcut) { const { shortcutString, accelerator } = stringifyShortcut(shortcut) @@ -41,50 +37,6 @@ function register(shortcut: Shortcut, shortcutHandler: (accelerator: string) => } export const registerShortcut = (shortcut: Shortcut, shortcutHandler: (accelerator: string) => void) => { - const isWindows = process.platform === 'win32' - const isMacOS = process.platform === 'darwin' - const keyboardLayout = store('keyboardLayout') - const createAltGrShortcut = () => { - // remove AltGr and Alt from modifiers (Linux) - // remove AltGr, Alt and Control from modifiers (Windows) - const modifierKeys = shortcut.modifierKeys.filter((modifier) => - isWindows ? !modifier.startsWith('Alt') && modifier !== 'Control' : !modifier.startsWith('Alt') - ) - - // return new modifiers depending on OS + rest of shortcut - so that AltGr / Right Alt triggers in the same way as Left Alt - return { - ...shortcut, - modifierKeys: (isWindows - ? [...modifierKeys, 'Control', 'Alt'] - : [...modifierKeys, 'AltGr']) as typeof shortcut.modifierKeys - } - } - - // Windows & Linux Non-US key layout AltGr / Right Alt fix - if (!isMacOS) { - const altGrShortcut = createAltGrShortcut() - - // unregister any existing AltGr shortcut - unless it matches the one we are about to register - if (!keyboardLayout.isUS && !equivalentShortcuts(shortcut, altGrShortcut)) { - unregister(altGrShortcut) - } - - if ( - shortcut.modifierKeys.includes('AltGr') || - (shortcut.modifierKeys.includes('Alt') && !keyboardLayout.isUS) - ) { - // register the AltGr shortcut - register(altGrShortcut, shortcutHandler) - - // replace AltGr with Alt in the main shortcut - shortcut = { - ...shortcut, - modifierKeys: shortcut.modifierKeys.map((key) => (key === 'AltGr' ? 'Alt' : key)) - } - } - } - - // register the shortcut unregister(shortcut) register(shortcut, shortcutHandler) } diff --git a/main/store/migrations/index.js b/main/store/migrations/index.js index b59610501..b7ae4e6b3 100644 --- a/main/store/migrations/index.js +++ b/main/store/migrations/index.js @@ -927,6 +927,17 @@ const migrations = { initial.main.shortcuts.summon.enabled = true } + return initial + }, + 37: (initial) => { + const isWindows = process.platform === 'win32' + const { shortcuts } = initial.main || {} + const altGrIndex = shortcuts.summon.modifierKeys.indexOf('AltGr') + if (altGrIndex > -1) { + const altGrReplacement = isWindows ? ['Alt', 'Control'] : ['Alt'] + initial.main.shortcuts.summon.modifierKeys.splice(altGrIndex, 1, ...altGrReplacement) + } + return initial } } diff --git a/main/store/state/types/shortcuts.ts b/main/store/state/types/shortcuts.ts index 9fbeacf2a..35dc95b09 100644 --- a/main/store/state/types/shortcuts.ts +++ b/main/store/state/types/shortcuts.ts @@ -1,6 +1,6 @@ import { z } from 'zod' -const supportedModifierKey = z.enum(['Alt', 'AltGr', 'Control', 'Meta', 'Super', 'CommandOrCtrl']) +const supportedModifierKey = z.enum(['Alt', 'Control', 'Meta', 'Super', 'CommandOrCtrl']) const supportedShortcutKey = z.enum([ 'Comma', diff --git a/resources/Components/KeyboardShortcutConfigurator/index.js b/resources/Components/KeyboardShortcutConfigurator/index.js index afe2b1a85..5efb882b1 100644 --- a/resources/Components/KeyboardShortcutConfigurator/index.js +++ b/resources/Components/KeyboardShortcutConfigurator/index.js @@ -12,7 +12,7 @@ const KeyboardShortcutConfigurator = ({ actionText = '', platform, shortcut, sho hotkeys('*', { capture: true }, (event) => { event.preventDefault() - const allowedModifierKeys = ['Meta', 'Alt', 'AltGr', 'Control', 'Command'] + const allowedModifierKeys = ['Meta', 'Alt', 'Control', 'Command'] const isModifierKey = allowedModifierKeys.includes(event.key) // ignore modifier key solo keypresses and disabled keys diff --git a/resources/keyboard/index.ts b/resources/keyboard/index.ts index dc3994fe2..93dae70c4 100644 --- a/resources/keyboard/index.ts +++ b/resources/keyboard/index.ts @@ -31,10 +31,6 @@ function getModifierKey(key: ModifierKey, platform: Platform) { return isMacOS ? 'Option' : 'Alt' } - if (key === 'AltGr') { - return 'Alt' - } - if (key === 'Control' || key === 'CommandOrCtrl') { return isMacOS ? 'Control' : 'Ctrl' } @@ -60,13 +56,11 @@ export const getDisplayShortcut = (platform: Platform, shortcut: Shortcut) => { export const getShortcutFromKeyEvent = (e: KeyboardEvent, pressedKeyCodes: number[], platform: Platform) => { const isWindows = platform === 'win32' - const isLinux = platform === 'linux' - const altGrPressed = !e.altKey && - ((pressedKeyCodes.includes(17) && pressedKeyCodes.includes(18) && isWindows) || (pressedKeyCodes.includes(18) && isLinux)) + const altGrPressed = !e.altKey && pressedKeyCodes.includes(17) && pressedKeyCodes.includes(18) const modifierKeys = [] - if (altGrPressed) { - modifierKeys.push('AltGr') + if (isWindows && altGrPressed) { + modifierKeys.push('Alt', 'Control') } if (e.altKey) { modifierKeys.push('Alt') diff --git a/test/main/keyboardShortcuts/index.test.js b/test/main/keyboardShortcuts/index.test.js index 50fb435e6..b155c8a3c 100644 --- a/test/main/keyboardShortcuts/index.test.js +++ b/test/main/keyboardShortcuts/index.test.js @@ -1,26 +1,9 @@ import { globalShortcut } from 'electron' -import store from '../../../main/store' jest.mock('electron', () => ({ app: { on: jest.fn(), getName: jest.fn(), getVersion: jest.fn(), getPath: jest.fn() }, globalShortcut: { register: jest.fn(), unregister: jest.fn() } })) -jest.mock('../../../main/store/state', () => () => ({ keyboardLayout: { isUS: true } })) -jest.mock('../../../main/accounts', () => ({ updatePendingFees: jest.fn() })) -jest.mock('../../../main/store/persist') - -async function withPlatform(platform, test) { - const originalPlatform = process.platform - Object.defineProperty(process, 'platform', { - value: platform - }) - - await test() - - Object.defineProperty(process, 'platform', { - value: originalPlatform - }) -} let registerShortcut @@ -37,261 +20,25 @@ describe('registerShortcut', () => { registerShortcut = keyboardShortcuts.registerShortcut }) - describe('on MacOS', () => { - it('should unregister an existing shortcut', () => { - withPlatform('darwin', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.unregister).toHaveBeenCalledWith('Alt+/') - expect(globalShortcut.unregister).toHaveBeenCalledTimes(1) - }) - }) - - it('should register the new shortcut', () => { - withPlatform('darwin', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.register).toHaveBeenCalledWith('Alt+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledTimes(1) - }) - }) - }) - - describe('on Windows', () => { - describe('when registering a shortcut without an Alt modifier and a US keyboard layout', () => { - beforeEach(() => { - shortcut.modifierKeys = ['Control'] - store.setKeyboardLayout({ isUS: true }) - }) - - it('should unregister the requested shortcut', () => { - withPlatform('win32', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.unregister).toHaveBeenCalledWith('Control+/') - expect(globalShortcut.unregister).toHaveBeenCalledTimes(1) - }) - }) - - it('should register the requested shortcut', () => { - withPlatform('win32', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.register).toHaveBeenCalledWith('Control+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledTimes(1) - }) - }) - }) - - describe('when registering a shortcut without an Alt modifier and a Non-US keyboard layout', () => { - beforeEach(() => { - shortcut.modifierKeys = ['Control'] - store.setKeyboardLayout({ isUS: false }) - }) - - it('should unregister the requested shortcut and an AltGr shortcut', () => { - withPlatform('win32', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.unregister).toHaveBeenCalledWith('Control+/') - expect(globalShortcut.unregister).toHaveBeenCalledWith('Alt+Control+/') - expect(globalShortcut.unregister).toHaveBeenCalledTimes(2) - }) - }) - - it('should register the requested shortcut', () => { - withPlatform('win32', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.register).toHaveBeenCalledWith('Control+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledTimes(1) - }) - }) - }) - - describe('when registering a shortcut with an AltGr modifier', () => { - beforeEach(() => { - shortcut.modifierKeys = ['AltGr'] - store.setKeyboardLayout({ isUS: false }) - }) - - it('should unregister the equivalent Alt-based shortcut and an AltGr shortcut', () => { - withPlatform('win32', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.unregister).toHaveBeenCalledWith('Alt+/') - expect(globalShortcut.unregister).toHaveBeenCalledWith('Alt+Control+/') - expect(globalShortcut.unregister).toHaveBeenCalledTimes(2) - }) - }) - - it('should register the equivalent Alt-based shortcut and an AltGr shortcut', () => { - withPlatform('win32', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.register).toHaveBeenCalledWith('Alt+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledWith('Alt+Control+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledTimes(2) - }) - }) - }) - - describe('when registering a shortcut with an Alt modifier and a US keyboard layout', () => { - beforeEach(() => { - shortcut.modifierKeys = ['Alt'] - store.setKeyboardLayout({ isUS: true }) - }) + it('should unregister an existing shortcut', () => { + registerShortcut(shortcut, () => {}) - it('should unregister the requested shortcut', () => { - withPlatform('win32', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.unregister).toHaveBeenCalledWith('Alt+/') - expect(globalShortcut.unregister).toHaveBeenCalledTimes(1) - }) - }) - - it('should register the requested shortcut', () => { - withPlatform('win32', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.register).toHaveBeenCalledWith('Alt+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledTimes(1) - }) - }) - }) - - describe('when registering a shortcut with an Alt modifier and a Non-US keyboard layout', () => { - beforeEach(() => { - shortcut.modifierKeys = ['Alt'] - store.setKeyboardLayout({ isUS: false }) - }) - - it('should unregister the equivalent Alt-based shortcut and an AltGr shortcut', () => { - withPlatform('win32', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.unregister).toHaveBeenCalledWith('Alt+/') - expect(globalShortcut.unregister).toHaveBeenCalledWith('Alt+Control+/') - expect(globalShortcut.unregister).toHaveBeenCalledTimes(2) - }) - }) - - it('should register the equivalent Alt-based shortcut and an AltGr shortcut', () => { - withPlatform('win32', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.register).toHaveBeenCalledWith('Alt+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledWith('Alt+Control+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledTimes(2) - }) - }) - }) + expect(globalShortcut.unregister).toHaveBeenCalledWith('Alt+/') + expect(globalShortcut.unregister).toHaveBeenCalledTimes(1) }) - describe('on Linux', () => { - describe('when registering a shortcut without an Alt modifier', () => { - beforeEach(() => { - shortcut.modifierKeys = ['Control'] - }) - - it('should unregister the requested shortcut and an AltGr shortcut', () => { - withPlatform('linux', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.unregister).toHaveBeenCalledWith('Control+/') - expect(globalShortcut.unregister).toHaveBeenCalledWith('AltGr+Control+/') - expect(globalShortcut.unregister).toHaveBeenCalledTimes(2) - }) - }) - - it('should register the requested shortcut', () => { - withPlatform('linux', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.register).toHaveBeenCalledWith('Control+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledTimes(1) - }) - }) - }) - - describe('when registering a shortcut with an AltGr modifier', () => { - beforeEach(() => { - shortcut.modifierKeys = ['AltGr'] - store.setKeyboardLayout({ isUS: false }) - }) - - it('should unregister the equivalent Alt-based shortcut and an AltGr shortcut', () => { - withPlatform('linux', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.unregister).toHaveBeenCalledWith('Alt+/') - expect(globalShortcut.unregister).toHaveBeenCalledWith('AltGr+/') - expect(globalShortcut.unregister).toHaveBeenCalledTimes(2) - }) - }) - - it('should register the equivalent Alt-based shortcut and an AltGr shortcut', () => { - withPlatform('linux', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.register).toHaveBeenCalledWith('Alt+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledWith('AltGr+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledTimes(2) - }) - }) - }) - - describe('when registering a shortcut with an Alt modifier and a US keyboard layout', () => { - beforeEach(() => { - shortcut.modifierKeys = ['Alt'] - store.setKeyboardLayout({ isUS: true }) - }) - - it('should unregister the requested shortcut', () => { - withPlatform('linux', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.unregister).toHaveBeenCalledWith('Alt+/') - expect(globalShortcut.unregister).toHaveBeenCalledTimes(1) - }) - }) - - it('should register the requested shortcut', () => { - withPlatform('linux', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.register).toHaveBeenCalledWith('Alt+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledTimes(1) - }) - }) - }) - - describe('when registering a shortcut with an Alt modifier and a Non-US keyboard layout', () => { - beforeEach(() => { - shortcut.modifierKeys = ['Alt'] - store.setKeyboardLayout({ isUS: false }) - }) - - it('should unregister the equivalent Alt-based shortcut and an AltGr shortcut', () => { - withPlatform('linux', () => { - registerShortcut(shortcut, () => {}) - - expect(globalShortcut.unregister).toHaveBeenCalledWith('Alt+/') - expect(globalShortcut.unregister).toHaveBeenCalledWith('AltGr+/') - expect(globalShortcut.unregister).toHaveBeenCalledTimes(2) - }) - }) + it('should register the new shortcut', () => { + globalShortcut.register.mockImplementationOnce((accelerator, handlerFn) => handlerFn(accelerator)) - it('should register the equivalent Alt-based shortcut and an AltGr shortcut', () => { - withPlatform('linux', () => { - registerShortcut(shortcut, () => {}) + return new Promise((resolve) => { + const handlerFn = (accelerator) => { + expect(accelerator).toBe('Alt+/') + resolve() + } + registerShortcut(shortcut, handlerFn) - expect(globalShortcut.register).toHaveBeenCalledWith('Alt+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledWith('AltGr+/', expect.any(Function)) - expect(globalShortcut.register).toHaveBeenCalledTimes(2) - }) - }) + expect(globalShortcut.register).toHaveBeenCalledWith('Alt+/', expect.any(Function)) + expect(globalShortcut.register).toHaveBeenCalledTimes(1) }) }) }) diff --git a/test/main/store/migrations/index.test.js b/test/main/store/migrations/index.test.js index a7c4ee186..d4a5c2605 100644 --- a/test/main/store/migrations/index.test.js +++ b/test/main/store/migrations/index.test.js @@ -3,6 +3,7 @@ import log from 'electron-log' import migrations from '../../../../main/store/migrations' import { getDefaultAccountName } from '../../../../resources/domain/account' import { capitalize } from '../../../../resources/utils' +import { withPlatform } from '../../../util' let state @@ -1475,3 +1476,89 @@ describe('migration 35', () => { }) }) }) + +describe('migration 37', () => { + beforeEach(() => { + state.main._version = 36 + }) + + describe('when altGr is a modifier', () => { + beforeEach(() => { + state.main.shortcuts = { + summon: { modifierKeys: ['AltGr'], shortcutKey: 'Slash', enabled: true, configuring: false } + } + }) + + it('should update the summon shortcut on Linux', () => { + withPlatform('linux', () => { + const updatedState = migrations.apply(state, 37) + const { shortcuts } = updatedState.main + + expect(shortcuts).toStrictEqual({ + summon: { + modifierKeys: ['Alt'], + shortcutKey: 'Slash', + enabled: true, + configuring: false + } + }) + }) + }) + + it('should update the summon shortcut on Windows', () => { + withPlatform('win32', () => { + const updatedState = migrations.apply(state, 37) + const { shortcuts } = updatedState.main + + expect(shortcuts).toStrictEqual({ + summon: { + modifierKeys: ['Alt', 'Control'], + shortcutKey: 'Slash', + enabled: true, + configuring: false + } + }) + }) + }) + }) + + describe('when altGr is not a modifier', () => { + beforeEach(() => { + state.main.shortcuts = { + summon: { modifierKeys: ['Alt'], shortcutKey: 'Slash', enabled: true, configuring: false } + } + }) + + it('should not update the summon shortcut on Windows', () => { + withPlatform('win32', () => { + const updatedState = migrations.apply(state, 37) + const { shortcuts } = updatedState.main + + expect(shortcuts).toStrictEqual({ + summon: { + modifierKeys: ['Alt'], + shortcutKey: 'Slash', + enabled: true, + configuring: false + } + }) + }) + }) + + it('should not update the summon shortcut on Linux', () => { + withPlatform('linux', () => { + const updatedState = migrations.apply(state, 37) + const { shortcuts } = updatedState.main + + expect(shortcuts).toStrictEqual({ + summon: { + modifierKeys: ['Alt'], + shortcutKey: 'Slash', + enabled: true, + configuring: false + } + }) + }) + }) + }) +}) diff --git a/test/util.js b/test/util.js index 34747b5ef..79c0dc653 100644 --- a/test/util.js +++ b/test/util.js @@ -2,3 +2,16 @@ import { intToHex } from '@ethereumjs/util' export const gweiToHex = (gwei) => intToHex(gwei * 1e9) export const flushPromises = () => new Promise(jest.requireActual('timers').setImmediate) + +export async function withPlatform(platform, test) { + const originalPlatform = process.platform + Object.defineProperty(process, 'platform', { + value: platform + }) + + await test() + + Object.defineProperty(process, 'platform', { + value: originalPlatform + }) +}