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

toggle scrollable outputs by cell #179212

Merged
merged 7 commits into from Apr 5, 2023
Merged
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
31 changes: 12 additions & 19 deletions extensions/notebook-renderers/src/index.ts
Expand Up @@ -145,18 +145,14 @@ function renderError(
if (err.stack) {
outputElement.classList.add('traceback');

const outputScrolling = ctx.settings.outputScrolling;
const outputScrolling = scrollingEnabled(outputInfo, ctx.settings);
const content = createOutputContent(outputInfo.id, [err.stack ?? ''], ctx.settings.lineLimit, outputScrolling, true);
const contentParent = document.createElement('div');
contentParent.classList.toggle('word-wrap', ctx.settings.outputWordWrap);
disposableStore.push(ctx.onDidChangeSettings(e => {
contentParent.classList.toggle('word-wrap', e.outputWordWrap);
}));
contentParent.classList.toggle('scrollable', outputScrolling);
outputElement.classList.toggle('hide-refresh', !outputScrolling);
disposableStore.push(ctx.onDidChangeSettings(e => {
outputElement.classList.toggle('hide-refresh', !e.outputScrolling);
}));
outputElement.classList.toggle('remove-padding', outputScrolling);

contentParent.appendChild(content);
Expand Down Expand Up @@ -221,9 +217,13 @@ function findScrolledHeight(scrollableElement: HTMLElement): number | undefined
return undefined;
}

function scrollingEnabled(output: OutputItem, options: RenderOptions) {
return output.metadata?.scrollable !== undefined ? output.metadata?.scrollable : options.outputScrolling;
}

function renderStream(outputInfo: OutputItem, outputElement: HTMLElement, error: boolean, ctx: IRichRenderContext): IDisposable {
const disposableStore = createDisposableStore();
const outputScrolling = ctx.settings.outputScrolling;
const outputScrolling = scrollingEnabled(outputInfo, ctx.settings);

outputElement.classList.add('output-stream');
outputElement.classList.toggle('remove-padding', outputScrolling);
Expand Down Expand Up @@ -252,11 +252,6 @@ function renderStream(outputInfo: OutputItem, outputElement: HTMLElement, error:
const contentParent = document.createElement('div');
contentParent.appendChild(content);
contentParent.classList.toggle('scrollable', outputScrolling);
outputElement.classList.toggle('hide-refresh', !outputScrolling);
disposableStore.push(ctx.onDidChangeSettings(e => {
outputElement.classList.toggle('hide-refresh', !e.outputScrolling);
}));

contentParent.classList.toggle('word-wrap', ctx.settings.outputWordWrap);
disposableStore.push(ctx.onDidChangeSettings(e => {
contentParent.classList.toggle('word-wrap', e.outputWordWrap);
Expand All @@ -278,18 +273,14 @@ function renderText(outputInfo: OutputItem, outputElement: HTMLElement, ctx: IRi
clearContainer(outputElement);

const text = outputInfo.text();
const outputScrolling = scrollingEnabled(outputInfo, ctx.settings);
const content = createOutputContent(outputInfo.id, [text], ctx.settings.lineLimit, ctx.settings.outputScrolling, false);
content.classList.add('output-plaintext');
if (ctx.settings.outputWordWrap) {
content.classList.add('word-wrap');
}

const outputScrolling = ctx.settings.outputScrolling;
content.classList.toggle('scrollable', outputScrolling);
outputElement.classList.toggle('hide-refresh', !outputScrolling);
disposableStore.push(ctx.onDidChangeSettings(e => {
outputElement.classList.toggle('hide-refresh', !e.outputScrolling);
}));
outputElement.classList.toggle('remove-padding', outputScrolling);
outputElement.appendChild(content);
initializeScroll(content, disposableStore);
Expand Down Expand Up @@ -340,6 +331,11 @@ export const activate: ActivationFunction<void> = (ctx) => {
border-width: 1px;
border-color: transparent;
}
#container div.truncation-message {
font-style: italic;
font-family: var(--theme-font-family);
padding-top: 4px;
}
#container div.output .scrollable div {
cursor: text;
}
Expand All @@ -352,9 +348,6 @@ export const activate: ActivationFunction<void> = (ctx) => {
#container div.output .scrollable.scrollbar-visible {
border-color: var(--vscode-editorWidget-border);
}
#container div.output.hide-refresh .scroll-refresh {
display: none;
}
.output-plaintext .code-bold,
.output-stream .code-bold,
.traceback .code-bold {
Expand Down
52 changes: 29 additions & 23 deletions extensions/notebook-renderers/src/textHelper.ts
Expand Up @@ -7,37 +7,42 @@ import { handleANSIOutput } from './ansi';

export const scrollableClass = 'scrollable';

/**
* Output is Truncated. View as a [scrollable element] or open in a [text editor]. Adjust cell output [settings...]
*/
function generateViewMoreElement(outputId: string) {

const container = document.createElement('div');
container.classList.add('truncation-message');
const first = document.createElement('span');
first.textContent = 'Output exceeds the ';

const second = document.createElement('a');
second.textContent = 'size limit';
second.href = `command:workbench.action.openSettings?%5B%22notebook.output.textLineLimit%22%5D`;
first.textContent = 'Output is Truncated. View as a ';
container.appendChild(first);

const viewAsScrollableLink = document.createElement('a');
viewAsScrollableLink.textContent = 'scrollable element';
viewAsScrollableLink.href = `command:cellOutput.enableScrolling?${outputId}`;
viewAsScrollableLink.ariaLabel = 'enable scrollable output';
container.appendChild(viewAsScrollableLink);

const second = document.createElement('span');
second.textContent = ' or open in a ';
container.appendChild(second);

const third = document.createElement('span');
third.textContent = '. Open the full output data ';
const openInTextEditorLink = document.createElement('a');
openInTextEditorLink.textContent = 'text editor';
openInTextEditorLink.href = `command:workbench.action.openLargeOutput?${outputId}`;
openInTextEditorLink.ariaLabel = 'open output in text editor';
container.appendChild(openInTextEditorLink);

const forth = document.createElement('a');
forth.textContent = 'in a text editor';
forth.href = `command:workbench.action.openLargeOutput?${outputId}`;
const third = document.createElement('span');
third.textContent = '. Adjust cell output ';
container.appendChild(third);
container.appendChild(forth);

const refreshSpan = document.createElement('span');
refreshSpan.classList.add('scroll-refresh');
const fifth = document.createElement('span');
fifth.textContent = '. Refresh to view ';

const sixth = document.createElement('a');
sixth.textContent = 'scrollable element';
sixth.href = `command:cellOutput.enableScrolling?${outputId}`;
refreshSpan.appendChild(fifth);
refreshSpan.appendChild(sixth);
container.appendChild(refreshSpan);
const layoutSettingsLink = document.createElement('a');
layoutSettingsLink.textContent = 'settings...';
layoutSettingsLink.href = `command:workbench.action.openSettings?%5B%22%40tag%3AnotebookOutputLayout%22%5D`;
layoutSettingsLink.ariaLabel = 'notebook output settings';
container.appendChild(layoutSettingsLink);

return container;
}
Expand Down Expand Up @@ -66,7 +71,6 @@ function truncatedArrayOfString(id: string, buffer: string[], linesLimit: number
return container;
}

container.appendChild(generateViewMoreElement(id));
container.appendChild(handleANSIOutput(buffer.slice(0, linesLimit - 5).join('\n'), trustHtml));

// truncated piece
Expand All @@ -76,6 +80,8 @@ function truncatedArrayOfString(id: string, buffer: string[], linesLimit: number

container.appendChild(handleANSIOutput(buffer.slice(lineCount - 5).join('\n'), trustHtml));

container.appendChild(generateViewMoreElement(id));

return container;
}

Expand Down
Expand Up @@ -15,12 +15,13 @@ import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegis
import { ResourceNotebookCellEdit } from 'vs/workbench/contrib/bulkEdit/browser/bulkCellEdits';
import { changeCellToKind, computeCellLinesContents, copyCellRange, joinCellsWithSurrounds, joinSelectedCells, moveCellRange } from 'vs/workbench/contrib/notebook/browser/controller/cellOperations';
import { cellExecutionArgs, CellOverflowToolbarGroups, CellToolbarOrder, CELL_TITLE_CELL_GROUP_ID, INotebookCellActionContext, INotebookCellToolbarActionContext, INotebookCommandContext, NotebookCellAction, NotebookMultiCellAction, parseMultiCellExecutionArgs } from 'vs/workbench/contrib/notebook/browser/controller/coreActions';
import { CellFocusMode, EXPAND_CELL_INPUT_COMMAND_ID, EXPAND_CELL_OUTPUT_COMMAND_ID, ICellViewModel, INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { CellFocusMode, EXPAND_CELL_INPUT_COMMAND_ID, EXPAND_CELL_OUTPUT_COMMAND_ID, ICellOutputViewModel, ICellViewModel, INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { NOTEBOOK_CELL_EDITABLE, NOTEBOOK_CELL_HAS_OUTPUTS, NOTEBOOK_CELL_INPUT_COLLAPSED, NOTEBOOK_CELL_LIST_FOCUSED, NOTEBOOK_CELL_OUTPUT_COLLAPSED, NOTEBOOK_CELL_TYPE, NOTEBOOK_EDITOR_EDITABLE, NOTEBOOK_EDITOR_FOCUSED, NOTEBOOK_IS_ACTIVE_EDITOR, NOTEBOOK_OUTPUT_FOCUSED } from 'vs/workbench/contrib/notebook/common/notebookContextKeys';
import * as icons from 'vs/workbench/contrib/notebook/browser/notebookIcons';
import { CellEditType, CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { CellEditType, CellKind, NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { INotificationService } from 'vs/platform/notification/common/notification';
import { EditorContextKeys } from 'vs/editor/common/editorContextKeys';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';

//#region Move/Copy cells
const MOVE_CELL_UP_COMMAND_ID = 'notebook.cell.moveUp';
Expand Down Expand Up @@ -381,6 +382,7 @@ const EXPAND_ALL_CELL_INPUTS_COMMAND_ID = 'notebook.cell.expandAllCellInputs';
const COLLAPSE_ALL_CELL_OUTPUTS_COMMAND_ID = 'notebook.cell.collapseAllCellOutputs';
const EXPAND_ALL_CELL_OUTPUTS_COMMAND_ID = 'notebook.cell.expandAllCellOutputs';
const TOGGLE_CELL_OUTPUTS_COMMAND_ID = 'notebook.cell.toggleOutputs';
const TOGGLE_CELL_OUTPUT_SCROLLING = 'notebook.cell.toggleOutputScrolling';

registerAction2(class CollapseCellInputAction extends NotebookMultiCellAction {
constructor() {
Expand Down Expand Up @@ -592,6 +594,51 @@ registerAction2(class ExpandAllCellOutputsAction extends NotebookMultiCellAction
}
});

registerAction2(class ToggleCellOutputScrolling extends NotebookMultiCellAction {
constructor() {
super({
id: TOGGLE_CELL_OUTPUT_SCROLLING,
title: {
value: localize('notebookActions.toggleScrolling', "Toggle Scroll Cell Output"),
original: 'Toggle Scroll Cell Output'
},
keybinding: {
when: ContextKeyExpr.and(NOTEBOOK_CELL_LIST_FOCUSED, InputFocusedContext.toNegated(), NOTEBOOK_CELL_HAS_OUTPUTS),
primary: KeyChord(KeyMod.CtrlCmd | KeyCode.KeyK, KeyCode.KeyY),
weight: KeybindingWeight.WorkbenchContrib
}
});
}

private toggleOutputScrolling(viewModel: ICellOutputViewModel, globalScrollSetting: boolean, collapsed: boolean) {
const cellMetadata = viewModel.model.metadata;
// TODO: when is cellMetadata undefined? Is that a case we need to support? It is currently a read-only property.
if (cellMetadata) {
const currentlyEnabled = cellMetadata['scrollable'] !== undefined ? cellMetadata['scrollable'] : globalScrollSetting;
const shouldEnableScrolling = collapsed || !currentlyEnabled;
cellMetadata['scrollable'] = shouldEnableScrolling;
viewModel.resetRenderer();
}
}

async runWithContext(accessor: ServicesAccessor, context: INotebookCommandContext | INotebookCellToolbarActionContext): Promise<void> {
const globalScrolling = accessor.get(IConfigurationService).getValue<boolean>(NotebookSetting.outputScrolling);
if (context.ui) {
context.cell.outputsViewModels.forEach((viewModel) => {
this.toggleOutputScrolling(viewModel, globalScrolling, context.cell.isOutputCollapsed);
});
context.cell.isOutputCollapsed = false;
} else {
context.selectedCells.forEach(cell => {
cell.outputsViewModels.forEach((viewModel) => {
this.toggleOutputScrolling(viewModel, globalScrolling, cell.isOutputCollapsed);
});
cell.isOutputCollapsed = false;
});
}
}
});

//#endregion

function forEachCell(editor: INotebookEditor, callback: (cell: ICellViewModel, index: number) => void) {
Expand Down
Expand Up @@ -862,7 +862,7 @@ configurationRegistry.registerConfiguration({
markdownDescription: nls.localize('notebook.textOutputLineLimit', "Controls how many lines of text are displayed in a text output. If {0} is enabled, this setting is used to determine the scroll height of the output.", '`#notebook.output.scrolling#`'),
type: 'number',
default: 30,
tags: ['notebookLayout']
tags: ['notebookLayout', 'notebookOutputLayout']
},
[NotebookSetting.markupFontSize]: {
markdownDescription: nls.localize('notebook.markup.fontSize', "Controls the font size in pixels of rendered markup in notebooks. When set to {0}, 120% of {1} is used.", '`0`', '`#editor.fontSize#`'),
Expand All @@ -881,29 +881,29 @@ configurationRegistry.registerConfiguration({
markdownDescription: nls.localize('notebook.outputLineHeight', "Line height of the output text within notebook cells.\n - When set to 0, editor line height is used.\n - Values between 0 and 8 will be used as a multiplier with the font size.\n - Values greater than or equal to 8 will be used as effective values."),
type: 'number',
default: 0,
tags: ['notebookLayout']
tags: ['notebookLayout', 'notebookOutputLayout']
},
[NotebookSetting.outputFontSize]: {
markdownDescription: nls.localize('notebook.outputFontSize', "Font size for the output text within notebook cells. When set to 0, {0} is used.", '`#editor.fontSize#`'),
type: 'number',
default: 0,
tags: ['notebookLayout']
tags: ['notebookLayout', 'notebookOutputLayout']
},
[NotebookSetting.outputFontFamily]: {
markdownDescription: nls.localize('notebook.outputFontFamily', "The font family of the output text within notebook cells. When set to empty, the {0} is used.", '`#editor.fontFamily#`'),
type: 'string',
tags: ['notebookLayout']
tags: ['notebookLayout', 'notebookOutputLayout']
},
[NotebookSetting.outputScrolling]: {
markdownDescription: nls.localize('notebook.outputScrolling', "Use a scrollable region for notebook output when longer than the limit"),
type: 'boolean',
tags: ['notebookLayout'],
tags: ['notebookLayout', 'notebookOutputLayout'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe output font size and font family should have this tag too.

default: typeof product.quality === 'string' && product.quality !== 'stable' // only enable as default in insiders
},
[NotebookSetting.outputWordWrap]: {
markdownDescription: nls.localize('notebook.outputWordWrap', "Controls whether the lines in output should wrap."),
type: 'boolean',
tags: ['notebookLayout'],
tags: ['notebookLayout', 'notebookOutputLayout'],
default: false
},
[NotebookSetting.formatOnSave]: {
Expand Down
Expand Up @@ -53,6 +53,8 @@ import { ITextEditorOptions } from 'vs/platform/editor/common/editor';
import { EditorInput } from 'vs/workbench/common/editor/editorInput';
import { INotebookLoggingService } from 'vs/workbench/contrib/notebook/common/notebookLoggingService';
import { IDisposable } from 'vs/base/common/lifecycle';
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { WorkbenchActionExecutedClassification, WorkbenchActionExecutedEvent } from 'vs/base/common/actions';

const LINE_COLUMN_REGEX = /:([\d]+)(?::([\d]+))?$/;
const LineQueryRegex = /line=(\d+)$/;
Expand Down Expand Up @@ -170,6 +172,7 @@ export class BackLayerWebView<T extends ICommonCellInfo> extends Themable {
@IPathService private readonly pathService: IPathService,
@INotebookLoggingService private readonly notebookLogService: INotebookLoggingService,
@IThemeService themeService: IThemeService,
@ITelemetryService private readonly telemetryService: ITelemetryService
) {
super(themeService);

Expand Down Expand Up @@ -709,10 +712,17 @@ export class BackLayerWebView<T extends ICommonCellInfo> extends Themable {
const cell = this.reversedInsetMapping.get(outputId);

if (cell) {
cell.resetRenderer();
this.telemetryService.publicLog2<WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification>
('workbenchActionExecuted', { id: 'notebook.cell.toggleOutputScrolling', from: 'inlineLink' });

cell.cellViewModel.outputsViewModels.forEach((vm) => {
if (vm.model.metadata) {
vm.model.metadata['scrollable'] = true;
vm.resetRenderer();
}
});
}

console.log(outputId);
return;
}

Expand Down
Expand Up @@ -109,6 +109,7 @@ export class SettingsEditor2 extends EditorPane {
private static SUGGESTIONS: string[] = [
`@${MODIFIED_SETTING_TAG}`,
'@tag:notebookLayout',
'@tag:notebookOutputLayout',
`@tag:${REQUIRE_TRUSTED_WORKSPACE_SETTING_TAG}`,
`@tag:${WORKSPACE_TRUST_SETTING_TAG}`,
'@tag:sync',
Expand Down