Skip to content

Commit dc8062b

Browse files
koesie10koesie10
authored
Merge pull request #3242 from github/koesie10/suggest-box-open-key
Open suggest box with Ctrl + Space
2 parents 1b84906 + 5c13288 commit dc8062b

File tree

7 files changed

+375
-80
lines changed

7 files changed

+375
-80
lines changed

‎extensions/ql-vscode/src/view/common/SuggestBox/SuggestBox.tsx

+3-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { findMatchingOptions } from "./options";
2020
import { SuggestBoxItem } from "./SuggestBoxItem";
2121
import { LabelText } from "./LabelText";
2222
import type { Diagnostic } from "./diagnostics";
23+
import { useOpenKey } from "./useOpenKey";
2324

2425
const Input = styled(VSCodeTextField)<{ $error: boolean }>`
2526
width: 430px;
@@ -152,6 +153,7 @@ export const SuggestBox = <
152153
const focus = useFocus(context);
153154
const role = useRole(context, { role: "listbox" });
154155
const dismiss = useDismiss(context);
156+
const openKey = useOpenKey(context);
155157
const listNav = useListNavigation(context, {
156158
listRef,
157159
activeIndex,
@@ -161,7 +163,7 @@ export const SuggestBox = <
161163
});
162164

163165
const { getReferenceProps, getFloatingProps, getItemProps } = useInteractions(
164-
[focus, role, dismiss, listNav],
166+
[focus, role, dismiss, openKey, listNav],
165167
);
166168

167169
const handleInput = useCallback(

‎extensions/ql-vscode/src/view/common/SuggestBox/__tests__/useEffectEvent.test.ts

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { renderHook } from "@testing-library/react";
2+
import { useEffectEvent } from "../useEffectEvent";
3+
4+
describe("useEffectEvent", () => {
5+
it("does not change reference when changing the callback function", () => {
6+
const callback1 = jest.fn();
7+
const callback2 = jest.fn();
8+
9+
const { result, rerender } = renderHook(
10+
(callback) => useEffectEvent(callback),
11+
{
12+
initialProps: callback1,
13+
},
14+
);
15+
16+
const callbackResult = result.current;
17+
18+
rerender();
19+
20+
expect(result.current).toBe(callbackResult);
21+
22+
rerender(callback2);
23+
24+
expect(result.current).toBe(callbackResult);
25+
});
26+
});

‎extensions/ql-vscode/src/view/common/SuggestBox/__tests__/useOpenKey.test.ts

+194
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
import type { KeyboardEvent } from "react";
2+
import { renderHook } from "@testing-library/react";
3+
import type { FloatingContext } from "@floating-ui/react";
4+
import { mockedObject } from "../../../../../test/mocked-object";
5+
import { useOpenKey } from "../useOpenKey";
6+
7+
describe("useOpenKey", () => {
8+
const onOpenChange = jest.fn();
9+
10+
beforeEach(() => {
11+
onOpenChange.mockReset();
12+
});
13+
14+
const render = ({ open }: { open: boolean }) => {
15+
const context = mockedObject<FloatingContext>({
16+
open,
17+
onOpenChange,
18+
});
19+
20+
const { result } = renderHook(() => useOpenKey(context));
21+
22+
expect(result.current).toEqual({
23+
reference: {
24+
onKeyDown: expect.any(Function),
25+
},
26+
});
27+
28+
const onKeyDown = result.current.reference?.onKeyDown;
29+
if (!onKeyDown) {
30+
throw new Error("onKeyDown is undefined");
31+
}
32+
33+
return {
34+
onKeyDown,
35+
};
36+
};
37+
38+
const mockKeyboardEvent = ({
39+
key = "",
40+
altKey = false,
41+
ctrlKey = false,
42+
metaKey = false,
43+
shiftKey = false,
44+
preventDefault = jest.fn(),
45+
}: Partial<KeyboardEvent>) =>
46+
mockedObject<KeyboardEvent>({
47+
key,
48+
altKey,
49+
ctrlKey,
50+
metaKey,
51+
shiftKey,
52+
preventDefault,
53+
});
54+
55+
const pressKey = (event: Parameters<typeof mockKeyboardEvent>[0]) => {
56+
const { onKeyDown } = render({
57+
open: false,
58+
});
59+
60+
const keyboardEvent = mockKeyboardEvent(event);
61+
62+
onKeyDown(keyboardEvent);
63+
64+
return {
65+
onKeyDown,
66+
keyboardEvent,
67+
};
68+
};
69+
70+
it("opens when pressing Ctrl + Space and it is closed", () => {
71+
const { keyboardEvent } = pressKey({
72+
key: " ",
73+
ctrlKey: true,
74+
});
75+
76+
expect(keyboardEvent.preventDefault).toHaveBeenCalledTimes(1);
77+
expect(onOpenChange).toHaveBeenCalledTimes(1);
78+
expect(onOpenChange).toHaveBeenCalledWith(true, keyboardEvent);
79+
});
80+
81+
it("does not open when pressing Ctrl + Space and it is open", () => {
82+
const { onKeyDown } = render({
83+
open: true,
84+
});
85+
86+
// Do not mock any properties to ensure that none of them are used.
87+
const keyboardEvent = mockedObject<KeyboardEvent>({});
88+
89+
onKeyDown(keyboardEvent);
90+
91+
expect(onOpenChange).not.toHaveBeenCalled();
92+
});
93+
94+
it("does not open when pressing Cmd + Space", () => {
95+
pressKey({
96+
key: " ",
97+
metaKey: true,
98+
});
99+
100+
expect(onOpenChange).not.toHaveBeenCalled();
101+
});
102+
103+
it("does not open when pressing Ctrl + Shift + Space", () => {
104+
pressKey({
105+
key: " ",
106+
ctrlKey: true,
107+
shiftKey: true,
108+
});
109+
110+
expect(onOpenChange).not.toHaveBeenCalled();
111+
});
112+
113+
it("does not open when pressing Ctrl + Alt + Space", () => {
114+
pressKey({
115+
key: " ",
116+
ctrlKey: true,
117+
altKey: true,
118+
});
119+
120+
expect(onOpenChange).not.toHaveBeenCalled();
121+
});
122+
123+
it("does not open when pressing Ctrl + Cmd + Space", () => {
124+
pressKey({
125+
key: " ",
126+
ctrlKey: true,
127+
metaKey: true,
128+
});
129+
130+
expect(onOpenChange).not.toHaveBeenCalled();
131+
});
132+
133+
it("does not open when pressing Ctrl + Shift + Alt + Space", () => {
134+
pressKey({
135+
key: " ",
136+
ctrlKey: true,
137+
altKey: true,
138+
shiftKey: true,
139+
});
140+
141+
expect(onOpenChange).not.toHaveBeenCalled();
142+
});
143+
144+
it("does not open when pressing Space", () => {
145+
pressKey({
146+
key: " ",
147+
});
148+
149+
expect(onOpenChange).not.toHaveBeenCalled();
150+
});
151+
152+
it("does not open when pressing Ctrl + Tab", () => {
153+
pressKey({
154+
key: "Tab",
155+
ctrlKey: true,
156+
});
157+
158+
expect(onOpenChange).not.toHaveBeenCalled();
159+
});
160+
161+
it("does not open when pressing Ctrl + a letter", () => {
162+
pressKey({
163+
key: "a",
164+
ctrlKey: true,
165+
});
166+
167+
expect(onOpenChange).not.toHaveBeenCalled();
168+
});
169+
170+
it("does not change reference when the context changes", () => {
171+
const context = mockedObject<FloatingContext>({
172+
open: false,
173+
onOpenChange,
174+
});
175+
176+
const { result, rerender } = renderHook((context) => useOpenKey(context), {
177+
initialProps: context,
178+
});
179+
180+
const firstOnKeyDown = result.current.reference?.onKeyDown;
181+
expect(firstOnKeyDown).toBeDefined();
182+
183+
rerender(
184+
mockedObject<FloatingContext>({
185+
open: true,
186+
onOpenChange: jest.fn(),
187+
}),
188+
);
189+
190+
const secondOnKeyDown = result.current.reference?.onKeyDown;
191+
// test that useEffectEvent is used correctly and the reference doesn't change
192+
expect(secondOnKeyDown).toBe(firstOnKeyDown);
193+
});
194+
});

‎extensions/ql-vscode/src/view/common/SuggestBox/useEffectEvent.ts

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { useCallback, useInsertionEffect, useRef } from "react";
2+
3+
// Copy of https://github.com/floating-ui/floating-ui/blob/5d025db1167e0bc13e7d386d7df2498b9edf2f8a/packages/react/src/hooks/utils/useEffectEvent.ts
4+
// since it's not exported
5+
6+
/**
7+
* Creates a reference to a callback that will never change in value. This will ensure that when a callback gets changed,
8+
* no new reference to the callback will be created and thus no unnecessary re-renders will be triggered.
9+
*
10+
* @param callback The callback to call when the event is triggered.
11+
*/
12+
export function useEffectEvent<T extends (...args: any[]) => any>(callback: T) {
13+
const ref = useRef<T>(callback);
14+
15+
useInsertionEffect(() => {
16+
ref.current = callback;
17+
});
18+
19+
return useCallback<(...args: Parameters<T>) => ReturnType<T>>(
20+
(...args) => ref.current(...args),
21+
[],
22+
) as T;
23+
}

‎extensions/ql-vscode/src/view/common/SuggestBox/useOpenKey.ts

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import type { KeyboardEvent } from "react";
2+
import { useMemo } from "react";
3+
import type {
4+
ElementProps,
5+
FloatingContext,
6+
ReferenceType,
7+
} from "@floating-ui/react";
8+
import { isReactEvent } from "@floating-ui/react/utils";
9+
import { useEffectEvent } from "./useEffectEvent";
10+
11+
/**
12+
* Open the floating element when Ctrl+Space is pressed.
13+
*/
14+
export const useOpenKey = <RT extends ReferenceType = ReferenceType>(
15+
context: FloatingContext<RT>,
16+
): ElementProps => {
17+
const { open, onOpenChange } = context;
18+
19+
const openOnOpenKey = useEffectEvent(
20+
(event: KeyboardEvent<Element> | KeyboardEvent) => {
21+
if (open) {
22+
return;
23+
}
24+
25+
if (
26+
event.key === " " &&
27+
event.ctrlKey &&
28+
!event.altKey &&
29+
!event.metaKey &&
30+
!event.shiftKey
31+
) {
32+
event.preventDefault();
33+
onOpenChange(true, isReactEvent(event) ? event.nativeEvent : event);
34+
}
35+
},
36+
);
37+
38+
return useMemo((): ElementProps => {
39+
return {
40+
reference: {
41+
onKeyDown: openOnOpenKey,
42+
},
43+
};
44+
}, [openOnOpenKey]);
45+
};

‎extensions/ql-vscode/test/mocked-object.ts

+80
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
export type DeepPartial<T> = T extends object
2+
? {
3+
[P in keyof T]?: DeepPartial<T[P]>;
4+
}
5+
: T;
6+
7+
type DynamicProperties<T extends object> = {
8+
[P in keyof T]?: () => T[P];
9+
};
10+
11+
type MockedObjectOptions<T extends object> = {
12+
/**
13+
* Properties for which the given method should be called when accessed.
14+
* The method should return the value to be returned when the property is accessed.
15+
* Methods which are explicitly defined in `methods` will take precedence over
16+
* dynamic properties.
17+
*/
18+
dynamicProperties?: DynamicProperties<T>;
19+
};
20+
21+
export function mockedObject<T extends object>(
22+
props: DeepPartial<T>,
23+
{ dynamicProperties }: MockedObjectOptions<T> = {},
24+
): T {
25+
return new Proxy<T>({} as unknown as T, {
26+
get: (_target, prop) => {
27+
if (prop in props) {
28+
return (props as any)[prop];
29+
}
30+
if (dynamicProperties && prop in dynamicProperties) {
31+
return (dynamicProperties as any)[prop]();
32+
}
33+
34+
// The `then` method is accessed by `Promise.resolve` to check if the object is a thenable.
35+
// We don't want to throw an error when this happens.
36+
if (prop === "then") {
37+
return undefined;
38+
}
39+
40+
// The `asymmetricMatch` is accessed by jest to check if the object is a matcher.
41+
// We don't want to throw an error when this happens.
42+
if (prop === "asymmetricMatch") {
43+
return undefined;
44+
}
45+
46+
// The `Symbol.iterator` is accessed by jest to check if the object is iterable.
47+
// We don't want to throw an error when this happens.
48+
if (prop === Symbol.iterator) {
49+
return undefined;
50+
}
51+
52+
// The `$$typeof` is accessed by jest to check if the object is a React element.
53+
// We don't want to throw an error when this happens.
54+
if (prop === "$$typeof") {
55+
return undefined;
56+
}
57+
58+
// The `nodeType` and `tagName` are accessed by jest to check if the object is a DOM node.
59+
// We don't want to throw an error when this happens.
60+
if (prop === "nodeType" || prop === "tagName") {
61+
return undefined;
62+
}
63+
64+
// The `@@__IMMUTABLE_ITERABLE__@@` and variants are accessed by jest to check if the object is an
65+
// immutable object (from Immutable.js).
66+
// We don't want to throw an error when this happens.
67+
if (prop.toString().startsWith("@@__IMMUTABLE_")) {
68+
return undefined;
69+
}
70+
71+
// The `Symbol.toStringTag` is accessed by jest.
72+
// We don't want to throw an error when this happens.
73+
if (prop === Symbol.toStringTag) {
74+
return "MockedObject";
75+
}
76+
77+
throw new Error(`Method ${String(prop)} not mocked`);
78+
},
79+
});
80+
}

0 commit comments

Comments
 (0)