Fix tab trap notebook cells

[gabalafou]

Short summary

This PR is an attempt to solve an issue that makes it difficult to use JupyterLab with a keyboard only.

Before this PR, any code cell editor in an open notebook was a tap trap, also known as a keyboard trap or focus trap. After this PR, it is no longer a keyboard trap.

This PR introduces a noticeable visual change. Whenever a cell has browser focus, it is surrounded by a blue outline as shown in the following two screenshots:

Light Theme	Dark Theme

Long explanation

A quick reminder: in the JupyterLab codebase, the Notebook widget can be thought of as a collection of Cell widgets. Most Cell widgets contain an Editor widget (technically a CodeEditorWrapper widget). The relationship can be expressed as a CSS selector: .jp-Cell .jp-Editor.

This PR makes two significant changes to solve the notebook tab trap problem:

1. Removes tab-inclusive focusability from cell editors.
   • This means that after this PR, an end user can no longer enter cell editor fields using the tab key. They must use either the mouse or the Enter keyboard shortcut.
2. Removes tab-inclusive focusability from the notebook node in favor of adding tab-inclusive focusability to the active cell, plus adding tab-exclusive focusability to all other cells.
   • Previously, the user could press the tab key to move focus to the notebook node then press it again to move focus to the first code cell editor in the notebook. But with this change, the user bypasses the notebook node and tabs directly to the active cell.

What do I mean by tab-inclusive versus tab-exclusive focusability? Tab-inclusive focusability means setting tabIndex = 0 (or higher) on a node. This allows the user to move focus to that node using the Tab key, in addition to tapping or clicking on it. Tab-exclusive focusability refers to setting tabIndex = -1. This allows the application to focus the node programmatically, as in: document.getElementById('#notebook').focus().

Taken together, what's the end result of all of these focusability changes for keyboard-only users? It's that they can move focus to the active cell by pressing the tab key and get into that cell's editor box by then pressing the enter key (they can get out of the editor by pressing the escape key). They can move focus to other cells by pressing the up and down arrow keys. When they tab through the JupyterLab UI, they can easily return to the cell where they left off via tab navigation (because with this PR, the app only sets tabIndex = 0 on the active cell and none of the other cells).

How did we get here?

The current library behind the code cell editor in the notebook widget is Code Mirror 6, which has a solution for tab traps, namely: you press the Escape key immediately before pressing Tab. However, JupyterLab intercepts the Escape key, because it is a keyboard shortcut to change from "edit" to "command" mode in the notebook. Furthermore, when entering command mode, focus is put on the notebook widget's node (the one that contains all of the cell nodes). This means that when the user presses the Tab key after hitting Escape, they start from the beginning of the notebook, therefore never able to get past the code cell editor using only the keyboard.

My solution, therefore, was to remove tab-focusability from the CodeMirror editor. (Or more specifically, remove it only from the notebook configuration of the CodeMirror editor because CodeMirror is used in other places in JupyterLab.) This means that a user can still enter the cell editor via the mouse or the keyboard (by pressing Enter after moving to the cell with up and down arrow keys), but they cannot enter the cell editor by pressing the tab key. That way, the editor cannot trap the tab key.

Problem solved, right?

Well... Not quite. The current dev build of JupyterLab makes it hard to get back, with keyboard only, to editing a notebook if you leave it. This is because of a change I made in PR #13415. The intent of that PR was to make sure that every element in the JupyterLab UI has a visible focus indicator. Prior to that PR, the MainAreaWidget class added tabIndex = 0 (tab-inclusive focusability) to any widget loaded in the main area. So my PR changed it to tabIndex = -1, removing the ability to tab-focus the node. My justification for doing that was that every main area widget I could find had some tab-focusable element inside of it, so it seemed there was no need to make the containing element tab-focusable.

But if you remove tab-focusability from cell editors, now you have notebooks without tab-focusable elements, which means it can be hard to get into, or back into the notebook using only the keyboard.

One seemingly easy way to get out of this knot is to make the notebook node tab-focusable again, but I didn't want to move backwards. I didn't want to solve this accessibility violation (focus trap) by re-introducing another (focus not visible).

Instead I started thinking about what would move the app towards better usability. From an accessibility perspective, it's not great to keep focus on the notebook node while a user presses up and down arrows to change the active cell, because if the browser-level focus is not changed then that information is not exposed to assistive tech. In other words, the JupyterLab app knows which cell is active and should be the target of certain commands, but the system does not. The app exposes that information visually via a blue bar, but the system literally cannot see it and neither can many blind or low-vision users. Therefore, it can become difficult for all but sighted users to know which cell is active and would be affected by pressing a keyboard shortcut.

So I decided to make the active cell tab-focusable. In order to conform to WCAG 2.4.7 - Focus Visible, that means that the active cell needs a visible focus indicator, which I added in this PR.

The shift from notebook-centric to cell-centric focus is a fairly big one, and it creates some issues that I have not solved, such as losing focus when a user scrolls an active cell so far out of the viewport that it gets popped off the DOM by the WindowedList class. This PR probably also introduces some bugs that slipped through my testing. But I think it moves the app's accessibility in the right direction, and the issues that it creates are smaller and less severe than the issues that it solves. I believe those issues can be solved in future PRs, but I did not want to indefinitely delay this PR.

References

• I believe the "tab trap in code cell" issue was first described in Accessibility Issues Needing Addressing for WCAG 2.1 compliance (As of Version 2.2.6) #9399. As the issue mentions, this bug is a violation of WCAG 2.1.2 - No Keyboard Trap.
• (External) Tab trap in JupyterLab notebook code cell Quansight-Labs/jupyter-a11y-mgmt#168

