lime-elements icon indicating copy to clipboard operation
lime-elements copied to clipboard

limel-menu: Improve keyboard navigation. Do not lose focus when closing menu.

Open adrianschmidt opened this issue 3 years ago • 1 comments

I was unfortunately unable to write a test for this :disappointed: I could not find a way to get the current activeElement, it was always set to undefined even when trying to manually invoke .focus() from within the test

Originally posted by @jgroth in https://github.com/Lundalogik/lime-elements/pull/1389#r730832886activeElement seems to always be null when I open the menu while testing manually too, whether I open it by clicking or by keyboard, at least in Chrome on macOS.

I'm not quite sure I understand what we're trying to do here? menuElements[selectedIndex]?.focus(); leads me to believe we want to set focus on a menu item, but I'm unsure why we need to first blur some other element?

Also, we need to handle the focus when closing the menu. Currently, focus is lost, and the tab flow "starts over" at the browser's address bar.

Originally posted by @adrianschmidt in https://github.com/Lundalogik/lime-elements/pull/1389#r733579463

I'm not sure if we actually need to move focus around between the list items for the menu to be accessible. It might be possible to keep focus on the trigger, and simply use aria labels to indicate which option is currently highlighted. I'm not sure. It would make focus handling on exit easier (or even a non-issue), but it might make other things more complicated instead.

Originally posted by @adrianschmidt in https://github.com/Lundalogik/lime-elements/pull/1389#r733585977

Yeah that's what the idea is. When the menu is opened the first (or selected) item will receive focus. When I tested it in the documentation the only way I could get it to work after the menu had been closed once was to first call blur()

Also, we need to handle the focus when closing the menu. Currently, focus is lost, and the tab flow "starts over" at the browser's address bar.

What item should get focus when it is closed? Maybe the trigger element? :thinking: Maybe fixing that will also fix the issue I had with blur

Originally posted by @jgroth in https://github.com/Lundalogik/lime-elements/pull/1389#r733587987

Yeah, the trigger element, at least if closed by pressing enter or escape. If tabbing out of it, preferably the next or previous component would get focus, but if we set focus back to the trigger, I think that's ok. It's much better than breaking the focus flow.

I'll open a new issue for these things though, as we talked about in the Slack call. This is definitely progress compared to what we currently have, so no reason to let "perfect" stand in the way of "better" 🙂

Originally posted by @adrianschmidt in https://github.com/Lundalogik/lime-elements/pull/1389#r734304330

adrianschmidt avatar Oct 22 '21 07:10 adrianschmidt