Fix scrolling to output area inputs on caret movement (jupyterlab#16068)

* Fix scrolling to output area inputs on caret movement

* Remove listener for `keydown` event on disposal

packages/cells/src/widget.ts

@@ -7,6 +7,8 @@ import { Extension } from '@codemirror/state';
 
 import { EditorView } from '@codemirror/view';
 
+import { ElementExt } from '@lumino/domutils';
+
 import { AttachmentsResolver } from '@jupyterlab/attachments';
 
 import { DOMUtils, ISessionContext } from '@jupyterlab/apputils';
@@ -706,6 +708,8 @@ export class Cell<T extends ICellModel = ICellModel> extends Widget {
   protected prompt = '';
   protected translator: ITranslator;
   protected _displayChanged = new Signal<this, void>(this);
+  protected _scrollRequested = new Signal<Cell, Cell.IScrollRequest>(this);
+  protected _inViewport: boolean | null;
 
   /**
    * Editor extension emitting `scrollRequested` signal on scroll.
@@ -736,13 +740,11 @@ export class Cell<T extends ICellModel = ICellModel> extends Widget {
   );
 
   private _editorConfig: Record<string, any> = {};
-  private _scrollRequested = new Signal<Cell, Cell.IScrollRequest>(this);
   private _editorExtensions: Extension[] = [];
   private _input: InputArea | null;
   private _inputHidden = false;
   private _inputWrapper: Widget | null;
   private _inputPlaceholder: InputPlaceholder | null;
-  private _inViewport: boolean | null;
   private _inViewportChanged: Signal<Cell, boolean> = new Signal<Cell, boolean>(
     this
   );
@@ -958,7 +960,7 @@ export namespace Cell {
      * require the cell to be first scrolled into the viewport to
      * then enable proper scrolling within cell.
      */
-    scrollWithinCell: () => void;
+    scrollWithinCell: (options: { scroller: HTMLElement }) => void;
     /**
      * Whether the default scrolling was prevented due to the cell being out of viewport.
      */
@@ -1083,6 +1085,8 @@ export class CodeCell extends Cell<ICodeCellModel> {
       promptOverlay: true,
       inputHistoryScope: options.inputHistoryScope
     }));
+    output.node.addEventListener('keydown', this._detectCaretMovementInOuput);
+
     output.addClass(CELL_OUTPUT_AREA_CLASS);
     output.toggleScrolling.connect(() => {
       this.outputsScrolled = !this.outputsScrolled;
@@ -1098,6 +1102,65 @@ export class CodeCell extends Cell<ICodeCellModel> {
     model.stateChanged.connect(this.onStateChanged, this);
   }
 
+  /**
+   * Detect the movement of the caret in the output area.
+   *
+   * Emits scroll request if the caret moved.
+   */
+  private _detectCaretMovementInOuput = (e: KeyboardEvent) => {
+    const inWindowedContainer = this._inViewport !== null;
+    const defaultPrevented = inWindowedContainer && !this._inViewport;
+
+    // Because we do not want to scroll on any key, but only on keys which
+    // move the caret (this on keys which cause input and on keys like left,
+    // right, top, bottom arrow, home, end, page down/up - but only if the
+    // cursor is not at the respective end of the input) we need to listen
+    // to the `selectionchange` event on target inputs/textareas, etc.
+    const target = e.target;
+
+    if (!target || !(target instanceof HTMLElement)) {
+      return;
+    }
+
+    // Make sure the previous listener gets disconnected
+    if (this._lastTarget) {
+      this._lastTarget.removeEventListener(
+        'selectionchange',
+        this._lastOnCaretMovedHandler
+      );
+      document.removeEventListener(
+        'selectionchange',
+        this._lastOnCaretMovedHandler
+      );
+    }
+
+    const onCaretMoved = () => {
+      this._scrollRequested.emit({
+        scrollWithinCell: ({ scroller }) => {
+          ElementExt.scrollIntoViewIfNeeded(scroller, target);
+        },
+        defaultPrevented
+      });
+    };
+
+    // Remember the most recent target/handler to disconnect them next time.
+    this._lastTarget = target;
+    this._lastOnCaretMovedHandler = onCaretMoved;
+
+    // Firefox only supports `selectionchange` on the actual input element,
+    // all other browsers only support it on the top-level document.
+    target.addEventListener('selectionchange', onCaretMoved, { once: true });
+    document.addEventListener('selectionchange', onCaretMoved, {
+      once: true
+    });
+
+    // Schedule removal of the listener.
+    setTimeout(() => {
+      target.removeEventListener('selectionchange', onCaretMoved);
+      document.removeEventListener('selectionchange', onCaretMoved);
+    }, 250);
+  };
+
   /**
    * Maximum number of outputs to display.
    */
@@ -1498,6 +1561,10 @@ export class CodeCell extends Cell<ICodeCellModel> {
       this._outputLengthHandler,
       this
     );
+    this._output.node.removeEventListener(
+      'keydown',
+      this._detectCaretMovementInOuput
+    );
     this._rendermime = null!;
     this._output = null!;
     this._outputWrapper = null!;
@@ -1586,6 +1653,8 @@ export class CodeCell extends Cell<ICodeCellModel> {
   private _outputPlaceholder: OutputPlaceholder | null = null;
   private _output: OutputArea;
   private _syncScrolled = false;
+  private _lastOnCaretMovedHandler: () => void;
+  private _lastTarget: HTMLElement | null = null;
 }
 
 /**

packages/notebook/src/widget.ts

@@ -2120,17 +2120,20 @@ export class Notebook extends StaticNotebook {
         // Nothing to do if scroll request was already handled.
         return;
       }
+      // Node which allows to scroll the notebook
+      const scroller = this.outerNode;
+
       if (cell.inViewport) {
         // If cell got scrolled to the viewport in the meantime,
         // proceed with scrolling within the cell.
-        return scrollRequest.scrollWithinCell();
+        return scrollRequest.scrollWithinCell({ scroller });
       }
       // If cell is not in the viewport and needs scrolling,
       // first scroll to the cell and then scroll within the cell.
       this.scrollToItem(this.activeCellIndex)
         .then(() => {
           void cell.ready.then(() => {
-            scrollRequest.scrollWithinCell();
+            scrollRequest.scrollWithinCell({ scroller });
           });
         })
         .catch(reason => {