Skip to content

Commit b4ea62f

Browse files
nrps9909nrps9909
authored
fix: focus wrapped dropdown menus (#268)
* fix: focus wrapped dropdown menus * fix: preserve optional overlay refs * test: preserve wrapped menu autoFocus scroll options * fix: try tab target when wrapped menu cannot receive focus
1 parent 768e244 commit b4ea62f

3 files changed

Lines changed: 152 additions & 49 deletions

File tree

‎src/hooks/useAccessibility.ts‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,28 @@ export default function useAccessibility({
2828
};
2929

3030
const focusMenu = (options?: FocusOptions) => {
31-
if (overlayRef.current?.focus) {
32-
overlayRef.current.focus(options);
33-
focusMenuRef.current = true;
34-
return true;
31+
const overlay = overlayRef?.current;
32+
if (!overlay?.focus) {
33+
return false;
3534
}
36-
return false;
35+
36+
const activeElement = document.activeElement;
37+
overlay.focus(options);
38+
if (document.activeElement === activeElement) {
39+
for (const selector of ['[role="menu"]', '[tabindex]']) {
40+
const focusTarget = overlay.querySelector?.(
41+
selector,
42+
) as HTMLElement | null;
43+
focusTarget?.focus(options);
44+
if (document.activeElement !== activeElement) {
45+
break;
46+
}
47+
}
48+
}
49+
50+
const focused = document.activeElement !== activeElement;
51+
focusMenuRef.current = focused;
52+
return focused;
3753
};
3854

3955
const handleKeyDown = (event) => {

‎tests/basic.test.tsx‎

Lines changed: 101 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -492,13 +492,63 @@ describe('dropdown', () => {
492492

493493
// Focus menu with Tab
494494
window.dispatchEvent(new KeyboardEvent('keydown', { keyCode: 9 })); // Tab
495+
expect(document.activeElement).toHaveClass('rc-menu');
496+
fireEvent.keyDown(document.activeElement, {
497+
key: 'ArrowDown',
498+
keyCode: 40,
499+
});
500+
await sleep(50);
501+
expect(document.activeElement).toHaveTextContent('one');
495502

496503
// Close menu with Tab
497-
window.dispatchEvent(new KeyboardEvent('keydown', { keyCode: 9 })); // Tab
504+
fireEvent.keyDown(document.activeElement, { key: 'Tab', keyCode: 9 });
498505
await sleep(200);
499506
expect(document.activeElement.className).toContain('my-button');
500507
});
501508

509+
it.each(['missing', 'unfocusable'])(
510+
'focuses a tab target when the wrapped menu is %s',
511+
async (menuState) => {
512+
jest.useFakeTimers();
513+
try {
514+
const { container, baseElement } = render(
515+
<Dropdown
516+
trigger={['click']}
517+
overlay={
518+
<div>
519+
{menuState === 'unfocusable' && <div role="menu" />}
520+
<button tabIndex={0} className="custom-target">
521+
action
522+
</button>
523+
</div>
524+
}
525+
>
526+
<button className="my-button">open</button>
527+
</Dropdown>,
528+
);
529+
const trigger =
530+
container.querySelector<HTMLButtonElement>('.my-button');
531+
trigger.focus();
532+
fireEvent.click(trigger);
533+
await waitForTime();
534+
535+
const event = new KeyboardEvent('keydown', {
536+
keyCode: 9,
537+
cancelable: true,
538+
});
539+
act(() => {
540+
window.dispatchEvent(event);
541+
});
542+
expect(document.activeElement).toBe(
543+
baseElement.querySelector('.custom-target'),
544+
);
545+
expect(event.defaultPrevented).toBe(true);
546+
} finally {
547+
jest.useRealTimers();
548+
}
549+
},
550+
);
551+
502552
it('support Menu expandIcon', async () => {
503553
const props = {
504554
overlay: (
@@ -584,50 +634,57 @@ describe('dropdown', () => {
584634
jest.useRealTimers();
585635
});
586636

587-
it('should support autoFocus', async () => {
588-
jest.useFakeTimers();
589-
const focusSpy = jest.spyOn(HTMLElement.prototype, 'focus');
637+
it.each(['direct', 'wrapped'])(
638+
'should support autoFocus for a %s menu',
639+
async (mode) => {
640+
jest.useFakeTimers();
641+
const focusSpy = jest.spyOn(HTMLElement.prototype, 'focus');
590642

591-
try {
592-
const overlay = (
593-
<Menu>
594-
<MenuItem key="1">
595-
<span className="my-menuitem">one</span>
596-
</MenuItem>
597-
<MenuItem key="2">two</MenuItem>
598-
</Menu>
599-
);
600-
const { container } = render(
601-
<Dropdown autoFocus trigger={['click']} overlay={overlay}>
602-
<button className="my-button">open</button>
603-
</Dropdown>,
604-
);
605-
const trigger = container.querySelector('.my-button');
606-
607-
// Open menu
608-
fireEvent.click(trigger);
609-
610-
await waitForTime();
611-
612-
expect(
613-
container
614-
.querySelector('.rc-dropdown')
615-
.classList.contains('rc-dropdown-hidden'),
616-
).toBeFalsy();
617-
expect(document.activeElement.className).toContain('menu');
618-
expect(focusSpy).toHaveBeenCalledWith({ preventScroll: true });
619-
620-
// Close menu with Tab
621-
window.dispatchEvent(new KeyboardEvent('keydown', { keyCode: 9 })); // Tab
622-
623-
await waitForTime();
624-
625-
expect(document.activeElement.className).toContain('my-button');
626-
} finally {
627-
focusSpy.mockRestore();
628-
jest.useRealTimers();
629-
}
630-
});
643+
try {
644+
const overlay = (
645+
<Menu>
646+
<MenuItem key="1">
647+
<span className="my-menuitem">one</span>
648+
</MenuItem>
649+
<MenuItem key="2">two</MenuItem>
650+
</Menu>
651+
);
652+
const { container } = render(
653+
<Dropdown
654+
autoFocus
655+
trigger={['click']}
656+
overlay={mode === 'wrapped' ? <div>{overlay}</div> : overlay}
657+
>
658+
<button className="my-button">open</button>
659+
</Dropdown>,
660+
);
661+
const trigger = container.querySelector('.my-button');
662+
663+
// Open menu
664+
fireEvent.click(trigger);
665+
666+
await waitForTime();
667+
668+
expect(
669+
container
670+
.querySelector('.rc-dropdown')
671+
.classList.contains('rc-dropdown-hidden'),
672+
).toBeFalsy();
673+
expect(document.activeElement.className).toContain('menu');
674+
expect(focusSpy).toHaveBeenLastCalledWith({ preventScroll: true });
675+
676+
// Close menu with Tab
677+
window.dispatchEvent(new KeyboardEvent('keydown', { keyCode: 9 })); // Tab
678+
679+
await waitForTime();
680+
681+
expect(document.activeElement.className).toContain('my-button');
682+
} finally {
683+
focusSpy.mockRestore();
684+
jest.useRealTimers();
685+
}
686+
},
687+
);
631688

632689
it('children cannot be given ref should not throw', () => {
633690
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});

‎tests/useAccessibility.test.tsx‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { act, renderHook } from '@testing-library/react';
2+
import useAccessibility from '../src/hooks/useAccessibility';
3+
4+
it('closes without consuming Tab when the overlay ref is omitted', () => {
5+
const trigger = document.createElement('button');
6+
document.body.appendChild(trigger);
7+
const onOpenChange = jest.fn();
8+
const { unmount } = renderHook(() =>
9+
useAccessibility({
10+
open: true,
11+
triggerRef: { current: trigger },
12+
onOpenChange,
13+
}),
14+
);
15+
try {
16+
const event = new KeyboardEvent('keydown', {
17+
keyCode: 9,
18+
cancelable: true,
19+
});
20+
act(() => {
21+
window.dispatchEvent(event);
22+
});
23+
expect(event.defaultPrevented).toBe(false);
24+
expect(onOpenChange).toHaveBeenCalledWith(false);
25+
expect(document.activeElement).toBe(trigger);
26+
} finally {
27+
unmount();
28+
trigger.remove();
29+
}
30+
});

0 commit comments

Comments
 (0)