Notebooks: Aligned scroll into view behaviour with vscode (#13742)

* Fixed focus loss of the notebook editor widget when a cell editor was unfocused Signed-off-by: Jonah Iden <jonah.iden@typefox.io> * aligned the scroll into view behaviour with vscode Signed-off-by: Jonah Iden <jonah.iden@typefox.io> * inserting new cell scrolls Signed-off-by: Jonah Iden <jonah.iden@typefox.io> * fixed delete still scrolling Signed-off-by: Jonah Iden <jonah.iden@typefox.io> * dont scroll into cell when click into text Signed-off-by: Jonah Iden <jonah.iden@typefox.io> --------- Signed-off-by: Jonah Iden <jonah.iden@typefox.io>
eclipse-theia · May 30, 2024 · 255231a · 255231a
1 parent 50f75fb
commit 255231a
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 16 deletions.
diff --git a/packages/notebook/src/browser/service/notebook-context-manager.ts b/packages/notebook/src/browser/service/notebook-context-manager.ts
@@ -88,6 +88,7 @@ export class NotebookContextManager {
         // Cell Selection realted keys
         this.scopedStore.setContext(NOTEBOOK_CELL_FOCUSED, !!widget.model?.selectedCell);
         widget.model?.onDidChangeSelectedCell(e => {
+            this.selectedCellChanged(e.cell);
             this.scopedStore.setContext(NOTEBOOK_CELL_FOCUSED, !!e);
             this.onDidChangeContextEmitter.fire(this.createContextKeyChangedEvent([NOTEBOOK_CELL_FOCUSED]));
         });
@@ -100,8 +101,6 @@ export class NotebookContextManager {
             }
         }));
 
