Fix tab trap notebook cells (jupyterlab#14115)

* WIP

* tabFocusable

* not sure what to do with this stuff

* update keyboard shortcuts

* remove data-lm-suppress-shortcuts

* focus active cell when type changed

* check for cell

* fix onActivateRequest test

* focus active cell when ready

* some cleanup

* use ready instead of attached

* add commend about tabFocusable default being true

* whitespace

* comment on active cell tabindex 0

* check that descendant node has focus

* ensureFocus active cell

* remove console.logs

* fix ctrl shift c

* update comment

* add shouldWait option to focusActiveCell()

* lint

* more lint

* missed a spot: lint

* consistent outline

* box-shadow outline

* box-shadow

* Solve jupyterlab#1947 in a more specific way

The keydown->ensureFocus code (jupyterlab#1958) was added to fix jupyterlab#1947.

The idea was that any key event that bubbles up from within the
notebook widget but causes the focus to move somewhere outside
the notebook was something like when the user presses enter from
within a Stdin widget, causing this widget, which had focus, to
remove itself from the DOM, causing focus to get lost, meaning
returned to the body node.

But if we change the _ensureFocus method so that it ensures
focus on the active cell rather than on the notebook as a whole
it can lead to bugs like the one where the user presses tab
while on the notebook footer, triggering the keydown->ensureFocus
code, causing focus to go up the notebook, backwards (because
all cells are above the footer), which makes it impossible to tab
forward out of the notebook. Because you get into the following
loop:

cell -> ... -> footer (-> keydown->ensureFocus) -> cell -> ... -> footer ->

* preventScroll

* use brand-color for cell focus ring

* use :read-write

* remove schema

* Remove no-longer needed traversable data attribute

* Fix styling when multi-selection is active

* Update Playwright Snapshots

* Update Playwright Snapshots

* Uses bubbling phase in keydown event in CI

* Avoid triggering indentation in codemirror if completer is available

* integrity

* snapshots

* moves the completer-enable class to the editor

* Change the selector to select and delete all cells

* Remove the linebreak shortcut in notebook console, since the linebreak is already handled by Enter

* Remove ':focus:not(:read-write)' in selector, useless now the keydown is handled on bubbling

* Remove duplicated shortcuts

* Handles 'Enter' key when the completer is active

* Set the focus to a child node instead of the Notebook node.

* Handle the focus on Notbeook in windowed panel

* Hide code cells with widget functions and CSS to avoid using checkVisibility(), and fix notebook actions tests

* Do not attach/detach codecell when windowing, to avoid inconstitancy state

* Restore actions sync

* ui-test snapshots

* Handle the expected failures in notebook actions tests

* Do not allow setting the keyboard event mode in page config

* Updates benchmark snapshots

* Merge main in fix-tab-trap-notebook-cells, and fix conficts

* Revert "Merge main in fix-tab-trap-notebook-cells, and fix conficts"

This reverts commit b7f5d93.

* Revert changes related to windowing (it is handle in main branch)

* Remove visibility check on activeCell before focusing, since it should always be visible now

* Wait for the cell to be ready before focusing

* fix promises not awaited and update ui-test -> documentation -> general snapshot

* Avoid focus looping in active cell

---------

Co-authored-by: Michał Krassowski <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Nicolas Brichet <[email protected]>

dev_mode/index.js

@@ -207,8 +207,7 @@ export async function main() {
   });
   register.forEach(function(item) { lab.registerPluginModule(item); });
 
-  const bubblingKeydown = PageConfig.getOption('bubbling_keydown') || false
-  lab.start({ ignorePlugins, bubblingKeydown });
+  lab.start({ ignorePlugins, bubblingKeydown: true });
 
   // Expose global app instance when in dev mode or when toggled explicitly.
   var exposeAppInBrowser = (PageConfig.getOption('exposeAppInBrowser') || '').toLowerCase() === 'true';

packages/cell-toolbar/style/base.css

@@ -28,7 +28,7 @@
   /* Override .jp-Toolbar */
   background-color: transparent;
   border-bottom: inherit;
-  box-shadow: inherit;
+  box-shadow: none;
 }
 
 /* Overrides for mobile view hiding cell toolbar */

packages/codeeditor/src/tokens.ts

@@ -7,6 +7,16 @@ import { CodeEditor } from './editor';
 import { IEditorFactoryService } from './factory';
 import { IEditorMimeTypeService } from './mimetype';
 
+/**
+ * A class added to editors that can host a completer.
+ */
+export const COMPLETER_ENABLED_CLASS: string = 'jp-mod-completer-enabled';
+
+/**
+ * A class added to editors that have an active completer.
+ */
+export const COMPLETER_ACTIVE_CLASS: string = 'jp-mod-completer-active';
+
 /**
  * Code editor services token.
  */

packages/codemirror/src/commands.ts

@@ -3,8 +3,16 @@
  * Distributed under the terms of the Modified BSD License.
  */
 
-import { indentMore, insertTab } from '@codemirror/commands';
+import {
+  indentMore,
+  insertNewlineAndIndent,
+  insertTab
+} from '@codemirror/commands';
 import { EditorState, Transaction } from '@codemirror/state';
