Repository navigation
fix(devtools): support the keyboard shortcut on non-QWERTY layouts #1136
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
Open
ahmetomerv
wants to merge
14
commits into
nuxt:main
Choose a base branch
from
ahmetomerv:fix/keyboard-shortcut-non-qwerty
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
7fbc461
fix(devtools): support the shortcut on non-QWERTY layouts
ahmetomerv 37f5fa9
fix(devtools): sync the shortcut with the dock state
ahmetomerv 789a829
chore(devtools): remove redundant shortcut comment
ahmetomerv 0929303
[autofix.ci] apply automated fixes
autofix-ci[bot] 4437d4d
fix(devtools): hide the panel when closing from the shortcut
ahmetomerv f2ac809
fix(devtools): ignore stale keyboard layout results
ahmetomerv 442ccb8
fix(devtools): open the panel when the shortcut is pressed
ahmetomerv 58599cf
fix(devtools): drop stale shortcut codes on refresh
ahmetomerv 498a339
fix(devtools): close the panel session when toggling
ahmetomerv ee20ff3
fix(devtools): keep shortcut codes empty during refresh
ahmetomerv 0524564
Merge branch 'main' into fix/keyboard-shortcut-non-qwerty
ahmetomerv 1a0768d
[autofix.ci] apply automated fixes
autofix-ci[bot] e30359a
Merge branch 'main' into fix/keyboard-shortcut-non-qwerty
ahmetomerv 78b980e
[autofix.ci] apply automated fixes
autofix-ci[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| // DevTools is toggled with `Shift + Alt + D`. `KeyboardEvent.code` describes | ||
| // the physical key position, so it only matches the letter D on a QWERTY | ||
| // layout. Users on Workman, Colemak, etc. press a different physical key, and | ||
| // macOS rewrites the produced character while Option is held (`d` -> `∂`, | ||
| // with Shift -> `Î`). Resolve the keys that actually type `d` in the active | ||
| // layout and fall back to the produced key and the legacy key code. | ||
|
|
||
| interface KeyboardLayoutMapLike { | ||
| forEach: (callback: (key: string, code: string) => void) => void | ||
| } | ||
|
|
||
| interface KeyboardLike { | ||
| getLayoutMap: () => Promise<KeyboardLayoutMapLike> | ||
| addEventListener?: (type: 'layoutchange', listener: () => void) => void | ||
| } | ||
|
|
||
| export interface DevtoolsShortcutEvent { | ||
| altKey: boolean | ||
| shiftKey: boolean | ||
| ctrlKey?: boolean | ||
| metaKey?: boolean | ||
| code?: string | ||
| key?: string | ||
| keyCode?: number | ||
| } | ||
|
|
||
| export interface DevtoolsShortcutOptions { | ||
| /** Require Shift to be held. Opening requires it; closing from the panel does not. */ | ||
| shift?: boolean | ||
| /** Physical codes that type `d` in the active layout. Exposed for testing. */ | ||
| codes?: ReadonlySet<string> | ||
| } | ||
|
|
||
| const D_KEY_CODE = 68 | ||
| const QWERTY_D_CODE = 'KeyD' | ||
|
|
||
| const shortcutCodes = new Set([QWERTY_D_CODE]) | ||
|
|
||
| export function resolveShortcutCodes(layout: KeyboardLayoutMapLike): Set<string> { | ||
| const codes = new Set<string>() | ||
| layout.forEach((key, code) => { | ||
| if (key.toLowerCase() === 'd') | ||
| codes.add(code) | ||
| }) | ||
| return codes | ||
| } | ||
|
|
||
| function getKeyboard(): KeyboardLike | undefined { | ||
| if (typeof navigator === 'undefined') | ||
| return | ||
| return (navigator as Navigator & { keyboard?: KeyboardLike }).keyboard | ||
| } | ||
|
|
||
| let watching = false | ||
| let refreshRequest = 0 | ||
|
|
||
| export function watchShortcutLayoutMap(): void { | ||
| if (watching) | ||
| return | ||
| watching = true | ||
|
|
||
| const keyboard = getKeyboard() | ||
| if (!keyboard?.getLayoutMap) | ||
| return | ||
|
|
||
| const refresh = () => { | ||
| const request = ++refreshRequest | ||
| try { | ||
| keyboard.getLayoutMap() | ||
| .then((layout) => { | ||
| if (request !== refreshRequest) | ||
| return | ||
| const codes = resolveShortcutCodes(layout) | ||
| // Keep the QWERTY fallback if the map unexpectedly has no letter D. | ||
| if (!codes.size) | ||
| return | ||
| shortcutCodes.clear() | ||
| for (const code of codes) | ||
| shortcutCodes.add(code) | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
| }) | ||
| .catch(() => {}) | ||
| } | ||
| catch { | ||
| // Blocked by a permissions policy. `code`, `key` and `keyCode` matching | ||
| // still work without the layout map. | ||
| } | ||
| } | ||
|
|
||
| refresh() | ||
| keyboard.addEventListener?.('layoutchange', refresh) | ||
| } | ||
|
|
||
| export function isDevtoolsShortcut( | ||
| event: DevtoolsShortcutEvent, | ||
| options: DevtoolsShortcutOptions = {}, | ||
| ): boolean { | ||
| if (!event.altKey) | ||
| return false | ||
| if (options.shift && !event.shiftKey) | ||
| return false | ||
| if (event.ctrlKey || event.metaKey) | ||
| return false | ||
|
|
||
| const codes = options.codes ?? shortcutCodes | ||
| if (event.code && codes.has(event.code)) | ||
| return true | ||
|
|
||
| if (event.key?.toLowerCase() === 'd') | ||
| return true | ||
|
|
||
| // Deprecated, but it identifies the key produced by the active layout and | ||
| // ignores modifiers, which matters on macOS where Option rewrites `key`. | ||
| return event.keyCode === D_KEY_CODE | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| import { afterEach, describe, expect, it, vi } from 'vitest' | ||
| import { isDevtoolsShortcut, resolveShortcutCodes } from '../src/runtime/shared/shortcut' | ||
|
|
||
| function deferred<T>() { | ||
| let resolve!: (value: T) => void | ||
| const promise = new Promise<T>((r) => { | ||
| resolve = r | ||
| }) | ||
| return { promise, resolve } | ||
| } | ||
|
|
||
| function createEvent(overrides: Partial<Parameters<typeof isDevtoolsShortcut>[0]> = {}) { | ||
| return { | ||
| altKey: true, | ||
| shiftKey: true, | ||
| code: 'KeyD', | ||
| key: 'd', | ||
| keyCode: 68, | ||
| ...overrides, | ||
| } | ||
| } | ||
|
|
||
| // Physical `KeyW` on a Workman layout. Option turns the produced `d` into `Î`, | ||
| // but the key code stays the one for the letter D. | ||
| const workmanD = createEvent({ | ||
| code: 'KeyW', | ||
| key: 'Î', | ||
| }) | ||
|
|
||
| describe('isDevtoolsShortcut', () => { | ||
| it('matches Shift + Alt + D on a QWERTY layout', () => { | ||
| expect(isDevtoolsShortcut(createEvent(), { shift: true })).toBe(true) | ||
| }) | ||
|
|
||
| it('matches the QWERTY position even when Option rewrites the key', () => { | ||
| expect(isDevtoolsShortcut(createEvent({ key: 'Î' }), { shift: true })).toBe(true) | ||
| }) | ||
|
|
||
| it('matches the key that types d on a non-QWERTY layout', () => { | ||
| const codes = resolveShortcutCodes(new Map([['KeyW', 'd'], ['KeyD', 's']])) | ||
| expect(isDevtoolsShortcut(workmanD, { shift: true, codes })).toBe(true) | ||
| }) | ||
|
|
||
| it('ignores the physical QWERTY position when it types another letter', () => { | ||
| const codes = resolveShortcutCodes(new Map([['KeyD', 's'], ['KeyG', 'd']])) | ||
| expect(isDevtoolsShortcut(createEvent({ code: 'KeyD', key: 'S', keyCode: 83 }), { shift: true, codes })).toBe(false) | ||
| }) | ||
|
|
||
| it('falls back to the legacy key code when the Keyboard API is unavailable', () => { | ||
| expect(isDevtoolsShortcut(workmanD, { shift: true })).toBe(true) | ||
| }) | ||
|
|
||
| it('matches the produced key where Option does not rewrite it', () => { | ||
| expect(isDevtoolsShortcut(createEvent({ code: 'KeyG', key: 'd', keyCode: 0 }), { shift: true })).toBe(true) | ||
| }) | ||
|
|
||
| it('ignores keys that do not type d', () => { | ||
| expect(isDevtoolsShortcut(createEvent({ code: 'KeyS', key: 's', keyCode: 83 }), { shift: true })).toBe(false) | ||
| }) | ||
|
|
||
| it('requires Alt', () => { | ||
| expect(isDevtoolsShortcut(createEvent({ altKey: false }), { shift: true })).toBe(false) | ||
| }) | ||
|
|
||
| it('requires Shift when opening', () => { | ||
| expect(isDevtoolsShortcut(createEvent({ shiftKey: false }), { shift: true })).toBe(false) | ||
| }) | ||
|
|
||
| it('allows Shift to be optional when closing', () => { | ||
| expect(isDevtoolsShortcut(createEvent({ shiftKey: false }))).toBe(true) | ||
| expect(isDevtoolsShortcut(createEvent())).toBe(true) | ||
| }) | ||
|
|
||
| it('ignores Ctrl and Meta combinations', () => { | ||
| expect(isDevtoolsShortcut(createEvent({ ctrlKey: true }), { shift: true })).toBe(false) | ||
| expect(isDevtoolsShortcut(createEvent({ metaKey: true }), { shift: true })).toBe(false) | ||
| }) | ||
| }) | ||
|
|
||
| describe('resolveShortcutCodes', () => { | ||
| it('adds every code that types d in the active layout', () => { | ||
| const codes = resolveShortcutCodes(new Map([['KeyW', 'd'], ['KeyG', 'D'], ['KeyS', 's']])) | ||
| expect([...codes].sort()).toEqual(['KeyG', 'KeyW']) | ||
| }) | ||
|
|
||
| it('does not keep physical positions that do not type d', () => { | ||
| const codes = resolveShortcutCodes(new Map([['KeyD', 's'], ['KeyG', 'd']])) | ||
| expect(codes.has('KeyD')).toBe(false) | ||
| expect(codes.has('KeyG')).toBe(true) | ||
| }) | ||
|
|
||
| it('returns an empty set when the layout has no letter d', () => { | ||
| expect(resolveShortcutCodes(new Map()).size).toBe(0) | ||
| }) | ||
| }) | ||
|
|
||
| describe('watchShortcutLayoutMap', () => { | ||
| afterEach(() => { | ||
| vi.unstubAllGlobals() | ||
| vi.resetModules() | ||
| }) | ||
|
|
||
| it('ignores a stale result from an older layout request', async () => { | ||
| vi.resetModules() | ||
| const first = deferred<Map<string, string>>() | ||
| const second = deferred<Map<string, string>>() | ||
| const pending = [first.promise, second.promise] | ||
| let onLayoutChange: (() => void) | undefined | ||
|
|
||
| vi.stubGlobal('navigator', { | ||
| keyboard: { | ||
| getLayoutMap: () => pending.shift()!, | ||
| addEventListener: (_type: string, listener: () => void) => { | ||
| onLayoutChange = listener | ||
| }, | ||
| }, | ||
| }) | ||
|
|
||
| const { isDevtoolsShortcut: isShortcut, watchShortcutLayoutMap } = await import('../src/runtime/shared/shortcut') | ||
|
|
||
| watchShortcutLayoutMap() | ||
| onLayoutChange?.() | ||
|
|
||
| // The second request reflects the new layout and resolves first. | ||
| second.resolve(new Map([['KeyG', 'd']])) | ||
| await Promise.resolve() | ||
| await Promise.resolve() | ||
|
|
||
| // The first request belongs to the old layout and resolves last. | ||
| first.resolve(new Map([['KeyD', 'd']])) | ||
| await Promise.resolve() | ||
| await Promise.resolve() | ||
|
|
||
| const colemakD = { altKey: true, shiftKey: true, code: 'KeyG', key: 'x', keyCode: 0 } | ||
| expect(isShortcut(colemakD, { shift: true })).toBe(true) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.