Skip to content

Commit

Permalink
Remove inputCollapsed and outputCollapsed metadata, make them view pr…
Browse files Browse the repository at this point in the history
…operties

Fix #125274
  • Loading branch information
roblourens committed Nov 4, 2021
1 parent 2f1e5cb commit a14ebdf
Show file tree
Hide file tree
Showing 19 changed files with 164 additions and 182 deletions.
2 changes: 0 additions & 2 deletions extensions/ipynb/src/ipynbMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ export function activate(context: vscode.ExtensionContext) {
transientOutputs: false,
transientCellMetadata: {
breakpointMargin: true,
inputCollapsed: true,
outputCollapsed: true,
custom: false
}
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,19 @@

import { KeyChord, KeyCode, KeyMod } from 'vs/base/common/keyCodes';
import { Mimes } from 'vs/base/common/mime';
import { IBulkEditService, ResourceTextEdit } from 'vs/editor/browser/services/bulkEditService';
import { localize } from 'vs/nls';
import { MenuId, registerAction2 } from 'vs/platform/actions/common/actions';
import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey';
import { InputFocusedContext, InputFocusedContextKey } from 'vs/platform/contextkey/common/contextkeys';
import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry';
import { ResourceNotebookCellEdit } from 'vs/workbench/contrib/bulkEdit/browser/bulkCellEdits';
import { changeCellToKind, computeCellLinesContents, copyCellRange, joinCellsWithSurrounds, 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, 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 } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { CellFocusMode, EXPAND_CELL_INPUT_COMMAND_ID, EXPAND_CELL_OUTPUT_COMMAND_ID, ICellViewModel, 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 } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import * as icons from 'vs/workbench/contrib/notebook/browser/notebookIcons';
import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
import { CellEditType, CellKind, ICellEditOperation, NotebookCellMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { IBulkEditService, ResourceTextEdit } from 'vs/editor/browser/services/bulkEditService';
import { changeCellToKind, computeCellLinesContents, copyCellRange, joinCellsWithSurrounds, moveCellRange } from 'vs/workbench/contrib/notebook/browser/controller/cellOperations';
import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel';
import { ResourceNotebookCellEdit } from 'vs/workbench/contrib/bulkEdit/browser/bulkCellEdits';
import { CellEditType, CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon';

//#region Move/Copy cells
const MOVE_CELL_UP_COMMAND_ID = 'notebook.cell.moveUp';
Expand Down Expand Up @@ -322,30 +321,7 @@ const COLLAPSE_CELL_INPUT_COMMAND_ID = 'notebook.cell.collapseCellInput';
const COLLAPSE_CELL_OUTPUT_COMMAND_ID = 'notebook.cell.collapseCellOutput';
const TOGGLE_CELL_OUTPUTS_COMMAND_ID = 'notebook.cell.toggleOutputs';

abstract class ChangeNotebookCellMetadataAction extends NotebookCellAction {
async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext): Promise<void> {
const textModel = context.notebookEditor.textModel;
if (!textModel) {
return;
}

const metadataDelta = this.getMetadataDelta();
const edits: ICellEditOperation[] = [];
const targetCells = (context.cell ? [context.cell] : context.selectedCells) ?? [];
for (const cell of targetCells) {
const index = textModel.cells.indexOf(cell.model);
if (index >= 0) {
edits.push({ editType: CellEditType.Metadata, index, metadata: { ...context.cell.metadata, ...metadataDelta } });
}
}

textModel.applyEdits(edits, true, undefined, () => undefined, undefined);
}

abstract getMetadataDelta(): NotebookCellMetadata;
}

registerAction2(class CollapseCellInputAction extends ChangeNotebookCellMetadataAction {
registerAction2(class CollapseCellInputAction extends NotebookMultiCellAction {
constructor() {
super({
id: COLLAPSE_CELL_INPUT_COMMAND_ID,
Expand All @@ -364,12 +340,16 @@ registerAction2(class CollapseCellInputAction extends ChangeNotebookCellMetadata
});
}

getMetadataDelta(): NotebookCellMetadata {
return { inputCollapsed: true };
async runWithContext(accessor: ServicesAccessor, context: INotebookCommandContext | INotebookCellToolbarActionContext): Promise<void> {
if (context.ui) {
context.cell.isInputCollapsed = true;
} else {
context.selectedCells.forEach(cell => cell.isInputCollapsed = true);
}
}
});

registerAction2(class ExpandCellInputAction extends ChangeNotebookCellMetadataAction {
registerAction2(class ExpandCellInputAction extends NotebookMultiCellAction {
constructor() {
super({
id: EXPAND_CELL_INPUT_COMMAND_ID,
Expand All @@ -388,12 +368,16 @@ registerAction2(class ExpandCellInputAction extends ChangeNotebookCellMetadataAc
});
}

getMetadataDelta(): NotebookCellMetadata {
return { inputCollapsed: false };
async runWithContext(accessor: ServicesAccessor, context: INotebookCommandContext | INotebookCellToolbarActionContext): Promise<void> {
if (context.ui) {
context.cell.isInputCollapsed = false;
} else {
context.selectedCells.forEach(cell => cell.isInputCollapsed = false);
}
}
});

registerAction2(class CollapseCellOutputAction extends ChangeNotebookCellMetadataAction {
registerAction2(class CollapseCellOutputAction extends NotebookMultiCellAction {
constructor() {
super({
id: COLLAPSE_CELL_OUTPUT_COMMAND_ID,
Expand All @@ -412,12 +396,16 @@ registerAction2(class CollapseCellOutputAction extends ChangeNotebookCellMetadat
});
}

getMetadataDelta(): NotebookCellMetadata {
return { outputCollapsed: true };
async runWithContext(accessor: ServicesAccessor, context: INotebookCommandContext | INotebookCellToolbarActionContext): Promise<void> {
if (context.ui) {
context.cell.isOutputCollapsed = true;
} else {
context.selectedCells.forEach(cell => cell.isOutputCollapsed = true);
}
}
});

registerAction2(class ExpandCellOuputAction extends ChangeNotebookCellMetadataAction {
registerAction2(class ExpandCellOuputAction extends NotebookMultiCellAction {
constructor() {
super({
id: EXPAND_CELL_OUTPUT_COMMAND_ID,
Expand All @@ -436,8 +424,12 @@ registerAction2(class ExpandCellOuputAction extends ChangeNotebookCellMetadataAc
});
}

getMetadataDelta(): NotebookCellMetadata {
return { outputCollapsed: false };
async runWithContext(accessor: ServicesAccessor, context: INotebookCommandContext | INotebookCellToolbarActionContext): Promise<void> {
if (context.ui) {
context.cell.isOutputCollapsed = false;
} else {
context.selectedCells.forEach(cell => cell.isOutputCollapsed = false);
}
}
});

Expand All @@ -459,25 +451,16 @@ registerAction2(class extends NotebookMultiCellAction {
}

async runWithContext(accessor: ServicesAccessor, context: INotebookCommandContext | INotebookCellToolbarActionContext): Promise<void> {
const textModel = context.notebookEditor.textModel;
let cells: NotebookCellTextModel[] = [];
let cells: readonly ICellViewModel[] = [];
if (context.ui) {
cells = [context.cell.model];
cells = [context.cell];
} else if (context.selectedCells) {
cells = context.selectedCells.map(cell => cell.model);
} else {
cells = [...textModel.cells];
cells = context.selectedCells;
}

const edits: ICellEditOperation[] = [];
for (const cell of cells) {
const index = textModel.cells.indexOf(cell);
if (index >= 0) {
edits.push({ editType: CellEditType.Metadata, index, metadata: { ...cell.metadata, outputCollapsed: !cell.metadata.outputCollapsed } });
}
for (let cell of cells) {
cell.isOutputCollapsed = !cell.isOutputCollapsed;
}

textModel.applyEdits(edits, true, undefined, () => undefined, undefined);
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class NotebookViewportContribution extends Disposable implements INotebookEditor
for (let i = 0; i < this._notebookEditor.getLength(); i++) {
const cell = this._notebookEditor.cellAt(i);

if (cell?.cellKind === CellKind.Markup && cell?.getEditState() === CellEditState.Preview && !cell.metadata.inputCollapsed) {
if (cell?.cellKind === CellKind.Markup && cell?.getEditState() === CellEditState.Preview && !cell.isInputCollapsed) {
// TODO@rebornix currently we disable markdown cell rendering in webview for accessibility
// this._notebookEditor.createMarkupPreview(cell);
} else if (cell?.cellKind === CellKind.Code) {
Expand All @@ -75,7 +75,7 @@ class NotebookViewportContribution extends Disposable implements INotebookEditor
cellRangesToIndexes(visibleRanges).forEach(index => {
const cell = this._notebookEditor.cellAt(index);

if (cell?.cellKind === CellKind.Markup && cell?.getEditState() === CellEditState.Preview && !cell.metadata.inputCollapsed) {
if (cell?.cellKind === CellKind.Markup && cell?.getEditState() === CellEditState.Preview && !cell.isInputCollapsed) {
(this._notebookEditor as INotebookEditorDelegate).createMarkupPreview(cell);
} else if (cell?.cellKind === CellKind.Code) {
this._renderCell((cell as CodeCellViewModel));
Expand All @@ -84,7 +84,7 @@ class NotebookViewportContribution extends Disposable implements INotebookEditor
}

private _renderCell(viewCell: CodeCellViewModel) {
if (viewCell.metadata.outputCollapsed) {
if (viewCell.isOutputCollapsed) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,14 +461,6 @@ class RegisterSchemasContribution extends Disposable implements IWorkbenchContri
['language']: {
type: 'string',
description: 'The language for the cell'
},
['inputCollapsed']: {
type: 'boolean',
description: `Whether a code cell's editor is collapsed`
},
['outputCollapsed']: {
type: 'boolean',
description: `Whether a code cell's outputs are collapsed`
}
},
// patternProperties: allSettings.patternProperties,
Expand Down
4 changes: 4 additions & 0 deletions src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,8 @@ export interface ICellViewModel extends IGenericCellViewModel {
readonly onDidChangeCellStatusBarItems: Event<void>;
readonly editStateSource: string;
readonly editorAttached: boolean;
isInputCollapsed: boolean;
isOutputCollapsed: boolean;
dragging: boolean;
handle: number;
uri: URI;
Expand Down Expand Up @@ -723,6 +725,8 @@ export interface CellViewModelStateChangeEvent {
readonly outputIsFocusedChanged?: boolean;
readonly cellIsHoveredChanged?: boolean;
readonly cellLineNumberChanged?: boolean;
readonly inputCollapsedChanged?: boolean;
readonly outputCollapsedChanged?: boolean;
}

export function getVisibleCells(cells: CellViewModel[], hiddenRanges: ICellRange[]) {
Expand Down
22 changes: 12 additions & 10 deletions src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1423,17 +1423,17 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
store.add((cell as CodeCellViewModel).onDidRemoveOutputs((outputs) => {
outputs.forEach(output => this.removeInset(output));
}));

store.add((cell as CodeCellViewModel).onDidHideOutputs((outputs) => {
outputs.forEach(output => this.hideInset(output));
}));
}

if (cell.cellKind === CellKind.Markup) {
store.add((cell as MarkupCellViewModel).onDidHideInput(() => {
store.add((cell as CellViewModel).onDidChangeState(e => {
if (e.inputCollapsedChanged && cell.isInputCollapsed && cell.cellKind === CellKind.Markup) {
this.hideMarkupPreviews([(cell as MarkupCellViewModel)]);
}));
}
}

if (e.outputCollapsedChanged && cell.isOutputCollapsed && cell.cellKind === CellKind.Code) {
cell.outputsViewModels.forEach(output => this.hideInset(output));
}
}));

return store;
}
Expand Down Expand Up @@ -1576,7 +1576,9 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
if (!state) {
return {
editingCells: {},
editorViewStates: {}
editorViewStates: {},
collapsedInputCells: {},
collapsedOutputCells: {},
};
}

Expand Down Expand Up @@ -2319,7 +2321,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
output: output.source,
cellTop,
outputOffset,
forceDisplay: !cell.metadata.outputCollapsed,
forceDisplay: !cell.isOutputCollapsed,
}], []);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
this._localDisposableStore.add(this.view.onMouseDblClick(() => {
const focus = this.getFocusedElements()[0];

if (focus && focus.cellKind === CellKind.Markup && !focus.metadata.inputCollapsed && !this._viewModel?.options.isReadOnly) {
if (focus && focus.cellKind === CellKind.Markup && !focus.isInputCollapsed && !this._viewModel?.options.isReadOnly) {
// scroll the cell into view if out of viewport
this.revealElementInView(focus);
focus.updateEditState(CellEditState.Editing, 'dbclick');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
import { IWorkspaceTrustManagementService } from 'vs/platform/workspace/common/workspaceTrust';
import { asWebviewUri, webviewGenericCspSource } from 'vs/workbench/api/common/shared/webview';
import { CellEditState, ICellOutputViewModel, ICommonCellInfo, IDisplayOutputLayoutUpdateRequest, IDisplayOutputViewModel, IFocusNotebookCellOptions, IGenericCellViewModel, IInsetRenderOutput, INotebookEditorCreationOptions, RenderOutputType } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { CellEditState, ICellOutputViewModel, ICellViewModel, ICommonCellInfo, IDisplayOutputLayoutUpdateRequest, IDisplayOutputViewModel, IFocusNotebookCellOptions, IGenericCellViewModel, IInsetRenderOutput, INotebookEditorCreationOptions, RenderOutputType } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { preloadsScriptStr, RendererMetadata } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads';
import { transformWebviewThemeVars } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewThemeMapping';
import { MarkupCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel';
Expand Down Expand Up @@ -836,7 +836,7 @@ var requirejs = (function() {
return false;
}

if (cell.metadata.outputCollapsed) {
if ('isOutputCollapsed' in cell && (cell as ICellViewModel).isOutputCollapsed) {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ export class CellContextKeyManager extends Disposable {
this.elementDisposables.add(element.onDidChangeOutputs(() => this.updateForOutputs()));
}

this.elementDisposables.add(element.model.onDidChangeMetadata(() => this.updateForCollapseState()));
this.elementDisposables.add(this.notebookEditor.onDidChangeActiveCell(() => this.updateForFocusState()));

this.element = element;
Expand Down Expand Up @@ -98,9 +97,9 @@ export class CellContextKeyManager extends Disposable {
this.cellLineNumbers.set(this.element.lineNumbers);
}

// if (e.collapseStateChanged) {
// this.updateForCollapseState();
// }
if (e.inputCollapsedChanged || e.outputCollapsedChanged) {
this.updateForCollapseState();
}
});
}

Expand Down Expand Up @@ -151,8 +150,8 @@ export class CellContextKeyManager extends Disposable {
}

private updateForCollapseState() {
this.cellContentCollapsed.set(!!this.element.metadata.inputCollapsed);
this.cellOutputCollapsed.set(!!this.element.metadata.outputCollapsed);
this.cellContentCollapsed.set(!!this.element.isInputCollapsed);
this.cellOutputCollapsed.set(!!this.element.isOutputCollapsed);
}

private updateForOutputs() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ export class CellOutputElement extends Disposable {
render(previousSibling: HTMLElement | undefined, forceBreakStreaming: boolean = false): IRenderResult | undefined {
const index = this.viewCell.outputsViewModels.indexOf(this.output);

if (this.viewCell.metadata.outputCollapsed || !this.notebookEditor.hasModel()) {
if (this.viewCell.isOutputCollapsed || !this.notebookEditor.hasModel()) {
return undefined;
}

Expand Down

0 comments on commit a14ebdf

Please sign in to comment.