Skip to content

Commit

Permalink
Make Cells Display Lazily When Diffing Unchanged Cells
Browse files Browse the repository at this point in the history
Motivation:
  During diffing Unchanged cells are displayed, which is not necessary.
  This is a performance issue when there are many unchanged cells.

Related Issues:
  - jupyter#635

Open Questions:
  - I am struggling with getting the svg images to build properly.
  - How can this be tested in JupyterLab?

Changes:
  - Alter Notebooks Widget to only display changed cells by using new linked
    list of cells.
  - Add new linked list of cells and lazy version of linked list of cells.
  • Loading branch information
gwincr11 authored and vidartf committed Nov 10, 2023
1 parent 554c2d3 commit 1d0d884
Show file tree
Hide file tree
Showing 10 changed files with 479 additions and 58 deletions.
1 change: 0 additions & 1 deletion nbdime/webapp/templates/diff.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ <h3>Notebook Diff</h3>
</fieldset>
</form> <!-- nbdime-forms -->
<div id="nbdime-header-buttonrow">
<label><input id="nbdime-hide-unchanged" type="checkbox"></input>Hide unchanged cells</label>
<button id="nbdime-trust" style="display: none">Trust outputs</button>
<button id="nbdime-close" type="checkbox" style="display: none">Close tool</button>
<button id="nbdime-export" type="checkbox" style="display: none">Export diff</button>
Expand Down
1 change: 1 addition & 0 deletions packages/nbdime/src/diff/widget/fold-down.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions packages/nbdime/src/diff/widget/fold-up.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions packages/nbdime/src/diff/widget/fold.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
222 changes: 222 additions & 0 deletions packages/nbdime/src/diff/widget/linked-cells.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
import { Panel, Widget } from '@lumino/widgets';
import type { CellDiffWidget } from './';
import { LabIcon } from '@jupyterlab/ui-components';

import foldDown from './fold-down.svg';
import foldUp from './fold-up.svg';
import fold from './fold.svg';

export const foldDownIcon = new LabIcon({
name: 'nbdime:fold-down',
svgstr: foldDown,
});

export const foldUpIcon = new LabIcon({
name: 'nbdime:fold-up',
svgstr: foldUp,
});

export const foldIcon = new LabIcon({
name: 'nbdime:fold',
svgstr: fold,
});

export interface ILinkedListCell {
_next: ILinkedListCell | null;
_prev: ILinkedListCell | null;
displayed: boolean;
lazy: boolean;
expandUp: () => void;
expandDown: () => void;
}

class LinkedListCell extends Panel implements ILinkedListCell {
renderFunc: () => CellDiffWidget;
displayed: boolean;
_next: ILinkedListCell | null;
_prev: ILinkedListCell | null;
lazy: boolean;

constructor(renderFunc: () => CellDiffWidget) {
super();
this._next = null;
this._prev = null;
this.renderFunc = renderFunc;
this.displayed = true;
this.renderCell();
this.addClass('linked-cell');
this.lazy = false;
}

protected renderCell() {
this.addWidget(this.renderFunc());
this.displayed = true;
}

get next() {
return this._next;
}

set next(nextCell) {
this._next = nextCell;
if (nextCell === null) {
return;
}

if (nextCell.lazy) {
nextCell.expandDown();
}
}

get prev() {
return this._prev;
}

set prev(prevCell) {
this._prev = prevCell;
if (prevCell === null) {
return;
}
prevCell._next = this as ILinkedListCell;
if (prevCell.lazy) {
prevCell.expandUp();
}
}

expandUp(): void {
return;
}

expandDown(): void {
return;
}
}

class LazyDisplayLinkedListCell extends LinkedListCell {
expandButton: HTMLDivElement;
expandButtonDisplayed: boolean;

// Path: packages/nbdime/src/diff/widget/wrapper_cells.ts
constructor(renderFunc: () => CellDiffWidget) {
super(renderFunc);
this.expandButton = document.createElement('div');
this.expandButton.className = 'jp-expand-output-wrapper';
this.expandButtonDisplayed = false;
this.addClass('lazy-linked-cell');
this.lazy = true;
}

set prev(prevCell: LinkedListCell | LazyDisplayLinkedListCell) {
this._prev = prevCell;
prevCell.next = this;
}

set next(nextCell: LinkedListCell | LazyDisplayLinkedListCell) {
this._next = nextCell;
}

protected renderCell() {
this.displayed = false;
}

expandUp(): void {
if (this.displayed) {
return;
}
if (this.expandButtonDisplayed) {
this._setupFoldButton();
} else {
this._setupExpandUpButton();
}
}

expandDown(): void {
if (this.displayed) {
return;
}
if (this.expandButtonDisplayed) {
this._setupFoldButton();
} else {
this._setupExpandDownButton();
}
}

_setupFoldButton() {
this.expandButton.innerHTML = '';
const button = this.createButton('Fold');
button.onclick = e => {
e.preventDefault();
this.showLazyCellUp();
};
this.expandButton.appendChild(button);
const widget = new Widget({ node: this.expandButton });
this.addWidget(widget);
}

_setupExpandUpButton() {
const button = this.createButton('Up');
button.onclick = e => {
e.preventDefault();
this.showLazyCellUp();
};
this.expandButton.appendChild(button);
const widget = new Widget({ node: this.expandButton });
this.addWidget(widget);
}

_setupExpandDownButton() {
const button = this.createButton('Down');
button.onclick = e => {
e.preventDefault();
this.showLazyCellDown();
};
this.expandButton.appendChild(button);
const widget = new Widget({ node: this.expandButton });
this.addWidget(widget);
}

createButton(direction: 'Up' | 'Down' | 'Fold'): HTMLAnchorElement {
this.expandButton.innerHTML = '';
const button = document.createElement('a');
button.title = `Expand ${direction}`;
button.setAttribute('aria-label', `Expand ${direction}`);
let icon = this.buttonSvg(direction);
icon.element({
container: button,
});
this.expandButtonDisplayed = true;
return button;
}

buttonSvg(direction: 'Up' | 'Down' | 'Fold'): LabIcon {
if (direction === 'Up') {
return foldUpIcon;
} else if (direction === 'Down') {
return foldDownIcon;
} else {
return foldIcon;
}
}

showLazyCellUp() {
this.showLazyCell();
if (this._prev) {
this._prev.expandUp();
}
}

showLazyCellDown() {
this.showLazyCell();
if (this._next) {
this._next.expandDown();
}
}

showLazyCell() {
this.addWidget(this.renderFunc());
this.displayed = true;
this.expandButton.remove();
}
}

export { LinkedListCell, LazyDisplayLinkedListCell };
100 changes: 61 additions & 39 deletions packages/nbdime/src/diff/widget/notebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { Panel } from '@lumino/widgets';

import type { IRenderMimeRegistry } from '@jupyterlab/rendermime';

import { LazyDisplayLinkedListCell, LinkedListCell } from './linked-cells';

import { CellDiffWidget } from './cell';

