Pārlūkot izejas kodu

Backport PR #12258 on branch 3.4.x (Default is no virtual rendering + Relax virtual notebook rendering on idle) (#12395)

* Backport PR #12258: Default is no virtual rendering + Relax virtual notebook rendering and ensure no structural change until rendering is completed

* cast window to any to call cancelIdlelCallback

* add optional translator option for notebook widget constructor

* lint

* Update packages/notebook/src/widget.ts

Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>

* pass translator to the notebook widget constructor

Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
Eric Charles 3 gadi atpakaļ
vecāks
revīzija
ae5ca82a65

+ 7 - 1
packages/notebook-extension/schema/tracker.json

@@ -818,7 +818,13 @@
       "title": "Number of cells to render directly",
       "description": "Define the number of cells to render directly when virtual notebook intersection observer is available",
       "type": "number",
-      "default": 20
+      "default": 99999
+    },
+    "remainingTimeBeforeRescheduling": {
+      "title": "Remaining time in milliseconds before virtual notebook rendering is rescheduled",
+      "description": "Define the remaining time in milliseconds before virtual notebook rendering is rescheduled. Set 0 if you want to disable any rescheduling",
+      "type": "number",
+      "default": 50
     },
     "renderCellOnIdle": {
       "title": "Render cell on browser idle time",

+ 3 - 0
packages/notebook-extension/src/index.ts

@@ -1354,6 +1354,9 @@ function activateNotebookHandler(
       recordTiming: settings.get('recordTiming').composite as boolean,
       numberCellsToRenderDirectly: settings.get('numberCellsToRenderDirectly')
         .composite as number,
+      remainingTimeBeforeRescheduling: settings.get(
+        'remainingTimeBeforeRescheduling'
+      ).composite as number,
       renderCellOnIdle: settings.get('renderCellOnIdle').composite as boolean,
       observedTopMargin: settings.get('observedTopMargin').composite as string,
       observedBottomMargin: settings.get('observedBottomMargin')

+ 58 - 0
packages/notebook/src/actions.tsx

@@ -149,6 +149,9 @@ export namespace NotebookActions {
       return;
     }
 
+    if (!Private.isNotebookRendered(notebook)) {
+      return;
+    }
     const state = Private.getState(notebook);
 
     notebook.deselectAll();
@@ -244,6 +247,9 @@ export namespace NotebookActions {
       return;
     }
 
+    if (!Private.isNotebookRendered(notebook)) {
+      return;
+    }
     const state = Private.getState(notebook);
     const toMerge: string[] = [];
     const toDelete: ICellModel[] = [];
@@ -340,6 +346,9 @@ export namespace NotebookActions {
       return;
     }
 
+    if (!Private.isNotebookRendered(notebook)) {
+      return;
+    }
     const state = Private.getState(notebook);
 
     Private.deleteCells(notebook);
@@ -362,6 +371,9 @@ export namespace NotebookActions {
       return;
     }
 
+    if (!Private.isNotebookRendered(notebook)) {
+      return;
+    }
     const state = Private.getState(notebook);
     const model = notebook.model;
     const cell = model.contentFactory.createCell(
@@ -394,6 +406,9 @@ export namespace NotebookActions {
       return;
     }
 
+    if (!Private.isNotebookRendered(notebook)) {
+      return;
+    }
     const state = Private.getState(notebook);
     const model = notebook.model;
     const cell = model.contentFactory.createCell(
@@ -419,6 +434,9 @@ export namespace NotebookActions {
       return;
     }
 
+    if (!Private.isNotebookRendered(notebook)) {
+      return;
+    }
     const state = Private.getState(notebook);
     const cells = notebook.model.cells;
     const widgets = notebook.widgets;
@@ -450,6 +468,9 @@ export namespace NotebookActions {
       return;
     }
 
+    if (!Private.isNotebookRendered(notebook)) {
+      return;
+    }
     const state = Private.getState(notebook);
     const cells = notebook.model.cells;
     const widgets = notebook.widgets;
@@ -594,6 +615,9 @@ export namespace NotebookActions {
       return Promise.resolve(false);
     }
 
+    if (!Private.isNotebookRendered(notebook)) {
+      return Promise.resolve(false);
+    }
     const state = Private.getState(notebook);
     const promise = Private.runSelected(notebook, sessionContext);
     const model = notebook.model;
@@ -948,6 +972,9 @@ export namespace NotebookActions {
    * A new code cell is added if all cells are cut.
    */
   export function cut(notebook: Notebook): void {
+    if (!Private.isNotebookRendered(notebook)) {
+      return;
+    }
     Private.copyOrCut(notebook, true);
   }
 
@@ -974,6 +1001,10 @@ export namespace NotebookActions {
       return;
     }
 
+    if (!Private.isNotebookRendered(notebook)) {
+      return;
+    }
+
     const clipboard = Clipboard.getInstance();
 
     if (!clipboard.hasData(JUPYTER_CELL_MIME)) {
@@ -1071,6 +1102,9 @@ export namespace NotebookActions {
       return;
     }
 
+    if (!Private.isNotebookRendered(notebook)) {
+      return;
+    }
     const state = Private.getState(notebook);
 
     notebook.mode = 'command';
@@ -1847,6 +1881,30 @@ namespace Private {
     activeCell: Cell | null;
   }
 
+  export function isNotebookRendered(notebook: Notebook): boolean {
+    const translator = notebook.translator;
+    const trans = translator.load('jupyterlab');
+
+    if (notebook.remainingCellToRenderCount !== 0) {
+      showDialog({
+        body: trans.__(
+          `Notebook is still rendering and has for now (%1) remaining cells to render.
+
+Please wait for the complete rendering before invoking that action.`,
+          notebook.remainingCellToRenderCount
+        ),
+        buttons: [Dialog.okButton({ label: trans.__('Ok') })]
+      }).catch(reason => {
+        console.error(
+          'An error occurred when displaying notebook rendering warning',
+          reason
+        );
+      });
+      return false;
+    }
+    return true;
+  }
+
   /**
    * Get the state of a widget before running an action.
    */

+ 66 - 8
packages/notebook/src/widget.ts

@@ -16,6 +16,7 @@ import { IChangedArgs } from '@jupyterlab/coreutils';
 import * as nbformat from '@jupyterlab/nbformat';
 import { IObservableList, IObservableMap } from '@jupyterlab/observables';
 import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
+import { ITranslator, nullTranslator } from '@jupyterlab/translation';
 import { ArrayExt, each, findIndex } from '@lumino/algorithm';
 import { MimeData, ReadonlyPartialJSONValue } from '@lumino/coreutils';
 import { ElementExt } from '@lumino/domutils';
@@ -190,6 +191,7 @@ export class StaticNotebook extends Widget {
     this.node.dataset[UNDOER] = 'true';
     this.node.dataset[CODE_RUNNER] = 'true';
     this.rendermime = options.rendermime;
+    this.translator = options.translator ?? nullTranslator;
     this.layout = new Private.NotebookPanelLayout();
     this.contentFactory =
       options.contentFactory || StaticNotebook.defaultContentFactory;
@@ -201,6 +203,7 @@ export class StaticNotebook extends Widget {
     this.renderingLayout = options.notebookConfig?.renderingLayout;
 
     // Section for the virtual-notebook behavior.
+    this._idleCallBack = null;
     this._toRenderMap = new Map<string, { index: number; cell: Cell }>();
     this._cellsArray = new Array<Cell>();
     if ('IntersectionObserver' in window) {
@@ -267,6 +270,11 @@ export class StaticNotebook extends Widget {
    */
   readonly rendermime: IRenderMimeRegistry;
 
+  /**
+   * Translator to be used by cell renderers
+   */
+  readonly translator: ITranslator;
+
   /**
    * The model for the widget.
    */
@@ -584,19 +592,47 @@ export class StaticNotebook extends Widget {
       // We have no intersection observer, or we insert, or we are below
       // the number of cells to render directly, so we render directly.
       layout.insertWidget(index, widget);
-      this._incrementRenderedCount();
       this.onCellInserted(index, widget);
+      this._incrementRenderedCount();
     }
+    this._scheduleCellRenderOnIdle();
+  }
 
-    if (this._observer && this.notebookConfig.renderCellOnIdle) {
-      const renderPlaceholderCells = this._renderPlaceholderCells.bind(this);
-      (window as any).requestIdleCallback(renderPlaceholderCells, {
-        timeout: 1000
-      });
+  private _scheduleCellRenderOnIdle() {
+    if (
+      this._observer &&
+      this.notebookConfig.renderCellOnIdle &&
+      !this.isDisposed
+    ) {
+      if (!this._idleCallBack) {
+        const renderPlaceholderCells = this._renderPlaceholderCells.bind(this);
+        this._idleCallBack = (window as any).requestIdleCallback(
+          renderPlaceholderCells,
+          {
+            timeout: 3000
+          }
+        );
+      }
     }
   }
 
   private _renderPlaceholderCells(deadline: any) {
+    if (this.notebookConfig.remainingTimeBeforeRescheduling > 0) {
+      const timeRemaining = deadline.timeRemaining();
+      // In case this got triggered because of timeout or when there are screen updates (https://w3c.github.io/requestidlecallback/#idle-periods),
+      // avoiding the render and rescheduling the place holder cell rendering.
+      if (
+        deadline.didTimeout ||
+        timeRemaining < this.notebookConfig.remainingTimeBeforeRescheduling
+      ) {
+        if (this._idleCallBack) {
+          (window as any).cancelIdleCallback(this._idleCallBack);
+          this._idleCallBack = null;
+        }
+        this._scheduleCellRenderOnIdle();
+      }
+    }
+
     if (
       this._renderedCellsCount < this._cellsArray.length &&
       this._renderedCellsCount >=
@@ -608,6 +644,11 @@ export class StaticNotebook extends Widget {
   }
 
   private _renderPlaceholderCell(cell: Cell, index: number) {
+    // We don't have cancel mechanism for scheduled requestIdleCallback(renderPlaceholderCells),
+    // adding defensive check for layout in case tab is closed.
+    if (!this.layout) {
+      return;
+    }
     const pl = this.layout as PanelLayout;
     pl.removeWidgetAt(index);
     pl.insertWidget(index, cell);
@@ -821,6 +862,10 @@ export class StaticNotebook extends Widget {
     this._renderedCellsCount++;
   }
 
+  public get remainingCellToRenderCount(): number {
+    return this._toRenderMap.size;
+  }
+
   private _editorConfig = StaticNotebook.defaultEditorConfig;
   private _notebookConfig = StaticNotebook.defaultNotebookConfig;
   private _mimetype = 'text/plain';
@@ -833,6 +878,7 @@ export class StaticNotebook extends Widget {
   private _observer: IntersectionObserver;
   private _renderedCellsCount = 0;
   private _toRenderMap: Map<string, { index: number; cell: Cell }>;
+  private _idleCallBack: any;
   private _cellsArray: Array<Cell>;
   private _renderingLayout: RenderingLayout | undefined;
 }
@@ -874,6 +920,11 @@ export namespace StaticNotebook {
      * The service used to look up mime types.
      */
     mimeTypeService: IEditorMimeTypeService;
+
+    /**
+     * The application language translator.
+     */
+    translator?: ITranslator;
   }
 
   /**
@@ -968,11 +1019,17 @@ export namespace StaticNotebook {
      */
     recordTiming: boolean;
 
+    /*
+     * Remaining time in milliseconds before
+     * virtual notebook rendering is rescheduled.
+     */
+    numberCellsToRenderDirectly: number;
+
     /*
      * Number of cells to render directly when virtual
      * notebook intersection observer is available.
      */
-    numberCellsToRenderDirectly: number;
+    remainingTimeBeforeRescheduling: number;
 
     /**
      * Defines if the placeholder cells should be rendered
@@ -1032,7 +1089,8 @@ export namespace StaticNotebook {
     scrollPastEnd: true,
     defaultCell: 'code',
     recordTiming: false,
-    numberCellsToRenderDirectly: 20,
+    numberCellsToRenderDirectly: 99999,
+    remainingTimeBeforeRescheduling: 50,
     renderCellOnIdle: true,
     observedTopMargin: '1000px',
     observedBottomMargin: '1000px',

+ 2 - 1
packages/notebook/src/widgetfactory.ts

@@ -93,7 +93,8 @@ export class NotebookWidgetFactory extends ABCWidgetFactory<
       editorConfig: source ? source.content.editorConfig : this._editorConfig,
       notebookConfig: source
         ? source.content.notebookConfig
-        : this._notebookConfig
+        : this._notebookConfig,
+      translator: this.translator
     };
     const content = this.contentFactory.createNotebook(nbOptions);
 

+ 2 - 1
packages/notebook/test/widget.spec.ts

@@ -37,7 +37,8 @@ const notebookConfig = {
   scrollPastEnd: true,
   defaultCell: 'code' as nbformat.CellType,
   recordTiming: false,
-  numberCellsToRenderDirectly: 2,
+  numberCellsToRenderDirectly: 9999,
+  remainingTimeBeforeRescheduling: 50,
   renderCellOnIdle: true,
   observedTopMargin: '1000px',
   observedBottomMargin: '1000px',