From 261fcf2d57f33bdd5b5145c25df7829bb12d9d07 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Mon, 24 Jul 2023 16:46:01 -0700 Subject: [PATCH] Fix #188428. Top Insert Toolbar height workaround is no longer needed. (#188726) --- .../notebook/browser/view/notebookCellList.ts | 9 ++------ .../test/browser/notebookCellList.test.ts | 22 ++++++------------- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts index 4e3eb4b10a5..0da4dd25742 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -135,8 +135,6 @@ export class NotebookCellList extends WorkbenchList implements ID private _isInLayout: boolean = false; - private readonly _viewContext: ViewContext; - private _webviewElement: FastDomNode | null = null; get webviewElement() { @@ -161,7 +159,6 @@ export class NotebookCellList extends WorkbenchList implements ID ) { super(listUser, container, delegate, renderers, options, contextKeyService, listService, configurationService, instantiationService); NOTEBOOK_CELL_LIST_FOCUSED.bindTo(this.contextKeyService).set(true); - this._viewContext = viewContext; this._previousFocusedElements = this.getFocusedElements(); this._localDisposableStore.add(this.onDidChangeFocus((e) => { this._previousFocusedElements.forEach(element => { @@ -826,9 +823,8 @@ export class NotebookCellList extends WorkbenchList implements ID const scrollHeight = this.view.scrollHeight; const scrollTop = this.getViewScrollTop(); const wrapperBottom = this.getViewScrollBottom(); - const topInsertToolbarHeight = this._viewContext.notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType); - this.view.setScrollTop(scrollHeight - (wrapperBottom - scrollTop) - topInsertToolbarHeight); + this.view.setScrollTop(scrollHeight - (wrapperBottom - scrollTop)); } //#region Reveal Cell synchronously @@ -1226,8 +1222,7 @@ export class NotebookCellList extends WorkbenchList implements ID } getViewScrollBottom() { - const topInsertToolbarHeight = this._viewContext.notebookOptions.computeTopInsertToolbarHeight(this.viewModel?.viewType); - return this.getViewScrollTop() + this.view.renderHeight - topInsertToolbarHeight; + return this.getViewScrollTop() + this.view.renderHeight; } setCellEditorSelection(cell: ICellViewModel, range: Range) { diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookCellList.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookCellList.test.ts index b8f060ed210..0d8e843411f 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookCellList.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookCellList.test.ts @@ -5,25 +5,17 @@ import * as assert from 'assert'; import { DisposableStore } from 'vs/base/common/lifecycle'; -import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; -import { NotebookOptions } from 'vs/workbench/contrib/notebook/browser/notebookOptions'; import { createNotebookCellList, setupInstantiationService, withTestNotebook } from 'vs/workbench/contrib/notebook/test/browser/testNotebookEditor'; suite('NotebookCellList', () => { let disposables: DisposableStore; let instantiationService: TestInstantiationService; - let notebookDefaultOptions: NotebookOptions; - let topInsertToolbarHeight: number; suiteSetup(() => { disposables = new DisposableStore(); instantiationService = setupInstantiationService(disposables); - notebookDefaultOptions = new NotebookOptions(instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService), false); - topInsertToolbarHeight = notebookDefaultOptions.computeTopInsertToolbarHeight(); - }); suiteTeardown(() => disposables.dispose()); @@ -51,7 +43,7 @@ suite('NotebookCellList', () => { cellList.attachViewModel(viewModel); // render height 210, it can render 3 full cells and 1 partial cell - cellList.layout(210 + topInsertToolbarHeight, 100); + cellList.layout(210, 100); // scroll a bit, scrollTop to bottom: 5, 215 cellList.scrollTop = 5; @@ -98,7 +90,7 @@ suite('NotebookCellList', () => { cellList.attachViewModel(viewModel); // render height 210, it can render 3 full cells and 1 partial cell - cellList.layout(210 + topInsertToolbarHeight, 100); + cellList.layout(210, 100); // init scrollTop and scrollBottom assert.deepStrictEqual(cellList.scrollTop, 0); @@ -144,7 +136,7 @@ suite('NotebookCellList', () => { cellList.attachViewModel(viewModel); // render height 210, it can render 3 full cells and 1 partial cell - cellList.layout(210 + topInsertToolbarHeight, 100); + cellList.layout(210, 100); // init scrollTop and scrollBottom assert.deepStrictEqual(cellList.scrollTop, 0); @@ -179,7 +171,7 @@ suite('NotebookCellList', () => { cellList.attachViewModel(viewModel); // render height 210, it can render 3 full cells and 1 partial cell - cellList.layout(210 + topInsertToolbarHeight, 100); + cellList.layout(210, 100); // init scrollTop and scrollBottom assert.deepStrictEqual(cellList.scrollTop, 0); @@ -221,7 +213,7 @@ suite('NotebookCellList', () => { cellList.attachViewModel(viewModel); // render height 210, it can render 3 full cells and 1 partial cell - cellList.layout(210 + topInsertToolbarHeight, 100); + cellList.layout(210, 100); // init scrollTop and scrollBottom assert.deepStrictEqual(cellList.scrollTop, 0); @@ -274,7 +266,7 @@ suite('NotebookCellList', () => { cellList.attachViewModel(viewModel); // render height 210, it can render 3 full cells and 1 partial cell - cellList.layout(210 + topInsertToolbarHeight, 100); + cellList.layout(210, 100); // init scrollTop and scrollBottom assert.deepStrictEqual(cellList.scrollTop, 0); @@ -310,7 +302,7 @@ suite('NotebookCellList', () => { cellList.attachViewModel(viewModel); // render height 210, it can render 3 full cells and 1 partial cell - cellList.layout(210 + topInsertToolbarHeight, 100); + cellList.layout(210, 100); // init scrollTop and scrollBottom assert.deepStrictEqual(cellList.scrollTop, 0);