Commit fe3c9d4e authored by Robert Knight's avatar Robert Knight

Focus first item in menu when opened by any input method

Previously the `Menu` component would focus the first item in the list if it was
opened by pressing Enter or Space but not if it was clicked. This meant that it
was not possible to mix input modalities (open menu with mouse, navigate using
keyboard) easily when using the menu.  More importantly if the menu was opened
using a screen reader shortcut (eg. VoiceOver Key + Space on macOS) per the
screen reader's verbal instructions, the focus didn't change. My understanding
is that SR shortcuts generally simulate mouse rather than keyboard input.
parent 12def07b
......@@ -55,7 +55,6 @@ export default function Menu({
title,
}) {
const [isOpen, setOpen] = useState(defaultOpen);
const [openedByKeyboard, setOpenedByKeyboard] = useState(false);
// Notify parent when menu is opened or closed.
const wasOpen = useRef(isOpen);
......@@ -81,17 +80,6 @@ export default function Menu({
event.preventDefault();
return;
}
// State variable so we know to set focus() on the first item when opened
// via the keyboard. Note, when opening the menu via keyboard by pressing
// enter or space, a simulated MouseEvent is created with a type value of
// "click". We also know this is not a mouseup event because that condition
// is checked above.
if (!isOpen && event.type === 'click') {
setOpenedByKeyboard(true);
} else {
setOpenedByKeyboard(false);
}
setOpen(!isOpen);
};
const closeMenu = useCallback(() => setOpen(false), [setOpen]);
......@@ -179,7 +167,7 @@ export default function Menu({
onClick={closeMenu}
onKeyDown={handleMenuKeyDown}
>
<MenuKeyboardNavigation visible={openedByKeyboard}>
<MenuKeyboardNavigation visible={true}>
{children}
</MenuKeyboardNavigation>
</div>
......
......@@ -50,7 +50,6 @@ describe('Menu', () => {
const wrapper = createMenu();
assert.isFalse(isOpen(wrapper));
wrapper.find('button').simulate('click');
assert.isTrue(wrapper.find('MenuKeyboardNavigation').prop('visible'));
assert.isTrue(isOpen(wrapper));
wrapper.find('button').simulate('click');
assert.isFalse(isOpen(wrapper));
......@@ -60,7 +59,6 @@ describe('Menu', () => {
const onOpenChanged = sinon.stub();
const wrapper = createMenu({ onOpenChanged });
wrapper.find('button').simulate('click');
assert.isTrue(wrapper.find('MenuKeyboardNavigation').prop('visible'));
assert.calledWith(onOpenChanged, true);
wrapper.find('button').simulate('click');
assert.calledWith(onOpenChanged, false);
......@@ -71,7 +69,6 @@ describe('Menu', () => {
assert.isFalse(isOpen(wrapper));
wrapper.find('button').simulate('mousedown');
assert.isFalse(wrapper.find('MenuKeyboardNavigation').prop('visible'));
// Make sure the follow-up click doesn't close the menu.
wrapper.find('button').simulate('click');
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment