Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebased: Make Cells Display Lazily When Diffing Unchanged Cells #733

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
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
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
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);
}