Skip to content

Commit 1fdc32a

Browse files
jquensejquense
authored
Fix dropdown close (#43)
* feat: add useClickOutside * fix: dropdown escape logic when in inputs * test: add test * fix name
1 parent 4551ef5 commit 1fdc32a

File tree

5 files changed

+164
-90
lines changed

5 files changed

+164
-90
lines changed

‎src/Dropdown.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,12 @@ function Dropdown({
273273
// Second only to https://github.com/twbs/bootstrap/blob/8cfbf6933b8a0146ac3fbc369f19e520bd1ebdac/js/src/dropdown.js#L400
274274
// in inscrutability
275275
const isInput = /input|textarea/i.test(target.tagName);
276-
if (isInput && (key === ' ' || (key !== 'Escape' && fromMenu))) {
276+
if (
277+
isInput &&
278+
(key === ' ' ||
279+
(key !== 'Escape' && fromMenu) ||
280+
(key === 'Escape' && (target as HTMLInputElement).type === 'search'))
281+
) {
277282
return;
278283
}
279284

‎src/DropdownMenu.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import usePopper, {
88
Offset,
99
UsePopperState,
1010
} from './usePopper';
11-
import useRootClose, { RootCloseOptions } from './useRootClose';
11+
import useClickOutside, { ClickOutsideOptions } from './useClickOutside';
1212
import mergeOptionsWithPopperConfig from './mergeOptionsWithPopperConfig';
1313

1414
export interface UseDropdownMenuOptions {
@@ -61,7 +61,7 @@ export interface UseDropdownMenuOptions {
6161
/**
6262
* Override the default event used by RootCloseWrapper.
6363
*/
64-
rootCloseEvent?: RootCloseOptions['clickTrigger'];
64+
rootCloseEvent?: ClickOutsideOptions['clickTrigger'];
6565

6666
/**
6767
* A set of popper options and props passed directly to react-popper's Popper component.
@@ -168,7 +168,7 @@ export function useDropdownMenu(options: UseDropdownMenuOptions = {}) {
168168
: {},
169169
};
170170

171-
useRootClose(menuElement, handleClose, {
171+
useClickOutside(menuElement, handleClose, {
172172
clickTrigger: rootCloseEvent,
173173
disabled: !show,
174174
});

‎src/useClickOutside.ts‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import contains from 'dom-helpers/contains';
2+
import listen from 'dom-helpers/listen';
3+
import ownerDocument from 'dom-helpers/ownerDocument';
4+
import { useCallback, useEffect, useRef } from 'react';
5+
6+
import useEventCallback from '@restart/hooks/useEventCallback';
7+
import warning from 'warning';
8+
9+
const noop = () => {};
10+
11+
export type MouseEvents = {
12+
[K in keyof GlobalEventHandlersEventMap]: GlobalEventHandlersEventMap[K] extends MouseEvent
13+
? K
14+
: never;
15+
}[keyof GlobalEventHandlersEventMap];
16+
17+
function isLeftClickEvent(event: MouseEvent) {
18+
return event.button === 0;
19+
}
20+
21+
function isModifiedEvent(event: MouseEvent) {
22+
return !!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey);
23+
}
24+
25+
export const getRefTarget = (
26+
ref: React.RefObject<Element> | Element | null | undefined,
27+
) => ref && ('current' in ref ? ref.current : ref);
28+
29+
export interface ClickOutsideOptions {
30+
disabled?: boolean;
31+
clickTrigger?: MouseEvents;
32+
}
33+
34+
/**
35+
* The `useClickOutside` hook registers your callback on the document that fires
36+
* when a pointer event is registered outside of the provided ref or element.
37+
*
38+
* @param {Ref<HTMLElement>| HTMLElement} ref The element boundary
39+
* @param {function} onClickOutside
40+
* @param {object=} options
41+
* @param {boolean=} options.disabled
42+
* @param {string=} options.clickTrigger The DOM event name (click, mousedown, etc) to attach listeners on
43+
*/
44+
function useClickOutside(
45+
ref: React.RefObject<Element> | Element | null | undefined,
46+
onClickOutside: (e: Event) => void = noop,
47+
{ disabled, clickTrigger = 'click' }: ClickOutsideOptions = {},
48+
) {
49+
const preventMouseClickOutsideRef = useRef(false);
50+
51+
const handleMouseCapture = useCallback(
52+
(e) => {
53+
const currentTarget = getRefTarget(ref);
54+
55+
warning(
56+
!!currentTarget,
57+
'ClickOutside captured a close event but does not have a ref to compare it to. ' +
58+
'useClickOutside(), should be passed a ref that resolves to a DOM node',
59+
);
60+
61+
preventMouseClickOutsideRef.current =
62+
!currentTarget ||
63+
isModifiedEvent(e) ||
64+
!isLeftClickEvent(e) ||
65+
!!contains(currentTarget, e.target);
66+
},
67+
[ref],
68+
);
69+
70+
const handleMouse = useEventCallback((e: MouseEvent) => {
71+
if (!preventMouseClickOutsideRef.current) {
72+
onClickOutside(e);
73+
}
74+
});
75+
76+
useEffect(() => {
77+
if (disabled || ref == null) return undefined;
78+
79+
const doc = ownerDocument(getRefTarget(ref)!);
80+
81+
// Store the current event to avoid triggering handlers immediately
82+
// https://github.com/facebook/react/issues/20074
83+
let currentEvent = (doc.defaultView || window).event;
84+
85+
// Use capture for this listener so it fires before React's listener, to
86+
// avoid false positives in the contains() check below if the target DOM
87+
// element is removed in the React mouse callback.
88+
const removeMouseCaptureListener = listen(
89+
doc as any,
90+
clickTrigger,
91+
handleMouseCapture,
92+
true,
93+
);
94+
95+
const removeMouseListener = listen(doc as any, clickTrigger, (e) => {
96+
// skip if this event is the same as the one running when we added the handlers
97+
if (e === currentEvent) {
98+
currentEvent = undefined;
99+
return;
100+
}
101+
handleMouse(e);
102+
});
103+
104+
let mobileSafariHackListeners = [] as Array<() => void>;
105+
if ('ontouchstart' in doc.documentElement) {
106+
mobileSafariHackListeners = [].slice
107+
.call(doc.body.children)
108+
.map((el) => listen(el, 'mousemove', noop));
109+
}
110+
111+
return () => {
112+
removeMouseCaptureListener();
113+
removeMouseListener();
114+
mobileSafariHackListeners.forEach((remove) => remove());
115+
};
116+
}, [ref, disabled, clickTrigger, handleMouseCapture, handleMouse]);
117+
}
118+
119+
export default useClickOutside;

‎src/useRootClose.ts‎

Lines changed: 10 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,20 @@
1-
import contains from 'dom-helpers/contains';
21
import listen from 'dom-helpers/listen';
32
import ownerDocument from 'dom-helpers/ownerDocument';
4-
import { useCallback, useEffect, useRef } from 'react';
3+
import { useEffect } from 'react';
54

65
import useEventCallback from '@restart/hooks/useEventCallback';
7-
import warning from 'warning';
6+
import useClickOutside, {
7+
ClickOutsideOptions,
8+
getRefTarget,
9+
} from './useClickOutside';
810

911
const escapeKeyCode = 27;
1012
const noop = () => {};
1113

12-
export type MouseEvents = {
13-
[K in keyof GlobalEventHandlersEventMap]: GlobalEventHandlersEventMap[K] extends MouseEvent
14-
? K
15-
: never;
16-
}[keyof GlobalEventHandlersEventMap];
17-
18-
function isLeftClickEvent(event: MouseEvent) {
19-
return event.button === 0;
20-
}
21-
22-
function isModifiedEvent(event: MouseEvent) {
23-
return !!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey);
24-
}
25-
26-
const getRefTarget = (
27-
ref: React.RefObject<Element> | Element | null | undefined,
28-
) => ref && ('current' in ref ? ref.current : ref);
29-
30-
export interface RootCloseOptions {
14+
export interface RootCloseOptions extends ClickOutsideOptions {
3115
disabled?: boolean;
32-
clickTrigger?: MouseEvents;
3316
}
17+
3418
/**
3519
* The `useRootClose` hook registers your callback on the document
3620
* when rendered. Powers the `<Overlay/>` component. This is used achieve modal
@@ -46,35 +30,11 @@ export interface RootCloseOptions {
4630
function useRootClose(
4731
ref: React.RefObject<Element> | Element | null | undefined,
4832
onRootClose: (e: Event) => void,
49-
{ disabled, clickTrigger = 'click' }: RootCloseOptions = {},
33+
{ disabled, clickTrigger }: RootCloseOptions = {},
5034
) {
51-
const preventMouseRootCloseRef = useRef(false);
5235
const onClose = onRootClose || noop;
5336

54-
const handleMouseCapture = useCallback(
55-
(e) => {
56-
const currentTarget = getRefTarget(ref);
57-
58-
warning(
59-
!!currentTarget,
60-
'RootClose captured a close event but does not have a ref to compare it to. ' +
61-
'useRootClose(), should be passed a ref that resolves to a DOM node',
62-
);
63-
64-
preventMouseRootCloseRef.current =
65-
!currentTarget ||
66-
isModifiedEvent(e) ||
67-
!isLeftClickEvent(e) ||
68-
!!contains(currentTarget, e.target);
69-
},
70-
[ref],
71-
);
72-
73-
const handleMouse = useEventCallback((e: MouseEvent) => {
74-
if (!preventMouseRootCloseRef.current) {
75-
onClose(e);
76-
}
77-
});
37+
useClickOutside(ref, onClose, { disabled, clickTrigger });
7838

7939
const handleKeyUp = useEventCallback((e: KeyboardEvent) => {
8040
if (e.keyCode === escapeKeyCode) {
@@ -91,25 +51,6 @@ function useRootClose(
9151
// https://github.com/facebook/react/issues/20074
9252
let currentEvent = (doc.defaultView || window).event;
9353

94-
// Use capture for this listener so it fires before React's listener, to
95-
// avoid false positives in the contains() check below if the target DOM
96-
// element is removed in the React mouse callback.
97-
const removeMouseCaptureListener = listen(
98-
doc as any,
99-
clickTrigger,
100-
handleMouseCapture,
101-
true,
102-
);
103-
104-
const removeMouseListener = listen(doc as any, clickTrigger, (e) => {
105-
// skip if this event is the same as the one running when we added the handlers
106-
if (e === currentEvent) {
107-
currentEvent = undefined;
108-
return;
109-
}
110-
handleMouse(e);
111-
});
112-
11354
const removeKeyupListener = listen(doc as any, 'keyup', (e) => {
11455
// skip if this event is the same as the one running when we added the handlers
11556
if (e === currentEvent) {
@@ -119,27 +60,10 @@ function useRootClose(
11960
handleKeyUp(e);
12061
});
12162

122-
let mobileSafariHackListeners = [] as Array<() => void>;
123-
if ('ontouchstart' in doc.documentElement) {
124-
mobileSafariHackListeners = [].slice
125-
.call(doc.body.children)
126-
.map((el) => listen(el, 'mousemove', noop));
127-
}
128-
12963
return () => {
130-
removeMouseCaptureListener();
131-
removeMouseListener();
13264
removeKeyupListener();
133-
mobileSafariHackListeners.forEach((remove) => remove());
13465
};
135-
}, [
136-
ref,
137-
disabled,
138-
clickTrigger,
139-
handleMouseCapture,
140-
handleMouse,
141-
handleKeyUp,
142-
]);
66+
}, [ref, disabled, handleKeyUp]);
14367
}
14468

14569
export default useRootClose;

‎test/DropdownSpec.js‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,32 @@ describe('<Dropdown>', () => {
350350
document.activeElement.should.equal(root.getByText('Toggle'));
351351
});
352352

353+
it('when open and a search input is focused and the key "Escape" is pressed the menu stays open', () => {
354+
const toggleSpy = sinon.spy();
355+
const root = render(
356+
<Dropdown defaultShow onToggle={toggleSpy}>
357+
<Toggle key="toggle">Toggle</Toggle>,
358+
<Menu key="menu">
359+
<input type="search" data-testid="input" />
360+
</Menu>
361+
</Dropdown>,
362+
{
363+
container: focusableContainer,
364+
},
365+
);
366+
367+
const input = root.getByTestId('input');
368+
369+
input.focus();
370+
document.activeElement.should.equal(input);
371+
372+
fireEvent.keyDown(input, { key: 'Escape' });
373+
374+
document.activeElement.should.equal(input);
375+
376+
toggleSpy.should.not.be.called;
377+
});
378+
353379
it('when open and the key "tab" is pressed the menu is closed and focus is progress to the next focusable element', () => {
354380
const root = render(
355381
<div>

0 commit comments

Comments
 (0)