Reader context menu keyboard navigation #139
Conversation
abaevbog
force-pushed
the
context_menu_keyboard_nav
branch
from
0ff54a7 to
4bdec1d
Compare
|
Sorry, I forgot that context menu navigation was working in the initial Reader 2.0 version. Let's see if we can add those extra features from this PR. |
|
Yeah, I'll revise this to just keep the last bullet point from the initial description
As well as that glitch fix I ran into |
- added keydown listener to contextmenu to select a menuitem by typing, similar to native menus. After something is typed, find buttons with text that begins with the input. If there is only one match, it is clicked. If there are multiple, the first one is focused. The input counter is reset after 2 seconds of not typing. - listen to even candidates in focus-manager and dispatch copy of keydown events from there to context-menu, in case one types when the menu itself is not focused. Also, fixed an encountered glitch where clicking inside of the rendered reader content when the context menu is open would make the annotation un-selectable via tab.
abaevbog
force-pushed
the
context_menu_keyboard_nav
branch
from
4bdec1d to
3eb0441
Compare
|
I just revised this to only keep the menuitem selection by typing. |
| // If there is only one candidate, click it right away | ||
| if (candidates.length == 1) { | ||
| candidates[0].click(); | ||
| } |
There was a problem hiding this comment.
But where is this concept from? It feels pretty unexpected that pressing a letter immediately activates an action.
There was a problem hiding this comment.
I thought it would be nice to skip an extra click or keypress if this is the only matching option. But you're right, the native menu does not do that, so it's probably best to stick to just focusing the menu item.
| function handlePointerDown(event) { | ||
| // Clicking on the rendered content when a contextmenu is open will | ||
| // lead to pointerup event not firing and the annotation becoming not-selectable | ||
| // via keyboard until it is clicked. | ||
| if (event.target.classList.contains("context-menu-overlay")) return; |
There was a problem hiding this comment.
I'm trying to reproduce the mentioned issue. Could you help me with the steps to reproduce it?
There was a problem hiding this comment.
Yeah, it actually is a bit harder to reproduce now than before. The recent fix to context menu focus regression must have helped.
I did get to reproduce it with an unloaded reader tab. After it loads, click on the annotation ... to open the context menu, then click on the rendered PDF content, and try to navigate to the sidebar with tab. It gets skipped for me. Clicking around resolves the issue, and on subsequent attempts, it doesn't seem to happen again.
Screen.Recording.2024-10-10.at.12.27.36.PM.mov
There was a problem hiding this comment.
It should be fixed in 97dd5ab. Thanks for helping to reproduce it.
There was a problem hiding this comment.
Yeah, it's fixed now. That was definitely a less-roundabout way to do it!
I removed this changed from here.
|
Oh, on a separate note, I just noticed that when a context-menu is opened, subsequent Escape keypress not only closes the contextmenu but also re-focuses the rendered content iframe. We probably want to treat Escape as a special case here so it only closes the contextmenu and refocuses the context-menu button? |
We can create a separate issue for that. |
|
Ok, looks good. We can merge. |
Fixes: zotero/zotero#4681
Also, fixed an encountered glitch where clicking inside of the rendered reader content when the context menu is open would make the annotation un-selectable via tab.
Currently, getting into the menu or navigating it with keyboard is not possible. For comparison, this is the updated behavior. In the end, i type "gre" to get the "Green" option selected.
reader.context.menu.keyboard.mov