Code changes

Fundamentally, the way the tab trap is removed in this PR is by turning off tab-focusability of Code Mirror 6 editor instances inside the Notebook widget.

Before this PR, when the user was in command mode, focus lived primarily on the Notebook widget node. Now focus shifts primarily among the notebook's cells, as the user moves around the notebook:

• Tab-inclusive focusability (node.tabIndex = 0) of Notebook widget removed.
• Tab-exclusive focusability (node.tabIndex = -1) added to Notebook cells, but tabIndex = 0 if cell is active.
• All or nearly all command-mode notebook keyboard shortcuts modified so that they match any focusable, non-input element inside the notebook, rather than the focussed notebook widget node itself. One exception is the enter key, which only works when a cell is focussed (as opposed to something inside the cell).
• Many notebook actions modified so that the active cell is focussed after the action occurs.

User-facing changes

• Getting into code cells
  • Before: pressing tab key could get you into a cell editor.
  • After: the only way to get into a cell editor is to enter edit mode by either tapping or clicking into the editor or pressing the enter key, when focussed on the cell that contains the editor.
• Focus
  • Before: focus tended to either be inside the cell editor (with focus indicator) or on the Notebook widget node (with no focus indicator).
  • After: notebook cells now receive focus. And when they are focussed using the keyboard, they have a big focus ring around them. It should now be clear when the cell versus the cell editor is focussed—or versus some button in the cell toolbar, or versus some element in the code cell output area.
• Keyboard shortcuts
  • Before: most notebook keyboard shortcuts would not work if some element inside the Notebook (e.g., cell toolbar) had focus.
  • After: most notebook keyboard shortcuts work so long as any non-input element inside the notebook is focussed. In other words, the user can be focused on anything inside a cell and still use the keyboard shortcuts on that cell, just so long as they're not in a text input of some sort, in which key presses should be interpreted as input characters rather than keyboard shortcuts.

Backwards-incompatible changes

None that I know of.

Known issues

Focus is lost when scrolling.

This is due to the WindowedList class, which pops cell nodes off the DOM when they scroll out of view, which in turn causes those nodes to lose focus.

Scroll-into-view logic does not take into account outline width

Update: I think this may have been fixed in #14407.

When using J/K to move up and down the document, cells scroll into view but part of the outline remains outside the view. The scroll into view logic does not account for outline width. We need to pad it with a few pixels.

Cannot use escape (or ctrl m) from a debug input prompt

Typing Exit then hitting Enter does what I want -> so we just need to hook up the escape key to that? What is a debug input prompt, you ask? It's something you can activate, for example, if you run the following line in a code cell: `input()`.

Cell focus ring on mouse click

The heuristics that Chromium uses for when to apply `:focus-visible` are not working how I want/expect. I was hoping the focus ring would only show when using the keyboard, but in my experience so far, it shows up when I use a keyboard but also when I use a mouse, and I'm not entirely sure why. My suspicion is that it has something to do with the app's rather aggressive management of focus. For example, when a main area widget is loaded, its content node is focussed, then if a user presses a key, any key, a keydown handler calls a Notebook method that attempts to ensure focus is on the right node.

[jupyterlab-probot]

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link:

[gabalafou]

I am putting this into draft mode until I add screenshots or videos and until I have had some time to do some additional testing.

[brichet]

@gabalafou @krassowski I think this PR is ready (until next conflict at least 😄).

If you have time, can you take a look at it ?

[krassowski]

I think this is good to go. I do not like that there is a slight jitter when changing cells due to the focus indicator showing with a delay. This can be addressed in a follow-up PR or we could change the CSS so that it is not as visible for example using filled/empty active cell indicator (as it was in Notebook <6) instead.

Still I think there is more value in getting this to users for testing in a pre-release than in perfecting styles in this PR.

[tonyfast]

yea it would be great to get a pre-release out of this sooner than later. we could also talk about a test plan for when it does drop. currently, we're massively limited in keyboard testing until this is resolved. so there is a to test that can't be tested now. we'll want to see how this work with widgets and extensions. the more pre release time the better. when we close this it should open a feedback issue.

[brichet]

@fcollonval @krassowski
I don't know whether the benchmark tests should be fixed or not. Otherwise we can merge this PR.

[krassowski]

I wish the top-level comment of this PR contained description for why bubblingKeydown was enabled and some of the discussion around this. For reference some relevant comments in the thread are:

• Fix tab trap notebook cells #14115 (comment) is puzzled as to why using the "capture" phase is essential for supporting chords; I don't know but Ctrl-O prefixed commands not working in jupyterlab>=4.1.0 jupyterlab-contrib/jupyterlab-vim#138 might be one example for chords being broken with "bubbling" approach
• Fix tab trap notebook cells #14115 (comment) explains that with "capture" rather than "bubbling" mode deleting multiple cells when no cell is focused would not be possible (if we kept the current set of selectors used in this PR)
• Fix tab trap notebook cells #14115 (comment) highlights that the only reason for enabling "bubbling" may be worked around by forcing focus on intermediary node in the DOM tree (between notebook and its content) - without modification of selectors

In general I feel like the assumption that "capture" mode was needed to support cords misses the point; it was needed to have higher priority over the CodeMirror native shortcuts, as seen in this PR (changes to indentMoreOrInsertTab and addition of completerOrInsertNewLine) and subsequent issues, recently Jupyterlab 4.1 dedent broken #15821. The issue is that currently we have no mechanism to assert a priority of user-configured lumino shortcuts over the CM6 shortcuts (beside overriding them manually which is not user-configurable).