Fix tab trap in menubar

[gabalafou]

There are a few tab traps in the JupyterLab UI. The worst offender is the top menu bar. The code change in this pull request is the most straightforward way to fix the menu bar tab trap in JupyterLab.

In terms of backwards compatibility, I have a hard time imagining that downstream dependents on Lumino would rely on the tab key event getting caught and swallowed (event.preventDefault + event.stopPropagation) by the Lumino menu bar, but it is something to consider.

[gabalafou]

Note that this PR does not fix all of the tab traps in JupyterLab. There are others, these two in particular:

• when a notebook is open, code cells become tab traps (technically, you can ESC-key out of the code cell and tab backwards through the UI, but I couldn't figure out a way to go forward past the code cell)
• opening the command-line terminal also creates a tab trap unless you know to press CTRL + D

[gabalafou]

Testing

Testing this change against the JupyterLab UI is a little bit tricky right now.

When I tried to link @lumino/widgets, I got errors when trying to do a dev build of the JupyterLab frontend code. I think that's because the JupyterLab frontend code has not yet been updated to be compatible with the latest Lumino code.

So I had to cherry pick the commit from this PR onto a branch based on the Lumino 1.x branch, and then do the linking.

In terms of the command line, it looks like the following.

First, with the Lumino repo as your current working directory:

git checkout -b no-tab-traps 1.x
git cherry-pick 911f4b067f27d88be3d9707e33ad4eac746009b7
yarn 
yarn build
cd packages/widgets
yarn link

Then, with the JupyterLab repo as current working directory (and assuming you have already pulled latest changes and installed everything):

jlpm link @lumino/widgets
jlpm build
jupyter lab --dev-mode

[fcollonval]

Just adding more information in the code comments. Otherwise it looks good to me.

[fcollonval]

Note that this PR does not fix all of the tab traps in JupyterLab. There are others, these two in particular:

* when a notebook is open, code cells become tab traps (technically, you can `ESC`-key out of the code cell and tab backwards through the UI, but I couldn't figure out a way to go forward past the code cell)

* opening the command-line terminal also creates a tab trap unless you know to press `CTRL` + `D`

Those are gonna be tricky as tab is meaningful for code editors and terminals. It may be interesting to see how vscode in its web version (aka vscode.dev or github.dev) deal with that.

just tested - terminals are not available on the online version (no server to back it up) and editor are also a tab trap.

Maybe there is an issue on JupyterLab to track this.

[fcollonval]

Thanks @gabalafou

[gabalafou]

I wonder if the logic here is sound.

Originally this handler only called preventDefault and stopPropagation if it actually handled the key. But then the handler was rewritten to handle mnemonics. I lightly suspect that the decision to call prevent and stop on all keys was done to make the handler more readable, without too much thought given to possible negative consequences.

I wrote this PR conservatively, meaning I kept everything else about this function the same EXCEPT when the tab key is pressed. But I guess it's worth asking whether the handler should be refactored to only intercept key events that it actually does something with.

I could imagine a handler that looks more like the following:

keyDown() {
  let kc = event.keyCode;
  let endEvent = () => { 
    event.preventDefault();
    event.stopPropagation();
  };
  // Enter, Up Arrow, Down Arrow
  if (kc === 13 || kc === 38 || kc === 40) {
    endEvent();
    this.openActiveMenu();
    return;
  }
  // ...
}

In other words, it would go back to looking like the original implementation.

[afshin]

Let's pursue this train of thought in another PR if necessary so we can merge this one and iterate.

[afshin]

Thank you!

[fcollonval]

@meeseeksdev please backport to 1.x