diff --git a/nbdime/webapp/templates/diff.html b/nbdime/webapp/templates/diff.html index 447675eb..c0fb9cc9 100644 --- a/nbdime/webapp/templates/diff.html +++ b/nbdime/webapp/templates/diff.html @@ -17,7 +17,6 @@

Notebook Diff

- diff --git a/packages/nbdime/src/diff/widget/fold-down.svg b/packages/nbdime/src/diff/widget/fold-down.svg new file mode 100644 index 00000000..e832c1e6 --- /dev/null +++ b/packages/nbdime/src/diff/widget/fold-down.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/packages/nbdime/src/diff/widget/fold-up.svg b/packages/nbdime/src/diff/widget/fold-up.svg new file mode 100644 index 00000000..9e15e592 --- /dev/null +++ b/packages/nbdime/src/diff/widget/fold-up.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/packages/nbdime/src/diff/widget/fold.svg b/packages/nbdime/src/diff/widget/fold.svg new file mode 100644 index 00000000..9ecf663f --- /dev/null +++ b/packages/nbdime/src/diff/widget/fold.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/packages/nbdime/src/diff/widget/linked-cells.ts b/packages/nbdime/src/diff/widget/linked-cells.ts new file mode 100644 index 00000000..3656132f --- /dev/null +++ b/packages/nbdime/src/diff/widget/linked-cells.ts @@ -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 }; diff --git a/packages/nbdime/src/diff/widget/notebook.ts b/packages/nbdime/src/diff/widget/notebook.ts index cc72dedf..4d020f51 100644 --- a/packages/nbdime/src/diff/widget/notebook.ts +++ b/packages/nbdime/src/diff/widget/notebook.ts @@ -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 { @@ -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'; @@ -35,6 +37,7 @@ export class NotebookDiffWidget extends DiffPanel { super(others); this._rendermime = rendermime; this.addClass(NBDIFF_CLASS); + this.previousCell = null; } /** @@ -43,8 +46,7 @@ export class NotebookDiffWidget extends DiffPanel { * Separated from constructor to allow 'live' adding of widgets */ init(): Promise { - let model = this._model; - let rendermime = this._rendermime; + const model = this._model; let work = Promise.resolve(); work = work.then(() => { @@ -59,44 +61,10 @@ export class NotebookDiffWidget extends DiffPanel { ); } }); - for (let chunk of model.chunkedCells) { + for (const chunk of model.chunkedCells) { work = work.then(() => { return new Promise(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(() => { @@ -108,6 +76,59 @@ export class NotebookDiffWidget extends DiffPanel { 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. * @@ -119,4 +140,5 @@ export class NotebookDiffWidget extends DiffPanel { } private _rendermime: IRenderMimeRegistry; + private previousCell: LinkedListCell | null; } diff --git a/packages/nbdime/src/styles/variables.css b/packages/nbdime/src/styles/variables.css index b7c5bbf5..bdf752f3 100644 --- a/packages/nbdime/src/styles/variables.css +++ b/packages/nbdime/src/styles/variables.css @@ -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); } diff --git a/packages/nbdime/test/src/diff/widget/linked-cells.spec.ts b/packages/nbdime/test/src/diff/widget/linked-cells.spec.ts new file mode 100644 index 00000000..f75ec346 --- /dev/null +++ b/packages/nbdime/test/src/diff/widget/linked-cells.spec.ts @@ -0,0 +1,159 @@ +import { createAddedCellDiffModel } from '../../../../src/diff/model'; +import { CellDiffWidget } from '../../../../src/diff/widget'; +import * as nbformat from '@jupyterlab/nbformat'; +import { + LazyDisplayLinkedListCell, + LinkedListCell, +} from '../../../../src/diff/widget/linked-cells'; + +import { + RenderMimeRegistry, + standardRendererFactories, +} from '@jupyterlab/rendermime'; + +let codeCellA: nbformat.ICodeCell = { + cell_type: 'code', + execution_count: 2, + metadata: { + collapsed: false, + trusted: false, + }, + outputs: [], + source: 'l = f(3, 4)\nprint(l)', +}; + +function renderFunc() { + let mimetype = 'text/python'; + let model = createAddedCellDiffModel(codeCellA, mimetype); + let rendermime = new RenderMimeRegistry({ + initialFactories: standardRendererFactories, + }); + + return new CellDiffWidget(model, rendermime, mimetype); +} + +// JSDOM Does not properly support createRange in the version we are using +// This patches it https://github.com/jsdom/jsdom/issues/3002 +document.createRange = () => { + const range = new Range(); + + range.getBoundingClientRect = jest.fn(); + + range.getClientRects = jest.fn(() => ({ + item: () => null, + length: 0, + })); + + return range; +}; + +let emptyCell = `
l = f(3, 4)
{
`; + +describe('linked cells', () => { + describe('has a linked list shape with a cell', () => { + it('has a previous and next neighbor', () => { + const cell = new LinkedListCell(renderFunc); + expect(cell.prev).toBeNull(); + expect(cell.next).toBeNull(); + const next = new LinkedListCell(renderFunc); + next.prev = cell; + expect(cell.next).toBe(next); + expect(next.prev).toBe(cell); + const previous = new LinkedListCell(renderFunc); + cell.prev = previous; + expect(cell.prev).toBe(previous); + expect(previous.next).toBe(cell); + }); + }); + + it('starts displayed when not lazy', () => { + const cell = new LinkedListCell(renderFunc); + expect(cell.displayed).toBe(true); + expect(cell.node.innerHTML.toString()).toBe(emptyCell); + }); + + describe('lazy linked list cells', () => { + it('can call the callback render function', () => { + const cell = new LazyDisplayLinkedListCell(renderFunc); + expect(cell.displayed).toBe(false); + expect(cell.node.innerHTML.toString()).toBe(''); + cell.showLazyCell(); + expect(cell.displayed).toBe(true); + expect(cell.node.innerHTML.toString()).toBe(emptyCell); + }); + + it('show the expand down button when added to a non-lazy previous cell', () => { + const cell = new LazyDisplayLinkedListCell(renderFunc); + const previous = new LinkedListCell(renderFunc); + cell.prev = previous; + expect(cell.node.innerHTML.toString()).toContain('Expand Down'); + expect(cell.node.innerHTML.toString()).not.toContain('Expand Up'); + }); + + it('show the expand up button when added to a non-lazy next cell', () => { + const cell = new LazyDisplayLinkedListCell(renderFunc); + const next = new LinkedListCell(renderFunc); + next.prev = cell; + expect(cell.node.innerHTML.toString()).toContain('Expand Up'); + expect(cell.node.innerHTML.toString()).not.toContain('Expand Down'); + }); + + it('triggering the expand up button calls show expand up on the previous cell', () => { + const shown = new LinkedListCell(renderFunc); + const prev = new LazyDisplayLinkedListCell(renderFunc); + const toBeCalled = new LazyDisplayLinkedListCell(renderFunc); + shown.prev = prev; + prev.prev = toBeCalled; + + const spy = jest.spyOn(toBeCalled, 'expandUp'); + const noSpy = jest.spyOn(toBeCalled, 'expandDown'); + const noRenderSpy = jest.spyOn(toBeCalled, 'showLazyCell'); + const renderSpy = jest.spyOn(prev, 'showLazyCell'); + + prev.showLazyCellUp(); + expect(renderSpy).toHaveBeenCalled(); + expect(noSpy).not.toHaveBeenCalled(); + expect(spy).toHaveBeenCalled(); + expect(noRenderSpy).not.toHaveBeenCalled(); + }); + + it('triggering the expand down button calls show expand down on the next cell', () => { + const shown = new LinkedListCell(renderFunc); + const next = new LazyDisplayLinkedListCell(renderFunc); + const toBeCalled = new LazyDisplayLinkedListCell(renderFunc); + shown.next = next; + next.next = toBeCalled; + + const renderSpy = jest.spyOn(next, 'showLazyCell'); + const noRenderSpy = jest.spyOn(toBeCalled, 'showLazyCell'); + const spy = jest.spyOn(toBeCalled, 'expandDown'); + const noSpy = jest.spyOn(toBeCalled, 'expandUp'); + next.showLazyCellDown(); + expect(renderSpy).toHaveBeenCalled(); + expect(spy).toHaveBeenCalled(); + expect(noSpy).not.toHaveBeenCalled(); + expect(noRenderSpy).not.toHaveBeenCalled(); + }); + + it('show a fold icon if the above and below cells are displayed', () => { + const shownTop = new LinkedListCell(renderFunc); + const shownBottom = new LinkedListCell(renderFunc); + const lazy = new LazyDisplayLinkedListCell(renderFunc); + lazy.prev = shownTop; + shownBottom.prev = lazy; + expect(lazy.node.innerHTML.toString()).toContain('Fold'); + }); + + it('shows a fold icon if the above and below cells are lazy and opened', () => { + const lazyTop = new LazyDisplayLinkedListCell(renderFunc); + const lazyBottom = new LazyDisplayLinkedListCell(renderFunc); + const LazyMiddle = new LazyDisplayLinkedListCell(renderFunc); + LazyMiddle.prev = lazyTop; + lazyBottom.prev = LazyMiddle; + + lazyTop.showLazyCellDown(); + lazyBottom.showLazyCellUp(); + expect(LazyMiddle.node.innerHTML.toString()).toContain('Fold'); + }); + }); +}); diff --git a/packages/webapp/src/app/diff.css b/packages/webapp/src/app/diff.css index 01104ebb..6c232223 100644 --- a/packages/webapp/src/app/diff.css +++ b/packages/webapp/src/app/diff.css @@ -111,3 +111,30 @@ border-bottom: solid #ccc 1px; text-align: center; } + +.jp-expand-output-wrapper { + height: 40px; + width: 100%; + display: inline-block; + background: var(--jp-expand-outer-wrapper); + color: var(--jp-expand-color); + a { + cursor: pointer; + height: 100%; + display: block; + width: 40px; + background-color: var(--jp-expand-activate-color); + &:hover { + background-color: var(--jp-expand-activate-focus-color); + svg { + fill: var(--jp-expand-background-focus-color); + } + } + } + svg { + height: 23px; + width: 23px; + margin: 8px; + fill: var(--jp-expand-background-color); + } +} diff --git a/packages/webapp/src/app/diff.ts b/packages/webapp/src/app/diff.ts index 773f6a43..aa2fa2d9 100644 --- a/packages/webapp/src/app/diff.ts +++ b/packages/webapp/src/app/diff.ts @@ -22,13 +22,7 @@ import { NotebookDiffWidget } from 'nbdime/lib/diff/widget'; import { requestDiff } from 'nbdime/lib/request'; -import { - getBaseUrl, - getConfigOption, - toggleSpinner, - toggleShowUnchanged, - markUnchangedRanges, -} from './common'; +import { getBaseUrl, getConfigOption, toggleSpinner } from './common'; import { exportDiff } from './staticdiff'; @@ -83,8 +77,6 @@ function showDiff(data: { throw new Error('Missing root element "nbidme-root"'); } root.innerHTML = ''; - // Hide unchanged cells by default: - toggleShowUnchanged(!getConfigOption('hideUnchanged', true)); let panel = new Panel(); panel.id = 'main'; @@ -174,7 +166,6 @@ function onDiffRequestCompleted(data: any) { ) as HTMLButtonElement; exportBtn.style.display = 'initial'; toggleSpinner(false); - markUnchangedRanges(); }); } @@ -255,14 +246,6 @@ export function initializeDiff() { let exportBtn = document.getElementById('nbdime-export') as HTMLButtonElement; exportBtn.onclick = exportDiff; - let hideUnchangedChk = document.getElementById( - 'nbdime-hide-unchanged', - ) as HTMLInputElement; - hideUnchangedChk.checked = getConfigOption('hideUnchanged', true); - hideUnchangedChk.onchange = () => { - toggleShowUnchanged(!hideUnchangedChk.checked, diffWidget); - }; - let trustBtn = document.getElementById('nbdime-trust') as HTMLButtonElement; trustBtn.onclick = trustOutputs; trustBtn.style.display = 'initial';