+import {
+  COMPLETER_ACTIVE_CLASS,
+  COMPLETER_ENABLED_CLASS
+} from '@jupyterlab/codeeditor';
 
 /**
  * CodeMirror commands namespace
@@ -14,9 +22,14 @@ export namespace StateCommands {
    * Indent or insert a tab as appropriate.
    */
   export function indentMoreOrInsertTab(target: {
+    dom: HTMLElement;
     state: EditorState;
     dispatch: (transaction: Transaction) => void;
   }): boolean {
+    if (target.dom.parentElement?.classList.contains(COMPLETER_ENABLED_CLASS)) {
+      return false;
+    }
+
     const arg = { state: target.state, dispatch: target.dispatch };
     const from = target.state.selection.main.from;
     const to = target.state.selection.main.to;
@@ -31,4 +44,21 @@ export namespace StateCommands {
       return insertTab(arg);
     }
   }
+
+  /**
+   * Insert new line if completer is not active.
+   */
+  export function completerOrInsertNewLine(target: {
+    dom: HTMLElement;
+    state: EditorState;
+    dispatch: (transaction: Transaction) => void;
+  }): boolean {
+    if (target.dom.parentElement?.classList.contains(COMPLETER_ACTIVE_CLASS)) {
+      // return true to avoid handling the default Enter from codemirror defaultKeymap.
+      return true;
+    }
+
+    const arg = { state: target.state, dispatch: target.dispatch };
+    return insertNewlineAndIndent(arg);
+  }
 }

packages/codemirror/src/extension.ts

@@ -695,6 +695,10 @@ export namespace EditorExtensionRegistry {
       Object.freeze({
         name: 'keymap',
         default: [
+          {
+            key: 'Enter',
+            run: StateCommands.completerOrInsertNewLine
+          },
           ...defaultKeymap,
           {
             key: 'Tab',
@@ -795,6 +799,42 @@ export namespace EditorExtensionRegistry {
           title: trans.__('Smart Indentation')
         }
       }),
+      /**
+       * tabFocusable
+       *
+       * Can the user use the tab key to focus on / enter the CodeMirror editor?
+       * If this is false, the CodeMirror editor can still be focused (via
+       * mouse-click, for example), just not via tab key navigation.
+       *
+       * It can be useful to set tabFocusable to false when the editor is
+       * embedded in a context that provides an alternative to the tab key for
+       * navigation. For example, the Notebook widget allows the user to move
+       * from one cell to another using the up and down arrow keys and to enter
+       * and exit the CodeMirror editor associated with that cell by pressing
+       * the enter and escape keys, respectively.
+       */
+      Object.freeze({
+        name: 'tabFocusable',
+        // The default for this needs to be true because the CodeMirror editor
+        // is used in lots of different places. By default, a user should be
+        // able to tab into a CodeMirror editor on the page, and by default, the
+        // user should be able to get out of the editor by pressing the escape
+        // key then immediately pressing the tab key (or shift+tab to go
+        // backwards on the page). If there are places in the app where this
+        // model of user interaction doesn't make sense or is broken, those
+        // places should be remedied on a case-by-case basis, **not** by making
+        // `tabFocusable` false by default.
+        default: true,
+        factory: () =>
+          createConditionalExtension(
+            EditorView.contentAttributes.of({
+              tabIndex: '0'
+            }),
+            EditorView.contentAttributes.of({
+              tabIndex: '-1'
+            })
+          )
+      }),
       Object.freeze({
         name: 'tabSize',
         default: 4,

packages/completer/src/handler.ts

@@ -1,7 +1,11 @@
 // Copyright (c) Jupyter Development Team.
 // Distributed under the terms of the Modified BSD License.
 
-import { CodeEditor } from '@jupyterlab/codeeditor';
+import {
+  CodeEditor,
+  COMPLETER_ACTIVE_CLASS,
+  COMPLETER_ENABLED_CLASS
+} from '@jupyterlab/codeeditor';
 import { Text } from '@jupyterlab/coreutils';
 import { ISharedText, SourceChange } from '@jupyter/ydoc';
 import { IDataConnector } from '@jupyterlab/statedb';
@@ -21,16 +25,6 @@ import {
 import { Completer } from './widget';
 import { InlineCompleter } from './inline';
 
-/**
- * A class added to editors that can host a completer.
- */
-const COMPLETER_ENABLED_CLASS: string = 'jp-mod-completer-enabled';
-
-/**
- * A class added to editors that have an active completer.
- */
-const COMPLETER_ACTIVE_CLASS: string = 'jp-mod-completer-active';
-
 /**
  * A completion handler for editors.
  */

packages/console-extension/schema/tracker.json

@@ -72,11 +72,6 @@
     ]
   },
   "jupyter.lab.shortcuts": [
-    {
-      "command": "console:linebreak",
-      "keys": ["Enter"],
-      "selector": ".jp-CodeConsole[data-jp-interaction-mode='notebook'] .jp-CodeConsole-promptCell"
-    },
     {
       "command": "console:run-forced",
       "keys": ["Shift Enter"],