import {
Expand All @@ -20,7 +22,7 @@ import { DiffPanel } from '../../common/basepanel';

import type { IMimeDiffWidgetOptions } from '../../common/interfaces';

import type { NotebookDiffModel } from '../model';
import type { NotebookDiffModel, CellDiffModel } from '../model';

const NBDIFF_CLASS = 'jp-Notebook-diff';

Expand All @@ -35,6 +37,7 @@ export class NotebookDiffWidget extends DiffPanel<NotebookDiffModel> {
super(others);
this._rendermime = rendermime;
this.addClass(NBDIFF_CLASS);
this.previousCell = null;
}

/**
Expand All @@ -43,8 +46,7 @@ export class NotebookDiffWidget extends DiffPanel<NotebookDiffModel> {
* Separated from constructor to allow 'live' adding of widgets
*/
init(): Promise<void> {
let model = this._model;
let rendermime = this._rendermime;
const model = this._model;

let work = Promise.resolve();
work = work.then(() => {
Expand All @@ -59,44 +61,10 @@ export class NotebookDiffWidget extends DiffPanel<NotebookDiffModel> {
);
}
});
for (let chunk of model.chunkedCells) {
for (const chunk of model.chunkedCells) {
work = work.then(() => {
return new Promise<void>(resolve => {
if (chunk.length === 1 && !(chunk[0].added || chunk[0].deleted)) {
this.addWidget(
new CellDiffWidget({
model: chunk[0],
rendermime,
mimetype: model.mimetype,
editorFactory: this._editorFactory,
translator: this._translator,
...this._viewOptions,
}),
);
} else {
let chunkPanel = new Panel();
chunkPanel.addClass(CHUNK_PANEL_CLASS);
let addedPanel = new Panel();
addedPanel.addClass(ADDED_CHUNK_PANEL_CLASS);
let removedPanel = new Panel();
removedPanel.addClass(REMOVED_CHUNK_PANEL_CLASS);
for (let cell of chunk) {
let target = cell.deleted ? removedPanel : addedPanel;
target.addWidget(
new CellDiffWidget({
model: cell,
rendermime,
mimetype: model.mimetype,
editorFactory: this._editorFactory,
translator: this._translator,
...this._viewOptions,
}),
);
}
chunkPanel.addWidget(addedPanel);
chunkPanel.addWidget(removedPanel);
this.addWidget(chunkPanel);
}
this.addDiffChunk(chunk);
// This limits us to drawing 60 cells per second, which shouldn't
// be a problem...
requestAnimationFrame(() => {
Expand All @@ -108,6 +76,59 @@ export class NotebookDiffWidget extends DiffPanel<NotebookDiffModel> {
return work;
}

private addDiffChunk(chunk: CellDiffModel[]): void {
if (chunk.length === 1 && !(chunk[0].added || chunk[0].deleted)) {
this.addWidget(this.addCellPanel(chunk[0]));
} else {
this.addChunkPanel(chunk);
}
}

private addChunkPanel(chunk: CellDiffModel[]): void {
let chunkPanel = new Panel();
chunkPanel.addClass(CHUNK_PANEL_CLASS);
let addedPanel = new Panel();
addedPanel.addClass(ADDED_CHUNK_PANEL_CLASS);
let removedPanel = new Panel();
removedPanel.addClass(REMOVED_CHUNK_PANEL_CLASS);
for (let cell of chunk) {
const target = cell.deleted ? removedPanel : addedPanel;
target.addWidget(this.addCellPanel(cell));
}
chunkPanel.addWidget(addedPanel);
chunkPanel.addWidget(removedPanel);
this.addWidget(chunkPanel);
}

private addCellPanel(
cell: CellDiffModel,
): LinkedListCell | LazyDisplayLinkedListCell {
let linkedCell: LinkedListCell | LazyDisplayLinkedListCell;
if (cell.unchanged) {
linkedCell = new LazyDisplayLinkedListCell(() =>
this.creatCellWidget(cell),
);
} else {
linkedCell = new LinkedListCell(() => this.creatCellWidget(cell));
}
if (this.previousCell) {
linkedCell.prev = this.previousCell;
}
this.previousCell = linkedCell;
return linkedCell;
}

creatCellWidget(cell: CellDiffModel): CellDiffWidget {
return new CellDiffWidget({
model: cell,
rendermime: this._rendermime,
mimetype: this._model.mimetype,
editorFactory: this._editorFactory,
translator: this._translator,
...this._viewOptions,
});
}

/**
* Get the model for the widget.
*
Expand All @@ -119,4 +140,5 @@ export class NotebookDiffWidget extends DiffPanel<NotebookDiffModel> {
}

private _rendermime: IRenderMimeRegistry;
private previousCell: LinkedListCell | null;
}
6 changes: 6 additions & 0 deletions packages/nbdime/src/styles/variables.css
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,10 @@

--jp-merge-either-color1: #aee;
--jp-merge-either-color2: #cff4f4;
--jp-expand-outer-wrapper: rgb(221, 244, 255, 1);
--jp-expand-color: rgb(87, 96, 106, 1);
--jp-expand-activate-color: rgba(84, 174, 255, 0.4);
--jp-expand-activate-focus-color: rgb(14, 94, 208, 1);
--jp-expand-background-color: rgb(87, 96, 106, 1);
--jp-expand-background-focus-color: rgb(255, 255, 255, 1);
}
Loading

0 comments on commit 1d0d884

Please sign in to comment.