From 6931bc7132032675c9330c84a5a4b4691aff497e Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 3 Feb 2021 11:33:36 -0800 Subject: [PATCH 1/2] merge stream output in rendering --- .../view/output/transforms/richTransform.ts | 2 +- .../view/output/transforms/streamTransform.ts | 4 +- .../view/output/transforms/textHelper.ts | 48 +++----------- .../browser/view/renderers/cellOutput.ts | 64 +++++++++++++++---- 4 files changed, 65 insertions(+), 53 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform.ts b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform.ts index 88deceb6225..bf27a0f05cb 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform.ts @@ -211,7 +211,7 @@ class RichRenderer implements IOutputTransformContribution { renderPlainText(output: IDisplayOutputViewModel, notebookUri: URI, container: HTMLElement): IRenderOutput { const data = output.model.data['text/plain']; const contentNode = DOM.$('.output-plaintext'); - truncatedArrayOfString(contentNode, isArray(data) ? data : [data], this.openerService, this.textFileService, this.themeService, true); + truncatedArrayOfString(contentNode, isArray(data) ? data : [data], this.openerService, this.textFileService, this.themeService); container.appendChild(contentNode); return { type: RenderOutputType.None, hasDynamicHeight: false }; diff --git a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/streamTransform.ts b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/streamTransform.ts index 5f6f9b7737a..17276e2c6b7 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/streamTransform.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/streamTransform.ts @@ -23,8 +23,8 @@ class StreamRenderer implements IOutputTransformContribution { render(viewModel: IStreamOutputViewModel, container: HTMLElement): IRenderOutput { const output = viewModel.model; - const contentNode = DOM.$('.output-stream'); - truncatedArrayOfString(contentNode, [output.text], this.openerService, this.textFileService, this.themeService, false); + const contentNode = DOM.$('span.output-stream'); + truncatedArrayOfString(contentNode, [output.text], this.openerService, this.textFileService, this.themeService); container.appendChild(contentNode); return { type: RenderOutputType.None, hasDynamicHeight: false }; } diff --git a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/textHelper.ts b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/textHelper.ts index c8dc7731886..635b079bc84 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/output/transforms/textHelper.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/output/transforms/textHelper.ts @@ -49,7 +49,7 @@ function generateViewMoreElement(outputs: string[], openerService: IOpenerServic return element; } -export function truncatedArrayOfString(container: HTMLElement, outputs: string[], openerService: IOpenerService, textFileService: ITextFileService, themeService: IThemeService, renderANSI: boolean) { +export function truncatedArrayOfString(container: HTMLElement, outputs: string[], openerService: IOpenerService, textFileService: ITextFileService, themeService: IThemeService) { const fullLen = outputs.reduce((p, c) => { return p + c.length; }, 0); @@ -65,14 +65,7 @@ export function truncatedArrayOfString(container: HTMLElement, outputs: string[] const sizeBufferLimitPosition = buffer.getPositionAt(SIZE_LIMIT); if (sizeBufferLimitPosition.lineNumber < LINES_LIMIT) { const truncatedText = buffer.getValueInRange(new Range(1, 1, sizeBufferLimitPosition.lineNumber, sizeBufferLimitPosition.column), EndOfLinePreference.TextDefined); - if (renderANSI) { - container.appendChild(handleANSIOutput(truncatedText, themeService)); - } else { - const pre = DOM.$('pre'); - pre.innerText = truncatedText; - container.appendChild(pre); - } - + container.appendChild(handleANSIOutput(truncatedText, themeService)); // view more ... container.appendChild(generateViewMoreElement(outputs, openerService, textFileService)); return; @@ -89,42 +82,19 @@ export function truncatedArrayOfString(container: HTMLElement, outputs: string[] if (buffer.getLineCount() < LINES_LIMIT) { const lineCount = buffer.getLineCount(); const fullRange = new Range(1, 1, lineCount, Math.max(1, buffer.getLineLastNonWhitespaceColumn(lineCount))); - - if (renderANSI) { - const pre = DOM.$('pre'); - container.appendChild(pre); - - pre.appendChild(handleANSIOutput(buffer.getValueInRange(fullRange, EndOfLinePreference.TextDefined), themeService)); - } else { - const pre = DOM.$('pre'); - container.appendChild(pre); - pre.innerText = buffer.getValueInRange(fullRange, EndOfLinePreference.TextDefined); - } + container.appendChild(handleANSIOutput(buffer.getValueInRange(fullRange, EndOfLinePreference.TextDefined), themeService)); return; } - if (renderANSI) { - const pre = DOM.$('pre'); - container.appendChild(pre); - pre.appendChild(handleANSIOutput(buffer.getValueInRange(new Range(1, 1, LINES_LIMIT - 5, buffer.getLineLastNonWhitespaceColumn(LINES_LIMIT - 5)), EndOfLinePreference.TextDefined), themeService)); - } else { - const pre = DOM.$('pre'); - pre.innerText = buffer.getValueInRange(new Range(1, 1, LINES_LIMIT - 5, buffer.getLineLastNonWhitespaceColumn(LINES_LIMIT - 5)), EndOfLinePreference.TextDefined); - container.appendChild(pre); - } - + const pre = DOM.$('pre'); + container.appendChild(pre); + pre.appendChild(handleANSIOutput(buffer.getValueInRange(new Range(1, 1, LINES_LIMIT - 5, buffer.getLineLastNonWhitespaceColumn(LINES_LIMIT - 5)), EndOfLinePreference.TextDefined), themeService)); // view more ... container.appendChild(generateViewMoreElement(outputs, openerService, textFileService)); const lineCount = buffer.getLineCount(); - if (renderANSI) { - const pre = DOM.$('div'); - container.appendChild(pre); - pre.appendChild(handleANSIOutput(buffer.getValueInRange(new Range(lineCount - 5, 1, lineCount, buffer.getLineLastNonWhitespaceColumn(lineCount)), EndOfLinePreference.TextDefined), themeService)); - } else { - const post = DOM.$('div'); - post.innerText = buffer.getValueInRange(new Range(lineCount - 5, 1, lineCount, buffer.getLineLastNonWhitespaceColumn(lineCount)), EndOfLinePreference.TextDefined); - container.appendChild(post); - } + const pre2 = DOM.$('div'); + container.appendChild(pre2); + pre2.appendChild(handleANSIOutput(buffer.getValueInRange(new Range(lineCount - 5, 1, lineCount, buffer.getLineLastNonWhitespaceColumn(lineCount)), EndOfLinePreference.TextDefined), themeService)); } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts index bca1187cd94..e56c4367c5f 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellOutput.ts @@ -36,6 +36,16 @@ export class OutputElement extends Disposable { domNode!: HTMLElement; renderResult?: IRenderOutput; + public useDedicatedDOM: boolean = true; + + get domClientHeight() { + if (this.useDedicatedDOM) { + return this.domNode.clientHeight; + } else { + return 0; + } + } + constructor( private notebookEditor: INotebookEditor, private notebookService: INotebookService, @@ -47,6 +57,17 @@ export class OutputElement extends Disposable { super(); } + detach() { + this.domNode.parentElement?.removeChild(this.domNode); + } + + updateDOMTop(top: number) { + if (this.useDedicatedDOM) { + this.domNode.style.top = `${top}px`; + } + } + + render(index: number, beforeElement?: HTMLElement) { if (this.viewCell.metadata.outputCollapsed) { return; @@ -58,10 +79,11 @@ export class OutputElement extends Disposable { const notebookTextModel = this.notebookEditor.viewModel.notebookDocument; - const outputItemDiv = document.createElement('div'); + let outputItemDiv; let result: IRenderOutput | undefined = undefined; if (this.output.isDisplayOutput()) { + outputItemDiv = document.createElement('div'); const [mimeTypes, pick] = this.output.resolveMimeTypes(notebookTextModel); if (mimeTypes.length > 1) { outputItemDiv.style.position = 'relative'; @@ -106,8 +128,27 @@ export class OutputElement extends Disposable { this.output.pickedMimeType = pick; } + } else if (this.output.isStreamOutput()) { + if (!beforeElement && this.outputContainer.lastChild && (this.outputContainer.lastChild).classList.contains('stream-output') && this.output.isStreamOutput()) { + this.useDedicatedDOM = false; + // the previous output and this one are both stream output + outputItemDiv = this.outputContainer.lastChild as HTMLElement; + const innerContainer = outputItemDiv.lastChild && (outputItemDiv.lastChild).classList.contains('output-inner-container') ? outputItemDiv.lastChild as HTMLElement : document.createElement('div'); + outputItemDiv.classList.add('stream-output'); + DOM.append(outputItemDiv, innerContainer); + + result = this.notebookEditor.getOutputRenderer().render(this.output, innerContainer, undefined, this.getNotebookUri(),); + } else { + outputItemDiv = document.createElement('div'); + const innerContainer = DOM.$('.output-inner-container'); + outputItemDiv.classList.add('stream-output'); + DOM.append(outputItemDiv, innerContainer); + + result = this.notebookEditor.getOutputRenderer().render(this.output, innerContainer, undefined, this.getNotebookUri(),); + } } else { // for text and error, there is no mimetype + outputItemDiv = document.createElement('div'); const innerContainer = DOM.$('.output-inner-container'); DOM.append(outputItemDiv, innerContainer); @@ -124,7 +165,7 @@ export class OutputElement extends Disposable { if (beforeElement) { this.outputContainer.insertBefore(outputItemDiv, beforeElement); - } else { + } else if (this.useDedicatedDOM) { this.outputContainer.appendChild(outputItemDiv); } @@ -165,11 +206,13 @@ export class OutputElement extends Disposable { this.resizeListener.add(elementSizeObserver); this.viewCell.updateOutputHeight(index, clientHeight); } else if (result.type === RenderOutputType.None) { // no-op if it's a webview - const clientHeight = Math.ceil(outputItemDiv.clientHeight); - this.viewCell.updateOutputHeight(index, clientHeight); + if (this.useDedicatedDOM) { + const clientHeight = Math.ceil(outputItemDiv.clientHeight); + this.viewCell.updateOutputHeight(index, clientHeight); - const top = this.viewCell.getOutputOffsetInContainer(index); - outputItemDiv.style.top = `${top}px`; + const top = this.viewCell.getOutputOffsetInContainer(index); + outputItemDiv.style.top = `${top}px`; + } } } @@ -268,7 +311,7 @@ export class OutputContainer extends Disposable { const index = viewCell.outputsViewModels.indexOf(key); if (index >= 0) { const top = this.viewCell.getOutputOffsetInContainer(index); - value.domNode.style.top = `${top}px`; + value.updateDOMTop(top); } }); })); @@ -330,8 +373,7 @@ export class OutputContainer extends Disposable { if (renderedOutput.renderResult.type !== RenderOutputType.None) { this.notebookEditor.createInset(this.viewCell, renderedOutput.renderResult as IInsetRenderOutput, this.viewCell.getOutputOffset(index)); } else { - // Anything else, just update the height - this.viewCell.updateOutputHeight(index, renderedOutput.domNode.clientHeight); + this.viewCell.updateOutputHeight(index, renderedOutput.domClientHeight); } } else { // Wasn't previously rendered, render it now @@ -352,7 +394,7 @@ export class OutputContainer extends Disposable { this.viewCell.outputsViewModels.forEach((o, i) => { const renderedOutput = this.outputEntries.get(o); if (renderedOutput && renderedOutput.renderResult && renderedOutput.renderResult.type === RenderOutputType.None && !renderedOutput.renderResult.hasDynamicHeight) { - this.viewCell.updateOutputHeight(i, renderedOutput.domNode.clientHeight); + this.viewCell.updateOutputHeight(i, renderedOutput.domClientHeight); } }); } @@ -420,7 +462,7 @@ export class OutputContainer extends Disposable { // already removed removedKeys.push(key); // remove element from DOM - this.templateData?.outputContainer?.removeChild(value.domNode); + value.detach(); if (key.isDisplayOutput()) { this.notebookEditor.removeInset(key); } From 4beba243a032ceee2bb85c5a304c3769ded254f5 Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 3 Feb 2021 11:47:46 -0800 Subject: [PATCH 2/2] fix missing total height change event. --- .../contrib/notebook/browser/view/renderers/codeCell.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts index f22e6fea28c..74e072419b0 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts @@ -122,6 +122,12 @@ export class CodeCell extends Disposable { } })); + this._register(viewCell.onDidChangeLayout((e) => { + if (e.totalHeight) { + this.relayoutCell(); + } + })); + this._register(templateData.editor.onDidContentSizeChange((e) => { if (e.contentHeightChanged) { if (this.viewCell.layoutInfo.editorHeight !== e.contentHeight) {