-
-
Notifications
You must be signed in to change notification settings - Fork 2
refactor(useKeydown): optimize keydown event handling #8
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
base: master
Are you sure you want to change the base?
Changes from 11 commits
2d01f84
dd63e72
789ed35
e43f89f
4d41b31
f4d8bfb
fb0aef3
9023613
edd8854
86551cf
5ef5ef4
1dc6fce
3513489
242bb48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,131 +1,89 @@ | ||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' | ||
import { useKeydown } from './index' | ||
|
||
// Mock Vue's lifecycle hooks | ||
vi.mock('vue', async () => { | ||
const actual = await vi.importActual('vue') | ||
return { | ||
...actual, | ||
onMounted: vi.fn(), | ||
getCurrentScope: vi.fn(() => true), | ||
onScopeDispose: vi.fn(), | ||
} | ||
}) | ||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' | ||
import { useKeydown, handlerMap } from './index' | ||
|
||
describe('useKeydown', () => { | ||
let addEventListenerSpy: ReturnType<typeof vi.spyOn> | ||
let removeEventListenerSpy: ReturnType<typeof vi.spyOn> | ||
|
||
beforeEach(() => { | ||
vi.clearAllMocks() | ||
// Mock document.addEventListener and removeEventListener | ||
vi.spyOn(document, 'addEventListener') | ||
vi.spyOn(document, 'removeEventListener') | ||
addEventListenerSpy = vi.spyOn(document, 'addEventListener') | ||
removeEventListenerSpy = vi.spyOn(document, 'removeEventListener') | ||
}) | ||
|
||
afterEach(() => { | ||
vi.restoreAllMocks() | ||
handlerMap.clear() | ||
}) | ||
|
||
describe('basic functionality', () => { | ||
it('should return startListening and stopListening functions', () => { | ||
const handler = { key: 'Enter', handler: vi.fn() } | ||
const result = useKeydown(handler) | ||
|
||
expect(result).toHaveProperty('startListening') | ||
expect(result).toHaveProperty('stopListening') | ||
expect(typeof result.startListening).toBe('function') | ||
expect(typeof result.stopListening).toBe('function') | ||
}) | ||
|
||
it('should accept single handler', () => { | ||
const handler = { key: 'Enter', handler: vi.fn() } | ||
|
||
expect(() => useKeydown(handler)).not.toThrow() | ||
}) | ||
|
||
it('should accept array of handlers', () => { | ||
const handlers = [ | ||
{ key: 'Enter', handler: vi.fn() }, | ||
{ key: 'Escape', handler: vi.fn() }, | ||
] | ||
|
||
expect(() => useKeydown(handlers)).not.toThrow() | ||
}) | ||
}) | ||
|
||
describe('event handling', () => { | ||
it('should call handler when matching key is pressed', () => { | ||
const mockHandler = vi.fn() | ||
const handler = { key: 'Enter', handler: mockHandler } | ||
const { startListening } = useKeydown(handler) | ||
it('should create only one global listener for multiple useKeydown calls', () => { | ||
const handler1 = vi.fn() | ||
const handler2 = vi.fn() | ||
const handler3 = vi.fn() | ||
|
||
startListening() | ||
const keydown1 = useKeydown({ key: 'Enter', handler: handler1 }) | ||
const keydown2 = useKeydown({ key: 'Escape', handler: handler2 }) | ||
const keydown3 = useKeydown({ key: 'Space', handler: handler3 }) | ||
|
||
// Simulate keydown event | ||
const event = new KeyboardEvent('keydown', { key: 'Enter' }) | ||
document.dispatchEvent(event) | ||
keydown1.startListening() | ||
keydown2.startListening() | ||
keydown3.startListening() | ||
|
||
expect(mockHandler).toHaveBeenCalledWith(event) | ||
}) | ||
expect(addEventListenerSpy).toHaveBeenCalledTimes(1) | ||
expect(addEventListenerSpy).toHaveBeenCalledWith('keydown', expect.any(Function)) | ||
|
||
it('should not call handler when non-matching key is pressed', () => { | ||
const mockHandler = vi.fn() | ||
const handler = { key: 'Enter', handler: mockHandler } | ||
const { startListening } = useKeydown(handler) | ||
expect(handlerMap.size).toBe(3) | ||
|
||
startListening() | ||
keydown1.stopListening() | ||
keydown2.stopListening() | ||
keydown3.stopListening() | ||
|
||
// Simulate keydown event with different key | ||
const event = new KeyboardEvent('keydown', { key: 'Escape' }) | ||
document.dispatchEvent(event) | ||
expect(removeEventListenerSpy).toHaveBeenCalledTimes(1) | ||
expect(handlerMap.size).toBe(0) | ||
}) | ||
|
||
expect(mockHandler).not.toHaveBeenCalled() | ||
}) | ||
it('should handle multiple handlers for the same key', () => { | ||
const handler1 = vi.fn() | ||
const handler2 = vi.fn() | ||
|
||
it('should prevent default when preventDefault is true', () => { | ||
const mockHandler = vi.fn() | ||
const handler = { key: 'Enter', handler: mockHandler, preventDefault: true } | ||
const { startListening } = useKeydown(handler) | ||
const keydown1 = useKeydown({ key: 'Enter', handler: handler1 }) | ||
const keydown2 = useKeydown({ key: 'Enter', handler: handler2 }) | ||
|
||
startListening() | ||
keydown1.startListening() | ||
keydown2.startListening() | ||
|
||
const event = new KeyboardEvent('keydown', { key: 'Enter' }) | ||
const preventDefaultSpy = vi.spyOn(event, 'preventDefault') | ||
document.dispatchEvent(event) | ||
const event = { key: 'Enter' } as KeyboardEvent | ||
|
||
expect(preventDefaultSpy).toHaveBeenCalled() | ||
}) | ||
const registeredHandler = addEventListenerSpy.mock.calls[0][1] as (event: KeyboardEvent) => void | ||
registeredHandler(event) | ||
|
||
it('should stop propagation when stopPropagation is true', () => { | ||
const mockHandler = vi.fn() | ||
const handler = { key: 'Enter', handler: mockHandler, stopPropagation: true } | ||
const { startListening } = useKeydown(handler) | ||
expect(handler1).toHaveBeenCalledWith(event) | ||
expect(handler2).toHaveBeenCalledWith(event) | ||
|
||
startListening() | ||
keydown1.stopListening() | ||
keydown2.stopListening() | ||
}) | ||
|
||
const event = new KeyboardEvent('keydown', { key: 'Enter' }) | ||
const stopPropagationSpy = vi.spyOn(event, 'stopPropagation') | ||
document.dispatchEvent(event) | ||
it('should not remove global listener if other handlers are still active', () => { | ||
const handler1 = vi.fn() | ||
const handler2 = vi.fn() | ||
|
||
expect(stopPropagationSpy).toHaveBeenCalled() | ||
}) | ||
}) | ||
const keydown1 = useKeydown({ key: 'Enter', handler: handler1 }) | ||
const keydown2 = useKeydown({ key: 'Escape', handler: handler2 }) | ||
|
||
describe('lifecycle management', () => { | ||
it('should add event listener when startListening is called', () => { | ||
const handler = { key: 'Enter', handler: vi.fn() } | ||
const { startListening } = useKeydown(handler) | ||
keydown1.startListening() | ||
keydown2.startListening() | ||
|
||
startListening() | ||
expect(addEventListenerSpy).toHaveBeenCalledTimes(1) | ||
|
||
expect(document.addEventListener).toHaveBeenCalledWith('keydown', expect.any(Function)) | ||
}) | ||
keydown1.stopListening() | ||
|
||
it('should remove event listener when stopListening is called', () => { | ||
const handler = { key: 'Enter', handler: vi.fn() } | ||
const { stopListening } = useKeydown(handler) | ||
expect(removeEventListenerSpy).not.toHaveBeenCalled() | ||
expect(handlerMap.size).toBe(1) | ||
|
||
stopListening() | ||
keydown2.stopListening() | ||
|
||
expect(document.removeEventListener).toHaveBeenCalledWith('keydown', expect.any(Function)) | ||
}) | ||
expect(removeEventListenerSpy).toHaveBeenCalledTimes(1) | ||
expect(handlerMap.size).toBe(0) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
// Utilities | ||
import { onMounted, getCurrentScope, onScopeDispose } from 'vue' | ||
import { onMounted, getCurrentScope, onScopeDispose, ref, shallowReadonly } from 'vue' | ||
import type { ID } from '#v0/types' | ||
import { genId } from '#v0/utils/helpers' | ||
|
||
export interface KeyHandler { | ||
key: string | ||
|
@@ -8,27 +9,78 @@ export interface KeyHandler { | |
stopPropagation?: boolean | ||
} | ||
|
||
export function useKeydown (handlers: KeyHandler[] | KeyHandler) { | ||
const keyHandlers = Array.isArray(handlers) ? handlers : [handlers] | ||
export interface UseKeydownOptions { | ||
autoStart?: boolean | ||
} | ||
|
||
let globalListener: ((event: KeyboardEvent) => void) | null = null | ||
const handlerMap: Map<ID, KeyHandler> = new Map() | ||
|
||
function onKeydown (event: KeyboardEvent) { | ||
const handler = keyHandlers.find(h => h.key === event.key) | ||
if (handler) { | ||
if (handler.preventDefault) event.preventDefault() | ||
if (handler.stopPropagation) event.stopPropagation() | ||
handler.handler(event) | ||
function startGlobalListener () { | ||
if (typeof document === 'undefined') return | ||
|
||
if (!globalListener) { | ||
globalListener = (event: KeyboardEvent) => { | ||
for (const h of handlerMap.values()) { | ||
if (h.key === event.key) { | ||
if (h.preventDefault) event.preventDefault() | ||
if (h.stopPropagation) event.stopPropagation() | ||
h.handler(event) | ||
} | ||
} | ||
} | ||
document.addEventListener('keydown', globalListener) | ||
} | ||
} | ||
|
||
function startListening () { | ||
document.addEventListener('keydown', onKeydown) | ||
function stopGlobalListener () { | ||
if (typeof document === 'undefined') return | ||
|
||
if (globalListener && handlerMap.size === 0) { | ||
document.removeEventListener('keydown', globalListener) | ||
globalListener = null | ||
} | ||
} | ||
|
||
export function useKeydown (handlers: KeyHandler[] | KeyHandler, options: UseKeydownOptions = {}) { | ||
const { autoStart = true } = options | ||
AndreyYolkin marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
const keyHandlers = Array.isArray(handlers) ? handlers : [handlers] | ||
const handlerIds = ref<ID[]>([]) | ||
const isListening = ref(false) | ||
|
||
const startListening = () => { | ||
if (!isListening.value) { | ||
const ids = Array.from({ length: keyHandlers.length }, genId) | ||
|
||
function stopListening () { | ||
document.removeEventListener('keydown', onKeydown) | ||
for (const [index, id] of ids.entries()) { | ||
handlerMap.set(id, keyHandlers[index]) | ||
} | ||
|
||
handlerIds.value = ids | ||
|
||
if (handlerMap.size > 0) { | ||
startGlobalListener() | ||
} | ||
|
||
isListening.value = true | ||
} | ||
} | ||
|
||
if (getCurrentScope()) { | ||
const stopListening = () => { | ||
|
||
if (isListening.value) { | ||
for (const id of handlerIds.value) { | ||
handlerMap.delete(id) | ||
} | ||
handlerIds.value = [] | ||
isListening.value = false | ||
|
||
if (handlerMap.size === 0) { | ||
stopGlobalListener() | ||
} | ||
} | ||
} | ||
|
||
if (getCurrentScope() && autoStart) { | ||
onMounted(startListening) | ||
} | ||
|
||
|
@@ -37,5 +89,8 @@ export function useKeydown (handlers: KeyHandler[] | KeyHandler) { | |
return { | ||
startListening, | ||
stopListening, | ||
isListening: shallowReadonly(isListening), | ||
} | ||
} | ||
|
||
export { handlerMap } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whenever
if { ... }
guards most of the logic within a function it should be replaced with early exitif (!..) return
, so we avoid unnecessary indentation and the inner logic is easier to read/scan-through.There are 3 methods here that would benefit from this quick refactoring.