refactor: useKeyboardEvents composable (#9959)
This PR has the following changes 1. Fix tab styles issue caused by adding an additional wrapper for getting an element ref on `ChatTypeTabs.vue` 2. Refactor `useKeyboardEvents` composable to not require an element ref. It will use a local abort controller to abort any listener --------- Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com>
This commit is contained in:
@@ -1,4 +1,3 @@
|
||||
import { unref } from 'vue';
|
||||
import { useKeyboardEvents } from 'dashboard/composables/useKeyboardEvents';
|
||||
|
||||
describe('useKeyboardEvents', () => {
|
||||
@@ -11,15 +10,13 @@ describe('useKeyboardEvents', () => {
|
||||
});
|
||||
|
||||
it('should set up listeners on mount and remove them on unmount', async () => {
|
||||
const el = document.createElement('div');
|
||||
const elRef = unref({ value: el });
|
||||
const events = {
|
||||
'ALT+KeyL': () => {},
|
||||
};
|
||||
|
||||
const mountedMock = vi.fn();
|
||||
const unmountedMock = vi.fn();
|
||||
useKeyboardEvents(events, elRef);
|
||||
useKeyboardEvents(events);
|
||||
mountedMock();
|
||||
unmountedMock();
|
||||
|
||||
|
||||
@@ -9,7 +9,6 @@ vi.mock('../useKeyboardEvents', () => ({
|
||||
}));
|
||||
|
||||
describe('useKeyboardNavigableList', () => {
|
||||
let elementRef;
|
||||
let items;
|
||||
let onSelect;
|
||||
let adjustScroll;
|
||||
@@ -18,7 +17,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
const createMockEvent = () => ({ preventDefault: vi.fn() });
|
||||
|
||||
beforeEach(() => {
|
||||
elementRef = ref(null);
|
||||
items = ref(['item1', 'item2', 'item3']);
|
||||
onSelect = vi.fn();
|
||||
adjustScroll = vi.fn();
|
||||
@@ -28,7 +26,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should return moveSelectionUp and moveSelectionDown functions', () => {
|
||||
const result = useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
@@ -43,7 +40,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should move selection up correctly', () => {
|
||||
const { moveSelectionUp } = useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
@@ -65,7 +61,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should move selection down correctly', () => {
|
||||
const { moveSelectionDown } = useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
@@ -87,7 +82,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should call adjustScroll after moving selection', () => {
|
||||
const { moveSelectionUp, moveSelectionDown } = useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
@@ -103,7 +97,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should include Enter key handler when onSelect is provided', () => {
|
||||
useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
@@ -118,7 +111,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should not include Enter key handler when onSelect is not provided', () => {
|
||||
useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
adjustScroll,
|
||||
selectedIndex,
|
||||
@@ -131,7 +123,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should not trigger onSelect when items are empty', () => {
|
||||
const { moveSelectionUp, moveSelectionDown } = useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items: ref([]),
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
@@ -145,23 +136,18 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should call useKeyboardEvents with correct parameters', () => {
|
||||
useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
selectedIndex,
|
||||
});
|
||||
|
||||
expect(useKeyboardEvents).toHaveBeenCalledWith(
|
||||
expect.any(Object),
|
||||
elementRef
|
||||
);
|
||||
expect(useKeyboardEvents).toHaveBeenCalledWith(expect.any(Object));
|
||||
});
|
||||
|
||||
// Keyboard event handlers
|
||||
it('should handle ArrowUp key', () => {
|
||||
useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
@@ -178,7 +164,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should handle Control+KeyP', () => {
|
||||
useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
@@ -195,7 +180,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should handle ArrowDown key', () => {
|
||||
useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
@@ -212,7 +196,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should handle Control+KeyN', () => {
|
||||
useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
@@ -229,7 +212,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should handle Enter key when onSelect is provided', () => {
|
||||
useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
@@ -245,7 +227,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should not have Enter key handler when onSelect is not provided', () => {
|
||||
useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
adjustScroll,
|
||||
selectedIndex,
|
||||
@@ -257,7 +238,6 @@ describe('useKeyboardNavigableList', () => {
|
||||
|
||||
it('should set allowOnFocusedInput to true for all key handlers', () => {
|
||||
useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
import { onMounted, onBeforeUnmount, unref } from 'vue';
|
||||
import {
|
||||
isActiveElementTypeable,
|
||||
isEscape,
|
||||
@@ -7,8 +6,7 @@ import {
|
||||
} from 'shared/helpers/KeyboardHelpers';
|
||||
import { useDetectKeyboardLayout } from 'dashboard/composables/useDetectKeyboardLayout';
|
||||
import { createKeybindingsHandler } from 'tinykeys';
|
||||
|
||||
const keyboardListenerMap = new WeakMap();
|
||||
import { onUnmounted, onMounted } from 'vue';
|
||||
|
||||
/**
|
||||
* Determines if the keyboard event should be ignored based on the element type and handler settings.
|
||||
@@ -69,49 +67,24 @@ async function wrapEventsInKeybindingsHandler(events) {
|
||||
return wrappedEvents;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets up keyboard event listeners on the specified element.
|
||||
* @param {Element} root - The DOM element to attach listeners to.
|
||||
* @param {Object} events - The events to listen for.
|
||||
*/
|
||||
const setupListeners = (root, events) => {
|
||||
if (root instanceof Element && events) {
|
||||
const keydownHandler = createKeybindingsHandler(events);
|
||||
document.addEventListener('keydown', keydownHandler);
|
||||
keyboardListenerMap.set(root, keydownHandler);
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Removes keyboard event listeners from the specified element.
|
||||
* @param {Element} root - The DOM element to remove listeners from.
|
||||
*/
|
||||
const removeListeners = root => {
|
||||
// In the future, let's use the abort controller to remove the listeners
|
||||
// https://caniuse.com/abortcontroller
|
||||
if (root instanceof Element) {
|
||||
const handlerToRemove = keyboardListenerMap.get(root);
|
||||
document.removeEventListener('keydown', handlerToRemove);
|
||||
keyboardListenerMap.delete(root);
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Vue composable to handle keyboard events with support for different keyboard layouts.
|
||||
* @param {Object} keyboardEvents - The keyboard events to handle.
|
||||
* @param {ref} elRef - A Vue ref to the element to attach the keyboard events to.
|
||||
*/
|
||||
export function useKeyboardEvents(keyboardEvents, elRef = null) {
|
||||
export async function useKeyboardEvents(keyboardEvents) {
|
||||
let abortController = new AbortController();
|
||||
|
||||
onMounted(async () => {
|
||||
const el = unref(elRef);
|
||||
const getKeyboardEvents = () => keyboardEvents || null;
|
||||
const events = getKeyboardEvents();
|
||||
const wrappedEvents = await wrapEventsInKeybindingsHandler(events);
|
||||
setupListeners(el, wrappedEvents);
|
||||
if (!keyboardEvents) return;
|
||||
const wrappedEvents = await wrapEventsInKeybindingsHandler(keyboardEvents);
|
||||
const keydownHandler = createKeybindingsHandler(wrappedEvents);
|
||||
|
||||
document.addEventListener('keydown', keydownHandler, {
|
||||
signal: abortController.signal,
|
||||
});
|
||||
});
|
||||
|
||||
onBeforeUnmount(() => {
|
||||
const el = unref(elRef);
|
||||
removeListeners(el);
|
||||
onUnmounted(() => {
|
||||
abortController.abort();
|
||||
});
|
||||
}
|
||||
|
||||
@@ -84,7 +84,6 @@ const updateSelectionIndex = (currentIndex, itemsLength, direction) => {
|
||||
* }} An object containing functions to move the selection up and down.
|
||||
*/
|
||||
export function useKeyboardNavigableList({
|
||||
elementRef,
|
||||
items,
|
||||
onSelect,
|
||||
adjustScroll,
|
||||
@@ -109,7 +108,7 @@ export function useKeyboardNavigableList({
|
||||
items
|
||||
);
|
||||
|
||||
useKeyboardEvents(keyboardEvents, elementRef);
|
||||
useKeyboardEvents(keyboardEvents);
|
||||
|
||||
return {
|
||||
moveSelectionUp,
|
||||
|
||||
Reference in New Issue
Block a user