Fix shifting in cell editors that have horizontal scrollba (#158232)

* Fix shifting in cell editors that have horizontal scrollbars, for #153708

* use BaseCellEditorOptions to check wordwrap
This commit is contained in:
Rob Lourens 2022-08-22 20:53:01 -05:00 committed by GitHub
parent f9093f6375
commit dcf0c0a141
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 56 additions and 20 deletions

View file

@ -158,6 +158,7 @@ export interface CodeCellLayoutInfo {
readonly fontInfo: FontInfo | null;
readonly editorHeight: number;
readonly editorWidth: number;
readonly estimatedHasHorizontalScrolling: boolean;
readonly statusBarHeight: number;
readonly commentHeight: number;
readonly totalHeight: number;

View file

@ -262,7 +262,10 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
this._notebookOptions = creationOptions.options ?? new NotebookOptions(this.configurationService, notebookExecutionStateService);
this._register(this._notebookOptions);
this._viewContext = new ViewContext(this._notebookOptions, new NotebookEventDispatcher());
this._viewContext = new ViewContext(
this._notebookOptions,
new NotebookEventDispatcher(),
language => this.getBaseCellEditorOptions(language));
this._register(this._viewContext.eventDispatcher.onDidChangeCellState(e => {
this._onDidChangeCellState.fire(e);
}));

View file

@ -188,7 +188,7 @@ export class CodeCell extends Disposable {
if (model && this.templateData.editor) {
this.templateData.editor.setModel(model);
this.viewCell.attachTextEditor(this.templateData.editor);
this.viewCell.attachTextEditor(this.templateData.editor, this.viewCell.layoutInfo.estimatedHasHorizontalScrolling);
const focusEditorIfNeeded = () => {
if (
this.notebookEditor.getActiveCell() === this.viewCell &&

View file

@ -23,7 +23,7 @@ import { CellViewModelStateChangeEvent } from 'vs/workbench/contrib/notebook/bro
import { ViewContext } from 'vs/workbench/contrib/notebook/browser/viewModel/viewContext';
import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel';
import { CellKind, INotebookCellStatusBarItem, INotebookSearchOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { NotebookOptionsChangeEvent } from 'vs/workbench/contrib/notebook/common/notebookOptions';
import { getEditorTopPadding, NotebookOptionsChangeEvent } from 'vs/workbench/contrib/notebook/common/notebookOptions';
export abstract class BaseCellViewModel extends Disposable {
@ -219,7 +219,7 @@ export abstract class BaseCellViewModel extends Disposable {
// }
// }
attachTextEditor(editor: ICodeEditor) {
attachTextEditor(editor: ICodeEditor, estimatedHasHorizontalScrolling?: boolean) {
if (!editor.hasModel()) {
throw new Error('Invalid editor: model is missing');
}
@ -237,6 +237,20 @@ export abstract class BaseCellViewModel extends Disposable {
if (this._editorViewStates) {
this._restoreViewState(this._editorViewStates);
} else {
// If no real editor view state was persisted, restore a default state.
// This forces the editor to measure its content width immediately.
if (estimatedHasHorizontalScrolling) {
this._restoreViewState({
contributionsState: {},
cursorState: [],
viewState: {
scrollLeft: 0,
firstPosition: { lineNumber: 1, column: 1 },
firstPositionDeltaTop: getEditorTopPadding()
}
});
}
}
if (this._editorTransientState) {

View file

@ -161,6 +161,7 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod
outputIndicatorHeight: 0,
bottomToolbarOffset: 0,
layoutState: CellLayoutState.Uninitialized,
estimatedHasHorizontalScrolling: false
};
}
@ -192,9 +193,12 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod
let newState: CellLayoutState;
let editorHeight: number;
let totalHeight: number;
let hasHorizontalScrolling = false;
if (!state.editorHeight && this._layoutInfo.layoutState === CellLayoutState.FromCache && !state.outputHeight) {
// No new editorHeight info - keep cached totalHeight and estimate editorHeight
editorHeight = this.estimateEditorHeight(state.font?.lineHeight ?? this._layoutInfo.fontInfo?.lineHeight);
const estimate = this.estimateEditorHeight(state.font?.lineHeight ?? this._layoutInfo.fontInfo?.lineHeight);
editorHeight = estimate.editorHeight;
hasHorizontalScrolling = estimate.hasHorizontalScrolling;
totalHeight = this._layoutInfo.totalHeight;
newState = CellLayoutState.FromCache;
} else if (state.editorHeight || this._layoutInfo.layoutState === CellLayoutState.Measured) {
@ -202,8 +206,11 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod
editorHeight = this._editorHeight;
totalHeight = this.computeTotalHeight(this._editorHeight, outputTotalHeight, outputShowMoreContainerHeight);
newState = CellLayoutState.Measured;
hasHorizontalScrolling = this._layoutInfo.estimatedHasHorizontalScrolling;
} else {
editorHeight = this.estimateEditorHeight(state.font?.lineHeight ?? this._layoutInfo.fontInfo?.lineHeight);
const estimate = this.estimateEditorHeight(state.font?.lineHeight ?? this._layoutInfo.fontInfo?.lineHeight);
editorHeight = estimate.editorHeight;
hasHorizontalScrolling = estimate.hasHorizontalScrolling;
totalHeight = this.computeTotalHeight(editorHeight, outputTotalHeight, outputShowMoreContainerHeight);
newState = CellLayoutState.Estimated;
}
@ -239,6 +246,7 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod
outputIndicatorHeight,
bottomToolbarOffset,
layoutState: newState,
estimatedHasHorizontalScrolling: hasHorizontalScrolling
};
} else {
const codeIndicatorHeight = notebookLayoutConfiguration.collapsedIndicatorHeight;
@ -276,6 +284,7 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod
outputIndicatorHeight,
bottomToolbarOffset,
layoutState: this._layoutInfo.layoutState,
estimatedHasHorizontalScrolling: false
};
}
@ -306,7 +315,8 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod
codeIndicatorHeight: this._layoutInfo.codeIndicatorHeight,
outputIndicatorHeight: this._layoutInfo.outputIndicatorHeight,
bottomToolbarOffset: this._layoutInfo.bottomToolbarOffset,
layoutState: CellLayoutState.FromCache
layoutState: CellLayoutState.FromCache,
estimatedHasHorizontalScrolling: this._layoutInfo.estimatedHasHorizontalScrolling
};
}
}
@ -323,32 +333,37 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod
getHeight(lineHeight: number) {
if (this._layoutInfo.layoutState === CellLayoutState.Uninitialized) {
const editorHeight = this.estimateEditorHeight(lineHeight);
return this.computeTotalHeight(editorHeight, 0, 0);
const estimate = this.estimateEditorHeight(lineHeight);
return this.computeTotalHeight(estimate.editorHeight, 0, 0);
} else {
return this._layoutInfo.totalHeight;
}
}
private estimateEditorHeight(lineHeight: number | undefined = 20): number {
let hasScrolling = false;
if (this.layoutInfo.fontInfo) {
private estimateEditorHeight(lineHeight: number | undefined = 20): { editorHeight: number; hasHorizontalScrolling: boolean } {
let hasHorizontalScrolling = false;
const cellEditorOptions = this.viewContext.getBaseCellEditorOptions(this.language);
if (this.layoutInfo.fontInfo && cellEditorOptions.value.wordWrap === 'off') {
for (let i = 0; i < this.lineCount; i++) {
const max = this.textBuffer.getLineLastNonWhitespaceColumn(i + 1);
const estimatedWidth = max * (this.layoutInfo.fontInfo.typicalHalfwidthCharacterWidth + this.layoutInfo.fontInfo.letterSpacing);
if (estimatedWidth > this.layoutInfo.editorWidth) {
hasScrolling = true;
hasHorizontalScrolling = true;
break;
}
}
}
const verticalScrollbarHeight = hasScrolling ? 12 : 0; // take zoom level into account
const verticalScrollbarHeight = hasHorizontalScrolling ? 12 : 0; // take zoom level into account
const editorPadding = this.viewContext.notebookOptions.computeEditorPadding(this.internalMetadata, this.uri);
return this.lineCount * lineHeight
const editorHeight = this.lineCount * lineHeight
+ editorPadding.top
+ editorPadding.bottom // EDITOR_BOTTOM_PADDING
+ verticalScrollbarHeight;
return {
editorHeight,
hasHorizontalScrolling
};
}
private computeTotalHeight(editorHeight: number, outputsTotalHeight: number, outputShowMoreContainerHeight: number): number {

View file

@ -3,13 +3,15 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { IBaseCellEditorOptions } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { NotebookEventDispatcher } from 'vs/workbench/contrib/notebook/browser/viewModel/eventDispatcher';
import { NotebookOptions } from 'vs/workbench/contrib/notebook/common/notebookOptions';
export class ViewContext {
constructor(
readonly notebookOptions: NotebookOptions,
readonly eventDispatcher: NotebookEventDispatcher
readonly eventDispatcher: NotebookEventDispatcher,
readonly getBaseCellEditorOptions: (language: string) => IBaseCellEditorOptions
) {
}
}

View file

@ -27,6 +27,7 @@ import { NotebookOptions } from 'vs/workbench/contrib/notebook/common/notebookOp
import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange';
import { NotebookEditorTestModel, setupInstantiationService, withTestNotebook } from 'vs/workbench/contrib/notebook/test/browser/testNotebookEditor';
import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
import { IBaseCellEditorOptions } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
suite('NotebookViewModel', () => {
let disposables: DisposableStore;
@ -55,7 +56,7 @@ suite('NotebookViewModel', () => {
test('ctor', function () {
const notebook = new NotebookTextModel('notebook', URI.parse('test'), [], {}, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false }, undoRedoService, modelService, languageService);
const model = new NotebookEditorTestModel(notebook);
const viewContext = new ViewContext(new NotebookOptions(instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService)), new NotebookEventDispatcher());
const viewContext = new ViewContext(new NotebookOptions(instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService)), new NotebookEventDispatcher(), () => ({} as IBaseCellEditorOptions));
const viewModel = new NotebookViewModel('notebook', model.notebook, viewContext, null, { isReadOnly: false }, instantiationService, bulkEditService, undoRedoService, textModelService);
assert.strictEqual(viewModel.viewType, 'notebook');
});

View file

@ -36,7 +36,7 @@ import { UndoRedoService } from 'vs/platform/undoRedo/common/undoRedoService';
import { IWorkspaceTrustRequestService } from 'vs/platform/workspace/common/workspaceTrust';
import { EditorInput } from 'vs/workbench/common/editor/editorInput';
import { EditorModel } from 'vs/workbench/common/editor/editorModel';
import { CellFindMatchWithIndex, IActiveNotebookEditorDelegate, ICellViewModel, INotebookEditorDelegate } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { CellFindMatchWithIndex, IActiveNotebookEditorDelegate, IBaseCellEditorOptions, ICellViewModel, INotebookEditorDelegate } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { ListViewInfoAccessor, NotebookCellList } from 'vs/workbench/contrib/notebook/browser/view/notebookCellList';
import { NotebookEventDispatcher } from 'vs/workbench/contrib/notebook/browser/viewModel/eventDispatcher';
import { CellViewModel, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl';
@ -195,7 +195,7 @@ function _createTestNotebookEditor(instantiationService: TestInstantiationServic
const model = new NotebookEditorTestModel(notebook);
const notebookOptions = new NotebookOptions(instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService));
const viewContext = new ViewContext(notebookOptions, new NotebookEventDispatcher());
const viewContext = new ViewContext(notebookOptions, new NotebookEventDispatcher(), () => ({} as IBaseCellEditorOptions));
const viewModel: NotebookViewModel = instantiationService.createInstance(NotebookViewModel, viewType, model.notebook, viewContext, null, { isReadOnly: false });
const cellList = createNotebookCellList(instantiationService, viewContext);
@ -362,7 +362,7 @@ export function createNotebookCellList(instantiationService: TestInstantiationSe
'NotebookCellList',
DOM.$('container'),
DOM.$('body'),
viewContext ?? new ViewContext(new NotebookOptions(instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService)), new NotebookEventDispatcher()),
viewContext ?? new ViewContext(new NotebookOptions(instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService)), new NotebookEventDispatcher(), () => ({} as IBaseCellEditorOptions)),
delegate,
[renderer],
instantiationService.get<IContextKeyService>(IContextKeyService),