Skip to content

Commit 2e440e1

Browse files
committed
introduce explicit isReadonly in notebook options
1 parent 831e530 commit 2e440e1

File tree

13 files changed

+87
-81
lines changed

13 files changed

+87
-81
lines changed

src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ export class InteractiveEditor extends EditorPane {
148148
this.#notebookExecutionStateService = notebookExecutionStateService;
149149
this.#extensionService = extensionService;
150150

151-
this.#notebookOptions = new NotebookOptions(configurationService, notebookExecutionStateService, { cellToolbarInteraction: 'hover', globalToolbar: true, dragAndDropEnabled: false, insertToolbarBetweenCells: false });
151+
this.#notebookOptions = new NotebookOptions(configurationService, notebookExecutionStateService, true, { cellToolbarInteraction: 'hover', globalToolbar: true, dragAndDropEnabled: false });
152152
this.#editorMemento = this.getEditorMemento<InteractiveEditorViewState>(editorGroupService, textResourceConfigurationService, INTERACTIVE_EDITOR_VIEW_STATE_PREFERENCE_KEY);
153153

154154
codeEditorService.registerDecorationType('interactive-decoration', DECORATION_KEY, {});

src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD
151151
@INotebookExecutionStateService notebookExecutionStateService: INotebookExecutionStateService,
152152
) {
153153
super(NotebookTextDiffEditor.ID, telemetryService, themeService, storageService);
154-
this._notebookOptions = new NotebookOptions(this.configurationService, notebookExecutionStateService);
154+
this._notebookOptions = new NotebookOptions(this.configurationService, notebookExecutionStateService, false);
155155
this._register(this._notebookOptions);
156156
const editorOptions = this.configurationService.getValue<ICodeEditorOptions>('editor');
157157
this._fontInfo = FontMeasurements.readFontInfo(BareFontInfo.createFromRawSettings(editorOptions, PixelRatio.value));

src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import { IWebviewElement } from 'vs/workbench/contrib/webview/browser/webview';
3030
import { IEditorOptions } from 'vs/editor/common/config/editorOptions';
3131
import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
3232
import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
33-
import { ViewContext } from 'vs/workbench/contrib/notebook/browser/viewModel/viewContext';
3433

3534
//#region Shared commands
3635
export const EXPAND_CELL_INPUT_COMMAND_ID = 'notebook.cell.expandCellInput';
@@ -709,8 +708,6 @@ export interface IBaseCellEditorOptions extends IDisposable {
709708
export interface INotebookEditorDelegate extends INotebookEditor {
710709
hasModel(): this is IActiveNotebookEditorDelegate;
711710

712-
readonly viewContext: ViewContext;
713-
714711
readonly creationOptions: INotebookEditorCreationOptions;
715712
readonly onDidChangeOptions: Event<void>;
716713
readonly onDidChangeDecorations: Event<void>;

src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -266,10 +266,6 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
266266
return this._notebookOptions;
267267
}
268268

269-
get viewContext() {
270-
return this._viewContext;
271-
}
272-
273269
constructor(
274270
readonly creationOptions: INotebookEditorCreationOptions,
275271
dimension: DOM.Dimension | undefined,
@@ -296,13 +292,12 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
296292
this.isEmbedded = creationOptions.isEmbedded ?? false;
297293
this._readOnly = creationOptions.isReadOnly ?? false;
298294

299-
this._notebookOptions = creationOptions.options ?? new NotebookOptions(this.configurationService, notebookExecutionStateService);
295+
this._notebookOptions = creationOptions.options ?? new NotebookOptions(this.configurationService, notebookExecutionStateService, this._readOnly);
300296
this._register(this._notebookOptions);
301297
this._viewContext = new ViewContext(
302298
this._notebookOptions,
303299
new NotebookEventDispatcher(),
304-
language => this.getBaseCellEditorOptions(language),
305-
() => this._readOnly);
300+
language => this.getBaseCellEditorOptions(language));
306301
this._register(this._viewContext.eventDispatcher.onDidChangeCellState(e => {
307302
this._onDidChangeCellState.fire(e);
308303
}));
@@ -603,15 +598,15 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
603598
collapsedIndicatorHeight,
604599
compactView,
605600
focusIndicator,
606-
insertToolbarBetweenCells,
601+
insertToolbarPosition,
607602
insertToolbarAlignment,
608603
fontSize,
609604
outputFontSize,
610605
focusIndicatorLeftMargin,
611606
focusIndicatorGap
612607
} = this._notebookOptions.getLayoutConfiguration();
613608

614-
const { bottomToolbarGap, bottomToolbarHeight } = this._notebookOptions.computeBottomToolbarDimensions(this.viewModel?.viewType, this.viewModel?.options.isReadOnly);
609+
const { bottomToolbarGap, bottomToolbarHeight } = this._notebookOptions.computeBottomToolbarDimensions(this.viewModel?.viewType);
615610

616611
const styleSheets: string[] = [];
617612
if (!this._fontInfo) {
@@ -721,7 +716,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
721716
}
722717

723718
// between cell insert toolbar
724-
if (insertToolbarBetweenCells) {
719+
if (insertToolbarPosition === 'betweenCells' || insertToolbarPosition === 'both') {
725720
styleSheets.push(`.monaco-workbench .notebookOverlay > .cell-list-container > .monaco-list > .monaco-scrollable-element > .monaco-list-rows > .monaco-list-row .cell-bottom-toolbar-container { display: flex; }`);
726721
styleSheets.push(`.monaco-workbench .notebookOverlay > .cell-list-container > .monaco-list > .monaco-scrollable-element > .monaco-list-rows > .cell-list-top-cell-toolbar-container { display: flex; }`);
727722
} else {
@@ -760,7 +755,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
760755
}
761756

762757
// top insert toolbar
763-
const topInsertToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType, this.viewModel?.options.isReadOnly);
758+
const topInsertToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType);
764759
styleSheets.push(`.notebookOverlay .cell-list-top-cell-toolbar-container { top: -${topInsertToolbarHeight - 3}px }`);
765760
styleSheets.push(`.notebookOverlay > .cell-list-container > .monaco-list > .monaco-scrollable-element,
766761
.notebookOverlay > .cell-list-container > .notebook-gutter > .monaco-list > .monaco-scrollable-element {
@@ -1053,12 +1048,12 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
10531048

10541049
async setModel(textModel: NotebookTextModel, viewState: INotebookEditorViewState | undefined, perf?: NotebookPerfMarks): Promise<void> {
10551050
if (this.viewModel === undefined || !this.viewModel.equal(textModel)) {
1056-
const oldTopInsertToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType, this.viewModel?.options.isReadOnly);
1057-
const oldBottomToolbarDimensions = this._notebookOptions.computeBottomToolbarDimensions(this.viewModel?.viewType, this.viewModel?.options.isReadOnly);
1051+
const oldTopInsertToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType);
1052+
const oldBottomToolbarDimensions = this._notebookOptions.computeBottomToolbarDimensions(this.viewModel?.viewType);
10581053
this._detachModel();
10591054
await this._attachModel(textModel, viewState, perf);
1060-
const newTopInsertToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType, this.viewModel?.options.isReadOnly);
1061-
const newBottomToolbarDimensions = this._notebookOptions.computeBottomToolbarDimensions(this.viewModel?.viewType, this.viewModel?.options.isReadOnly);
1055+
const newTopInsertToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType);
1056+
const newBottomToolbarDimensions = this._notebookOptions.computeBottomToolbarDimensions(this.viewModel?.viewType);
10621057

10631058
if (oldTopInsertToolbarHeight !== newTopInsertToolbarHeight
10641059
|| oldBottomToolbarDimensions.bottomToolbarGap !== newBottomToolbarDimensions.bottomToolbarGap
@@ -1181,6 +1176,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
11811176
}
11821177

11831178
this.viewModel.updateOptions({ isReadOnly: this._readOnly });
1179+
this.notebookOptions.updateOptions(this._readOnly);
11841180

11851181
// reveal cell if editor options tell to do so
11861182
const cellOptions = options?.cellOptions ?? this._parseIndexedCellOptions(options);
@@ -1369,6 +1365,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
13691365

13701366
this.viewModel = this.instantiationService.createInstance(NotebookViewModel, textModel.viewType, textModel, this._viewContext, this.getLayoutInfo(), { isReadOnly: this._readOnly });
13711367
this._viewContext.eventDispatcher.emit([new NotebookLayoutChangedEvent({ width: true, fontInfo: true }, this.getLayoutInfo())]);
1368+
this.notebookOptions.updateOptions(this._readOnly);
13721369

13731370
this._updateForOptions();
13741371
this._updateForNotebookConfiguration();
@@ -1472,7 +1469,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
14721469
}));
14731470

14741471
if (this._dimension) {
1475-
const topInserToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType, this.viewModel?.options.isReadOnly);
1472+
const topInserToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType);
14761473
this._list.layout(this._dimension.height - topInserToolbarHeight, this._dimension.width);
14771474
} else {
14781475
this._list.layout();
@@ -1766,7 +1763,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
17661763
const newBodyHeight = Math.max(dimension.height - (this._notebookTopToolbar?.useGlobalToolbar ? /** Toolbar height */ 26 : 0), 0);
17671764
DOM.size(this._body, dimension.width, newBodyHeight);
17681765

1769-
const topInserToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType, this.viewModel?.options.isReadOnly);
1766+
const topInserToolbarHeight = this._notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType);
17701767
const newCellListHeight = Math.max(newBodyHeight - topInserToolbarHeight, 0);
17711768
if (this._list.getRenderHeight() < newCellListHeight) {
17721769
// the new dimension is larger than the list viewport, update its additional height first, otherwise the list view will move down a bit (as the `scrollBottom` will move down)
@@ -2939,7 +2936,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
29392936
private _updateMarkupCellHeight(cellId: string, height: number, isInit: boolean) {
29402937
const cell = this._getCellById(cellId);
29412938
if (cell && cell instanceof MarkupCellViewModel) {
2942-
const { bottomToolbarGap } = this._notebookOptions.computeBottomToolbarDimensions(this.viewModel?.viewType, this.viewModel?.options.isReadOnly);
2939+
const { bottomToolbarGap } = this._notebookOptions.computeBottomToolbarDimensions(this.viewModel?.viewType);
29432940
this._debug('updateMarkdownCellHeight', cell.handle, height + bottomToolbarGap, isInit);
29442941
cell.renderedMarkdownHeight = height;
29452942
}

src/vs/workbench/contrib/notebook/browser/notebookOptions.ts

Lines changed: 51 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export interface NotebookLayoutConfiguration {
5858
cellToolbarInteraction: string;
5959
compactView: boolean;
6060
focusIndicator: 'border' | 'gutter';
61-
insertToolbarBetweenCells: boolean;
61+
insertToolbarPosition: 'betweenCells' | 'notebookToolbar' | 'both' | 'hidden';
6262
insertToolbarAlignment: 'left' | 'center';
6363
globalToolbar: boolean;
6464
consolidatedOutputButton: boolean;
@@ -133,7 +133,8 @@ export class NotebookOptions extends Disposable {
133133
constructor(
134134
private readonly configurationService: IConfigurationService,
135135
private readonly notebookExecutionStateService: INotebookExecutionStateService,
136-
private readonly overrides?: { cellToolbarInteraction: string; globalToolbar: boolean; dragAndDropEnabled: boolean; insertToolbarBetweenCells?: boolean }
136+
private isReadonly: boolean,
137+
private readonly overrides?: { cellToolbarInteraction: string; globalToolbar: boolean; dragAndDropEnabled: boolean }
137138
) {
138139
super();
139140
const showCellStatusBar = this.configurationService.getValue<ShowCellStatusBarType>(NotebookSetting.showCellStatusBar);
@@ -145,7 +146,7 @@ export class NotebookOptions extends Disposable {
145146
const cellToolbarInteraction = overrides?.cellToolbarInteraction ?? this.configurationService.getValue<string>(NotebookSetting.cellToolbarVisibility);
146147
const compactView = this.configurationService.getValue<boolean | undefined>(NotebookSetting.compactView) ?? true;
147148
const focusIndicator = this._computeFocusIndicatorOption();
148-
const insertToolbarBetweenCells = overrides?.insertToolbarBetweenCells ?? this._computeInsertToolbarBetweenCellsOption();
149+
const insertToolbarPosition = this._computeInsertToolbarPositionOption(this.isReadonly);
149150
const insertToolbarAlignment = this._computeInsertToolbarAlignmentOption();
150151
const showFoldingControls = this._computeShowFoldingControlsOption();
151152
// const { bottomToolbarGap, bottomToolbarHeight } = this._computeBottomToolbarDimensions(compactView, insertToolbarPosition, insertToolbarAlignment);
@@ -219,7 +220,7 @@ export class NotebookOptions extends Disposable {
219220
cellToolbarInteraction,
220221
compactView,
221222
focusIndicator,
222-
insertToolbarBetweenCells,
223+
insertToolbarPosition,
223224
insertToolbarAlignment,
224225
showFoldingControls,
225226
fontSize,
@@ -248,6 +249,22 @@ export class NotebookOptions extends Disposable {
248249
}));
249250
}
250251

252+
updateOptions(isReadonly: boolean) {
253+
if (this.isReadonly !== isReadonly) {
254+
this.isReadonly = isReadonly;
255+
256+
this._updateConfiguration({
257+
affectsConfiguration(configuration: string): boolean {
258+
return configuration === NotebookSetting.insertToolbarLocation;
259+
},
260+
source: ConfigurationTarget.DEFAULT,
261+
affectedKeys: new Set([NotebookSetting.insertToolbarLocation]),
262+
change: { keys: [NotebookSetting.insertToolbarLocation], overrides: [] },
263+
sourceConfig: undefined
264+
});
265+
}
266+
}
267+
251268
private _migrateDeprecatedSetting(deprecatedKey: string, key: string): void {
252269
const deprecatedSetting = this.configurationService.inspect(deprecatedKey);
253270

@@ -369,7 +386,7 @@ export class NotebookOptions extends Disposable {
369386
configuration.cellToolbarLocation = this.configurationService.getValue<string | { [key: string]: string }>(NotebookSetting.cellToolbarLocation) ?? { 'default': 'right' };
370387
}
371388

372-
if (cellToolbarInteraction && this.overrides?.cellToolbarInteraction === undefined) {
389+
if (cellToolbarInteraction && !this.overrides?.cellToolbarInteraction) {
373390
configuration.cellToolbarInteraction = this.configurationService.getValue<string>(NotebookSetting.cellToolbarVisibility);
374391
}
375392

@@ -389,8 +406,8 @@ export class NotebookOptions extends Disposable {
389406
configuration.insertToolbarAlignment = this._computeInsertToolbarAlignmentOption();
390407
}
391408

392-
if (insertToolbarPosition && this.overrides?.insertToolbarBetweenCells === undefined) {
393-
configuration.insertToolbarBetweenCells = this._computeInsertToolbarBetweenCellsOption();
409+
if (insertToolbarPosition) {
410+
configuration.insertToolbarPosition = this._computeInsertToolbarPositionOption(this.isReadonly);
394411
}
395412

396413
if (globalToolbar && this.overrides?.globalToolbar === undefined) {
@@ -479,9 +496,8 @@ export class NotebookOptions extends Disposable {
479496
});
480497
}
481498

482-
private _computeInsertToolbarBetweenCellsOption() {
483-
const configValue = this.configurationService.getValue<'betweenCells' | 'notebookToolbar' | 'both' | 'hidden'>(NotebookSetting.insertToolbarLocation) ?? 'both';
484-
return configValue === 'betweenCells' || configValue === 'both';
499+
private _computeInsertToolbarPositionOption(isReadOnly: boolean) {
500+
return isReadOnly ? 'hidden' : this.configurationService.getValue<'betweenCells' | 'notebookToolbar' | 'both' | 'hidden'>(NotebookSetting.insertToolbarLocation) ?? 'both';
485501
}
486502

487503
private _computeInsertToolbarAlignmentOption() {
@@ -513,16 +529,16 @@ export class NotebookOptions extends Disposable {
513529
return this._layoutConfiguration;
514530
}
515531

516-
computeCollapsedMarkdownCellHeight(viewType: string, isReadOnly: boolean): number {
517-
const { bottomToolbarGap } = this.computeBottomToolbarDimensions(viewType, isReadOnly);
532+
computeCollapsedMarkdownCellHeight(viewType: string): number {
533+
const { bottomToolbarGap } = this.computeBottomToolbarDimensions(viewType);
518534
return this._layoutConfiguration.markdownCellTopMargin
519535
+ this._layoutConfiguration.collapsedIndicatorHeight
520536
+ bottomToolbarGap
521537
+ this._layoutConfiguration.markdownCellBottomMargin;
522538
}
523539

524-
computeBottomToolbarOffset(totalHeight: number, viewType: string, isReadOnly: boolean) {
525-
const { bottomToolbarGap, bottomToolbarHeight } = this.computeBottomToolbarDimensions(viewType, isReadOnly);
540+
computeBottomToolbarOffset(totalHeight: number, viewType: string) {
541+
const { bottomToolbarGap, bottomToolbarHeight } = this.computeBottomToolbarDimensions(viewType);
526542

527543
return totalHeight
528544
- bottomToolbarGap
@@ -548,37 +564,34 @@ export class NotebookOptions extends Disposable {
548564
return this._layoutConfiguration.cellStatusBarHeight;
549565
}
550566

551-
private _computeBottomToolbarDimensions(compactView: boolean, insertToolbarBetweenCells: boolean, insertToolbarAlignment: 'left' | 'center', cellToolbar: 'right' | 'left' | 'hidden'): { bottomToolbarGap: number; bottomToolbarHeight: number } {
552-
if (!insertToolbarBetweenCells) {
553-
return compactView ? {
554-
bottomToolbarGap: 0,
555-
bottomToolbarHeight: 0
556-
} : {
557-
bottomToolbarGap: 12,
558-
bottomToolbarHeight: 12
559-
};
560-
}
561-
567+
private _computeBottomToolbarDimensions(compactView: boolean, insertToolbarPosition: 'betweenCells' | 'notebookToolbar' | 'both' | 'hidden', insertToolbarAlignment: 'left' | 'center', cellToolbar: 'right' | 'left' | 'hidden'): { bottomToolbarGap: number; bottomToolbarHeight: number } {
562568
if (insertToolbarAlignment === 'left' || cellToolbar !== 'hidden') {
563569
return {
564570
bottomToolbarGap: 18,
565571
bottomToolbarHeight: 18
566572
};
567573
}
568574

569-
return compactView ? {
570-
bottomToolbarGap: 12,
571-
bottomToolbarHeight: 20
572-
} : {
573-
bottomToolbarGap: 20,
574-
bottomToolbarHeight: 20
575-
};
575+
if (insertToolbarPosition === 'betweenCells' || insertToolbarPosition === 'both') {
576+
return compactView ? {
577+
bottomToolbarGap: 12,
578+
bottomToolbarHeight: 20
579+
} : {
580+
bottomToolbarGap: 20,
581+
bottomToolbarHeight: 20
582+
};
583+
} else {
584+
return {
585+
bottomToolbarGap: 0,
586+
bottomToolbarHeight: 0
587+
};
588+
}
576589
}
577590

578-
computeBottomToolbarDimensions(viewType?: string, isReadOnly?: boolean): { bottomToolbarGap: number; bottomToolbarHeight: number } {
591+
computeBottomToolbarDimensions(viewType?: string): { bottomToolbarGap: number; bottomToolbarHeight: number } {
579592
const configuration = this._layoutConfiguration;
580593
const cellToolbarPosition = this.computeCellToolbarLocation(viewType);
581-
const { bottomToolbarGap, bottomToolbarHeight } = this._computeBottomToolbarDimensions(configuration.compactView, configuration.insertToolbarBetweenCells && !isReadOnly, configuration.insertToolbarAlignment, cellToolbarPosition);
594+
const { bottomToolbarGap, bottomToolbarHeight } = this._computeBottomToolbarDimensions(configuration.compactView, configuration.insertToolbarPosition, configuration.insertToolbarAlignment, cellToolbarPosition);
582595
return {
583596
bottomToolbarGap,
584597
bottomToolbarHeight
@@ -619,8 +632,8 @@ export class NotebookOptions extends Disposable {
619632
return 'right';
620633
}
621634

622-
computeTopInsertToolbarHeight(viewType?: string, isReadOnly?: boolean): number {
623-
if (this._layoutConfiguration.insertToolbarBetweenCells && !isReadOnly) {
635+
computeTopInsertToolbarHeight(viewType?: string): number {
636+
if (this._layoutConfiguration.insertToolbarPosition === 'betweenCells' || this._layoutConfiguration.insertToolbarPosition === 'both') {
624637
return SCROLLABLE_ELEMENT_PADDING_TOP;
625638
}
626639

@@ -700,8 +713,8 @@ export class NotebookOptions extends Disposable {
700713
};
701714
}
702715

703-
computeIndicatorPosition(totalHeight: number, foldHintHeight: number, viewType?: string, isReadOnly?: boolean) {
704-
const { bottomToolbarGap } = this.computeBottomToolbarDimensions(viewType, isReadOnly);
716+
computeIndicatorPosition(totalHeight: number, foldHintHeight: number, viewType?: string) {
717+
const { bottomToolbarGap } = this.computeBottomToolbarDimensions(viewType);
705718

706719
return {
707720
bottomIndicatorTop: totalHeight - bottomToolbarGap - this._layoutConfiguration.cellBottomMargin - foldHintHeight,

0 commit comments

Comments
 (0)