-        widget.model?.onDidChangeSelectedCell(e => this.selectedCellChanged(e));
-
         widget.onDidChangeOutputInputFocus(focus => {
             this.scopedStore.setContext(NOTEBOOK_OUTPUT_INPUT_FOCUSED, focus);
             this.onDidChangeContextEmitter.fire(this.createContextKeyChangedEvent([NOTEBOOK_OUTPUT_INPUT_FOCUSED]));

diff --git a/packages/notebook/src/browser/view-model/notebook-model.ts b/packages/notebook/src/browser/view-model/notebook-model.ts
@@ -56,6 +56,11 @@ export interface NotebookModelProps {
     serializer: NotebookSerializer;
 }
 
+export interface SelectedCellChangeEvent {
+    cell: NotebookCellModel | undefined;
+    scrollIntoView: boolean;
+}
+
 @injectable()
 export class NotebookModel implements Saveable, Disposable {
 
@@ -74,7 +79,7 @@ export class NotebookModel implements Saveable, Disposable {
     protected readonly onContentChangedEmitter = new Emitter<void>();
     readonly onContentChanged = this.onContentChangedEmitter.event;
 
-    protected readonly onDidChangeSelectedCellEmitter = new Emitter<NotebookCellModel | undefined>();
+    protected readonly onDidChangeSelectedCellEmitter = new Emitter<SelectedCellChangeEvent>();
     readonly onDidChangeSelectedCell = this.onDidChangeSelectedCellEmitter.event;
 
     protected readonly onDidDisposeEmitter = new Emitter<void>();
@@ -246,10 +251,10 @@ export class NotebookModel implements Saveable, Disposable {
         this.undoRedoService.redo(this.uri);
     }
 
-    setSelectedCell(cell: NotebookCellModel): void {
+    setSelectedCell(cell: NotebookCellModel, scrollIntoView?: boolean): void {
         if (this.selectedCell !== cell) {
             this.selectedCell = cell;
-            this.onDidChangeSelectedCellEmitter.fire(cell);
+            this.onDidChangeSelectedCellEmitter.fire({ cell, scrollIntoView: scrollIntoView ?? true });
         }
     }
 
@@ -285,9 +290,12 @@ export class NotebookModel implements Saveable, Disposable {
             if (cell) {
                 this.cellDirtyChanged(cell, true);
             }
+
+            let scrollIntoView = true;
             switch (edit.editType) {
                 case CellEditType.Replace:
                     this.replaceCells(edit.index, edit.count, edit.cells, computeUndoRedo);
+                    scrollIntoView = edit.cells.length > 0;
                     break;
                 case CellEditType.Output: {
                     if (edit.append) {
@@ -330,7 +338,7 @@ export class NotebookModel implements Saveable, Disposable {
 
             // if selected cell is affected update it because it can potentially have been replaced
             if (cell === this.selectedCell) {
-                this.setSelectedCell(this.cells[Math.min(cellIndex, this.cells.length - 1)]);
+                this.setSelectedCell(this.cells[Math.min(cellIndex, this.cells.length - 1)], scrollIntoView);
             }
         }
 

diff --git a/packages/notebook/src/browser/view/notebook-cell-editor.tsx b/packages/notebook/src/browser/view/notebook-cell-editor.tsx
@@ -78,8 +78,8 @@ export class CellEditor extends React.Component<CellEditorProps, {}> {
             this.editor?.setLanguage(language);
         }));
 
-        this.toDispose.push(this.props.notebookModel.onDidChangeSelectedCell(cell => {
-            if (cell !== this.props.cell && this.editor?.getControl().hasTextFocus()) {
+        this.toDispose.push(this.props.notebookModel.onDidChangeSelectedCell(e => {
+            if (e.cell !== this.props.cell && this.editor?.getControl().hasTextFocus()) {
                 this.props.notebookContextManager.context?.focus();
             }
         }));
@@ -129,7 +129,7 @@ export class CellEditor extends React.Component<CellEditorProps, {}> {
             }));
             this.toDispose.push(this.editor.getControl().onDidFocusEditorText(() => {
                 this.props.notebookContextManager.onDidEditorTextFocus(true);
-                this.props.notebookModel.setSelectedCell(cell);
+                this.props.notebookModel.setSelectedCell(cell, false);
             }));
             this.toDispose.push(this.editor.getControl().onDidBlurEditorText(() => {
                 this.props.notebookContextManager.onDidEditorTextFocus(false);

diff --git a/packages/notebook/src/browser/view/notebook-cell-list-view.tsx b/packages/notebook/src/browser/view/notebook-cell-list-view.tsx
@@ -37,6 +37,7 @@ interface CellListProps {
 
 interface NotebookCellListState {
     selectedCell?: NotebookCellModel;
+    scrollIntoView: boolean;
     dragOverIndicator: { cell: NotebookCellModel, position: 'top' | 'bottom' } | undefined;
 }
 
@@ -48,17 +49,29 @@ export class NotebookCellListView extends React.Component<CellListProps, Noteboo
 
     constructor(props: CellListProps) {
         super(props);
-        this.state = { selectedCell: props.notebookModel.selectedCell, dragOverIndicator: undefined };
+        this.state = { selectedCell: props.notebookModel.selectedCell, dragOverIndicator: undefined, scrollIntoView: true };
         this.toDispose.push(props.notebookModel.onDidAddOrRemoveCell(e => {
             if (e.newCellIds && e.newCellIds.length > 0) {
-                this.setState({ ...this.state, selectedCell: this.props.notebookModel.cells.find(model => model.handle === e.newCellIds![e.newCellIds!.length - 1]) });
+                this.setState({
+                    ...this.state,
+                    selectedCell: this.props.notebookModel.cells.find(model => model.handle === e.newCellIds![e.newCellIds!.length - 1]),
+                    scrollIntoView: true
+                });
             } else {
-                this.setState({ ...this.state, selectedCell: this.props.notebookModel.cells.find(cell => cell === this.state.selectedCell) });
+                this.setState({
+                    ...this.state,
+                    selectedCell: this.props.notebookModel.cells.find(cell => cell === this.state.selectedCell),
+                    scrollIntoView: false
+                });
             }
         }));
 
-        this.toDispose.push(props.notebookModel.onDidChangeSelectedCell(cell => {
-            this.setState({ ...this.state, selectedCell: cell });
+        this.toDispose.push(props.notebookModel.onDidChangeSelectedCell(e => {
+            this.setState({
+                ...this.state,
+                selectedCell: e.cell,
+                scrollIntoView: e.scrollIntoView
+            });
         }));
     }
 
@@ -80,13 +93,13 @@ export class NotebookCellListView extends React.Component<CellListProps, Noteboo
                         <li className={'theia-notebook-cell' + (this.state.selectedCell === cell ? ' focused' : '') + (this.isEnabled() ? ' draggable' : '')}
                             onClick={e => {
                                 this.setState({ ...this.state, selectedCell: cell });
-                                this.props.notebookModel.setSelectedCell(cell);
+                                this.props.notebookModel.setSelectedCell(cell, false);
                             }}
                             onDragStart={e => this.onDragStart(e, index, cell)}
                             onDragOver={e => this.onDragOver(e, cell)}
                             onDrop={e => this.onDrop(e, index)}
                             draggable={true}
-                            ref={ref => cell === this.state.selectedCell && ref?.scrollIntoView({ block: 'nearest' })}>
+                            ref={ref => cell === this.state.selectedCell && this.state.scrollIntoView && ref?.scrollIntoView({ block: 'nearest' })}>
                             <div className={'theia-notebook-cell-marker' + (this.state.selectedCell === cell ? ' theia-notebook-cell-marker-selected' : '')}></div>
                             <div className='theia-notebook-cell-content'>
                                 {this.renderCellContent(cell, index)}