Page up/page down partially scrolls notebook back

[krassowski]

Description

Pressing Pg up/pg down in a cell scrolls notebook in unexpected way.

Reproduce

1. Switch windowing mode to full
2. Open a notebook with more cells than fit in the viewport
3. Press pg down in the cell once
4. See that it navigates to top/bottom of the cell (ok)
5. Press pg down in the cell again
6. See that it scrolls notebook (ok)
7. Press pg down in the cell again and again
8. See that notebook scrolls in chaotic way jumping back to the active cell (not ok)

Expected behavior

Scrolling in full windowing mode behaves predictably.

Context

• Browser: Chrome
• JupyterLab version: 4.1.0b0

[JasonWeill]

Because windowed notebooks are not the default in 4.1.0, this bug, while noticeable and annoying, should not block a 4.1.0 release.

[krassowski]

Note: in defer/none mode the behaviour is: first page down/up press causes navigation to edge, subsequent presses cause scrolling. The issue in full mode is that after subsequent presses the active cell gets scrolled back into view.

[krassowski]

When the active cell is scrolled out of view pressing any key, including Alt and PgUp/PgDown (which do not generate keystrokes) causes the viewport to scroll back to the active cell. This might be the root issue.

[krassowski]

Which comes about because the notebook widget handler forces focus on the active cell on keydown event:

jupyterlab/packages/notebook/src/widget.ts

Lines 1948 to 1950 in 4df55ca

	case 'keydown':
	// This works because CodeMirror does not stop the event propagation
	this._ensureFocus(true);

I think the solution is to filter the keydown events to these which cause (a) cursor movement (b) input (c) selection change.

[krassowski]

This is interesting, so keydown is invoked in both full and none/defer mode; the difference comes down to activeCell.inViewport property evaluating to false in full mode (correctly) and to true in none/defer modes (incorrectly?).

jupyterlab/packages/notebook/src/widget.ts

Lines 2212 to 2217 in 4df55ca

	const activeCell = this.activeCell;
	if (this.mode === 'edit' && activeCell) {
	// Test for !== true to cover hasFocus is false and editor is not yet rendered.
	if (activeCell.editor?.hasFocus() !== true || !activeCell.inViewport) {
	if (activeCell.inViewport) {
	activeCell.editor?.focus();

inViewport is just a boolean flag:

jupyterlab/packages/cells/src/widget.ts

Lines 240 to 251 in 4df55ca

	/**
	* Whether the cell is in viewport or not.
	*/
	get inViewport(): boolean {
	return this._inViewport;
	}
	set inViewport(v: boolean) {
	if (this._inViewport !== v) {
	this._inViewport = v;
	this._inViewportChanged.emit(this._inViewport);
	}
	}

which gets set when windowing is active:

jupyterlab/packages/notebook/src/windowing.ts

Line 238 in 4df55ca

(widget as Cell).inViewport = true;

It might be that we do not need to call focus if the cell is out of viewport, but it needs more investigation. The condition above was recently modified in #15286.

Edit: By attempting to remove || !activeCell.inViewport I can confirm that this is indeed needed for scrolling to the active cell when out of view on keypreses which should trigger scroll. However, there is some scrolling on keypress even if || !activeCell.inViewport is commented out.

[krassowski]

It feels like we should rely on the internal heuristics of CodeMirror to decide when to scroll. These unfortunately are not exposed via a public API and just invoke a very clever scroll implementation, which however cannot succeed when in a virtualized list as it does not know about the offset which is hidden. I opened codemirror/dev#1355 to ask if a public entrypoint could be added but it may be a big ask.

[erkin98]

i'm getting similar issue which, i have 50 line of code inside one cell, so when i try to add cell it scrols the notebook whole the way down