Skip to content

Commit 4571e28

Browse files
serhalpdanielroe
serhalp
and
danielroe
authored
fix: ensure keybindings always ignore modifiers and editable elements (#607)
Co-authored-by: Daniel Roe <daniel@roe.dev>
1 parent a2b6922 commit 4571e28

7 files changed

Lines changed: 113 additions & 49 deletions

File tree

‎app/app.vue‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ if (import.meta.server) {
4242
function handleGlobalKeydown(e: KeyboardEvent) {
4343
if (isEditableElement(e.target)) return
4444
45-
if (e.key === '/') {
45+
if (isKeyWithoutModifiers(e, '/')) {
4646
e.preventDefault()
4747
4848
// Try to find and focus search input on current page
@@ -58,7 +58,7 @@ function handleGlobalKeydown(e: KeyboardEvent) {
5858
router.push('/search')
5959
}
6060
61-
if (e.key === '?') {
61+
if (isKeyWithoutModifiers(e, '?')) {
6262
e.preventDefault()
6363
showKbdHints.value = true
6464
}

‎app/components/AppHeader.vue‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,23 +62,21 @@ function handleSearchFocus() {
6262
}
6363
6464
onKeyStroke(
65-
',',
65+
e => isKeyWithoutModifiers(e, ',') && !isEditableElement(e.target),
6666
e => {
67-
if (isEditableElement(e.target)) return
68-
6967
e.preventDefault()
7068
navigateTo('/settings')
7169
},
7270
{ dedupe: true },
7371
)
7472
7573
onKeyStroke(
76-
'c',
77-
e => {
74+
e =>
75+
isKeyWithoutModifiers(e, 'c') &&
76+
!isEditableElement(e.target) &&
7877
// Allow more specific handlers to take precedence
79-
if (e.defaultPrevented) return
80-
if (isEditableElement(e.target)) return
81-
78+
!e.defaultPrevented,
79+
e => {
8280
e.preventDefault()
8381
navigateTo('/compare')
8482
},

‎app/components/MobileMenu.vue‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,12 @@ const route = useRoute()
2929
watch(() => route.fullPath, closeMenu)
3030
3131
// Close on escape
32-
onKeyStroke('Escape', () => {
33-
if (isOpen.value) {
32+
onKeyStroke(
33+
e => isKeyWithoutModifiers(e, 'Escape') && isOpen.value,
34+
e => {
3435
isOpen.value = false
35-
}
36-
})
36+
},
37+
)
3738
3839
// Prevent body scroll when menu is open
3940
const isLocked = useScrollLock(document)

‎app/pages/[...package].vue‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -313,41 +313,38 @@ useSeoMeta({
313313
})
314314
315315
onKeyStroke(
316-
'.',
316+
e => isKeyWithoutModifiers(e, '.') && !isEditableElement(e.target),
317317
e => {
318-
if (isEditableElement(e.target)) return
319-
if (pkg.value && displayVersion.value) {
320-
e.preventDefault()
321-
navigateTo({
322-
name: 'code',
323-
params: {
324-
path: [pkg.value.name, 'v', displayVersion.value.version],
325-
},
326-
})
327-
}
318+
if (pkg.value == null || displayVersion.value == null) return
319+
e.preventDefault()
320+
navigateTo({
321+
name: 'code',
322+
params: {
323+
path: [pkg.value.name, 'v', displayVersion.value.version],
324+
},
325+
})
328326
},
329327
{ dedupe: true },
330328
)
331329
332330
onKeyStroke(
333-
'd',
331+
e => isKeyWithoutModifiers(e, 'd') && !isEditableElement(e.target),
334332
e => {
335-
if (isEditableElement(e.target)) return
336-
if (docsLink.value) {
337-
e.preventDefault()
338-
navigateTo(docsLink.value)
339-
}
333+
if (!docsLink.value) return
334+
e.preventDefault()
335+
navigateTo(docsLink.value)
340336
},
341337
{ dedupe: true },
342338
)
343339
344-
onKeyStroke('c', e => {
345-
if (isEditableElement(e.target)) return
346-
if (pkg.value) {
340+
onKeyStroke(
341+
e => isKeyWithoutModifiers(e, 'c') && !isEditableElement(e.target),
342+
e => {
343+
if (!pkg.value) return
347344
e.preventDefault()
348345
router.push({ path: '/compare', query: { packages: pkg.value.name } })
349-
}
350-
})
346+
},
347+
)
351348
352349
defineOgImageComponent('Package', {
353350
name: () => pkg.value?.name ?? 'Package',

‎app/pages/settings.vue‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,15 @@ const { locale, locales, setLocale: setNuxti18nLocale } = useI18n()
55
const colorMode = useColorMode()
66
const { currentLocaleStatus, isSourceLocale } = useI18nStatus()
77
8-
// Escape to go back (but not when focused on form elements)
8+
// Escape to go back (but not when focused on form elements or modal is open)
99
onKeyStroke(
10-
'Escape',
10+
e =>
11+
isKeyWithoutModifiers(e, 'Escape') &&
12+
!isEditableElement(e.target) &&
13+
!document.documentElement.matches('html:has(:modal)'),
1114
e => {
12-
const target = e.target as HTMLElement
13-
if (
14-
!['INPUT', 'SELECT', 'TEXTAREA'].includes(target?.tagName) &&
15-
!document.documentElement.matches('html:has(:modal)')
16-
) {
17-
e.preventDefault()
18-
router.back()
19-
}
15+
e.preventDefault()
16+
router.back()
2017
},
2118
{ dedupe: true },
2219
)
@@ -85,7 +82,9 @@ const setLocale: typeof setNuxti18nLocale = locale => {
8582
| 'system'
8683
"
8784
>
88-
<option value="system">{{ $t('settings.theme_system') }}</option>
85+
<option value="system">
86+
{{ $t('settings.theme_system') }}
87+
</option>
8988
<option value="light">{{ $t('settings.theme_light') }}</option>
9089
<option value="dark">{{ $t('settings.theme_dark') }}</option>
9190
</select>

‎app/utils/input.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,16 @@ export function isEditableElement(target: EventTarget | null): boolean {
1313
if (!target || !(target instanceof HTMLElement)) return false
1414
return target.tagName === 'INPUT' || target.tagName === 'TEXTAREA' || target.isContentEditable
1515
}
16+
17+
/**
18+
* Check if a keyboard event matches a specific key without any modifier keys.
19+
*/
20+
export function isKeyWithoutModifiers(event: KeyboardEvent, key: string): boolean {
21+
return (
22+
event.key.toLowerCase() === key.toLowerCase() &&
23+
!event.altKey &&
24+
!event.ctrlKey &&
25+
!event.metaKey &&
26+
!event.shiftKey
27+
)
28+
}

‎test/e2e/interactions.spec.ts‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ test.describe('Search Pages', () => {
4848
// Wait for navigation to /search (debounce is 250ms)
4949
await expect(page).toHaveURL(/\/search/, { timeout: 10000 })
5050

51-
await expect(page.locator('[data-result-index="0"]').first()).toBeVisible({ timeout: 15000 })
51+
await expect(page.locator('[data-result-index="0"]').first()).toBeVisible({
52+
timeout: 15000,
53+
})
5254

5355
// Home search input should be gone (we're on /search now)
5456
await expect(homeSearchInput).not.toBeVisible()
@@ -70,7 +72,9 @@ test.describe('Search Pages', () => {
7072

7173
await expect(page).toHaveURL(/\/search/, { timeout: 10000 })
7274

73-
await expect(page.locator('[data-result-index="0"]').first()).toBeVisible({ timeout: 15000 })
75+
await expect(page.locator('[data-result-index="0"]').first()).toBeVisible({
76+
timeout: 15000,
77+
})
7478

7579
const headerSearchInput = page.locator('#header-search')
7680
await expect(headerSearchInput).toBeFocused()
@@ -86,6 +90,21 @@ test.describe('Keyboard Shortcuts', () => {
8690
await expect(page).toHaveURL(/\/compare/)
8791
})
8892

93+
test('"c" does not navigate when any modifier key is pressed', async ({ page, goto }) => {
94+
await goto('/settings', { waitUntil: 'hydration' })
95+
96+
await page.keyboard.press('Shift+c')
97+
await expect(page).toHaveURL(/\/settings/)
98+
await page.keyboard.press('Control+c')
99+
await expect(page).toHaveURL(/\/settings/)
100+
await page.keyboard.press('Alt+c')
101+
await expect(page).toHaveURL(/\/settings/)
102+
await page.keyboard.press('Meta+c')
103+
await expect(page).toHaveURL(/\/settings/)
104+
await page.keyboard.press('ControlOrMeta+Shift+c')
105+
await expect(page).toHaveURL(/\/settings/)
106+
})
107+
89108
test('"c" on package page navigates to /compare with package pre-filled', async ({
90109
page,
91110
goto,
@@ -113,6 +132,24 @@ test.describe('Keyboard Shortcuts', () => {
113132
await expect(searchInput).toHaveValue('c')
114133
})
115134

135+
test('"c" on package page does not navigate when any modifier key is pressed', async ({
136+
page,
137+
goto,
138+
}) => {
139+
await goto('/vue', { waitUntil: 'hydration' })
140+
141+
await page.keyboard.press('Shift+c')
142+
await expect(page).toHaveURL(/\/vue/)
143+
await page.keyboard.press('Control+c')
144+
await expect(page).toHaveURL(/\/vue/)
145+
await page.keyboard.press('Alt+c')
146+
await expect(page).toHaveURL(/\/vue/)
147+
await page.keyboard.press('Meta+c')
148+
await expect(page).toHaveURL(/\/vue/)
149+
await page.keyboard.press('ControlOrMeta+Shift+c')
150+
await expect(page).toHaveURL(/\/vue/)
151+
})
152+
116153
test('"," navigates to /settings', async ({ page, goto }) => {
117154
await goto('/compare', { waitUntil: 'hydration' })
118155

@@ -121,6 +158,25 @@ test.describe('Keyboard Shortcuts', () => {
121158
await expect(page).toHaveURL(/\/settings/)
122159
})
123160

161+
test('"," does not navigate when any modifier key is pressed', async ({ page, goto }) => {
162+
await goto('/settings', { waitUntil: 'hydration' })
163+
164+
const searchInput = page.locator('#header-search')
165+
await searchInput.focus()
166+
await expect(searchInput).toBeFocused()
167+
168+
await page.keyboard.press('Shift+,')
169+
await expect(page).toHaveURL(/\/settings/)
170+
await page.keyboard.press('Control+,')
171+
await expect(page).toHaveURL(/\/settings/)
172+
await page.keyboard.press('Alt+,')
173+
await expect(page).toHaveURL(/\/settings/)
174+
await page.keyboard.press('Meta+,')
175+
await expect(page).toHaveURL(/\/settings/)
176+
await page.keyboard.press('ControlOrMeta+Shift+,')
177+
await expect(page).toHaveURL(/\/settings/)
178+
})
179+
124180
test('"," does not navigate when search input is focused', async ({ page, goto }) => {
125181
await goto('/compare', { waitUntil: 'hydration' })
126182

0 commit comments

Comments
 (0)