Skip to content

Commit 59e68e1

Browse files
authored
Fix #173982. Respect selection direction in cell reveal. (#174713)
1 parent 10cfd78 commit 59e68e1

File tree

5 files changed

+26
-19
lines changed

5 files changed

+26
-19
lines changed

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import * as editorCommon from 'vs/editor/common/editorCommon';
1212
import { FontInfo } from 'vs/editor/common/config/fontInfo';
1313
import { IPosition } from 'vs/editor/common/core/position';
1414
import { IRange, Range } from 'vs/editor/common/core/range';
15+
import { Selection } from 'vs/editor/common/core/selection';
1516
import { FindMatch, IModelDeltaDecoration, IReadonlyTextBuffer, ITextModel, TrackedRangeStickiness } from 'vs/editor/common/model';
1617
import { MenuId } from 'vs/platform/actions/common/actions';
1718
import { ITextEditorOptions, ITextResourceEditorInput } from 'vs/platform/editor/common/editor';
@@ -601,17 +602,17 @@ export interface INotebookEditor {
601602
/**
602603
* Reveal a range in notebook cell into viewport with minimal scrolling.
603604
*/
604-
revealRangeInViewAsync(cell: ICellViewModel, range: Range): Promise<void>;
605+
revealRangeInViewAsync(cell: ICellViewModel, range: Selection | Range): Promise<void>;
605606

606607
/**
607608
* Reveal a range in notebook cell into viewport center.
608609
*/
609-
revealRangeInCenterAsync(cell: ICellViewModel, range: Range): Promise<void>;
610+
revealRangeInCenterAsync(cell: ICellViewModel, range: Selection | Range): Promise<void>;
610611

611612
/**
612613
* Reveal a range in notebook cell into viewport center.
613614
*/
614-
revealRangeInCenterIfOutsideViewportAsync(cell: ICellViewModel, range: Range): Promise<void>;
615+
revealRangeInCenterIfOutsideViewportAsync(cell: ICellViewModel, range: Selection | Range): Promise<void>;
615616

616617
/**
617618
* Reveal a position with `offset` in a cell into viewport center.

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
3232
import { IEditorOptions } from 'vs/editor/common/config/editorOptions';
3333
import { BareFontInfo, FontInfo } from 'vs/editor/common/config/fontInfo';
3434
import { Range } from 'vs/editor/common/core/range';
35+
import { Selection } from 'vs/editor/common/core/selection';
3536
import { IEditor } from 'vs/editor/common/editorCommon';
3637
import { SuggestController } from 'vs/editor/contrib/suggest/browser/suggestController';
3738
import * as nls from 'vs/nls';
@@ -1988,15 +1989,15 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
19881989
return this._list.revealCellRangeAsync(cell, new Range(line, 1, line, 1), CellRevealRangeType.CenterIfOutsideViewport);
19891990
}
19901991

1991-
async revealRangeInViewAsync(cell: ICellViewModel, range: Range): Promise<void> {
1992+
async revealRangeInViewAsync(cell: ICellViewModel, range: Selection | Range): Promise<void> {
19921993
return this._list.revealCellRangeAsync(cell, range, CellRevealRangeType.Default);
19931994
}
19941995

1995-
async revealRangeInCenterAsync(cell: ICellViewModel, range: Range): Promise<void> {
1996+
async revealRangeInCenterAsync(cell: ICellViewModel, range: Selection | Range): Promise<void> {
19961997
return this._list.revealCellRangeAsync(cell, range, CellRevealRangeType.Center);
19971998
}
19981999

1999-
async revealRangeInCenterIfOutsideViewportAsync(cell: ICellViewModel, range: Range): Promise<void> {
2000+
async revealRangeInCenterIfOutsideViewportAsync(cell: ICellViewModel, range: Selection | Range): Promise<void> {
20002001
return this._list.revealCellRangeAsync(cell, range, CellRevealRangeType.CenterIfOutsideViewport);
20012002
}
20022003

src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { Disposable, DisposableStore, IDisposable, MutableDisposable } from 'vs/
1212
import { isMacintosh } from 'vs/base/common/platform';
1313
import { ScrollEvent } from 'vs/base/common/scrollable';
1414
import { Range } from 'vs/editor/common/core/range';
15+
import { Selection } from 'vs/editor/common/core/selection';
1516
import { TrackedRangeStickiness } from 'vs/editor/common/model';
1617
import { PrefixSumComputer } from 'vs/editor/common/model/prefixSumComputer';
1718
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
@@ -914,7 +915,7 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
914915
//#endregion
915916

916917
//#region Reveal Cell Editor Range asynchronously
917-
async revealCellRangeAsync(cell: ICellViewModel, range: Range, revealType: CellRevealRangeType): Promise<void> {
918+
async revealCellRangeAsync(cell: ICellViewModel, range: Selection | Range, revealType: CellRevealRangeType): Promise<void> {
918919
const index = this._getViewIndexUpperBound(cell);
919920

920921
if (index < 0) {
@@ -934,7 +935,7 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
934935
// List items have real dynamic heights, which means after we set `scrollTop` based on the `elementTop(index)`, the element at `index` might still be removed from the view once all relayouting tasks are done.
935936
// For example, we scroll item 10 into the view upwards, in the first round, items 7, 8, 9, 10 are all in the viewport. Then item 7 and 8 resize themselves to be larger and finally item 10 is removed from the view.
936937
// To ensure that item 10 is always there, we need to scroll item 10 to the top edge of the viewport.
937-
private async _revealRangeInternalAsync(viewIndex: number, range: Range, revealType: CellEditorRevealType): Promise<void> {
938+
private async _revealRangeInternalAsync(viewIndex: number, range: Selection | Range, revealType: CellEditorRevealType): Promise<void> {
938939
const scrollTop = this.getViewScrollTop();
939940
const wrapperBottom = this.getViewScrollBottom();
940941
const elementTop = this.view.elementTop(viewIndex);
@@ -968,10 +969,10 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
968969
}
969970
}
970971

971-
private async _revealRangeInCenterInternalAsync(viewIndex: number, range: Range, revealType: CellEditorRevealType): Promise<void> {
972+
private async _revealRangeInCenterInternalAsync(viewIndex: number, range: Selection | Range, revealType: CellEditorRevealType): Promise<void> {
972973
const reveal = (viewIndex: number, range: Range, revealType: CellEditorRevealType) => {
973974
const element = this.view.element(viewIndex);
974-
const positionOffset = element.getPositionScrollTopOffset(range.startLineNumber, range.startColumn);
975+
const positionOffset = element.getPositionScrollTopOffset(range);
975976
const positionOffsetInView = this.view.elementTop(viewIndex) + positionOffset;
976977
this.view.setScrollTop(positionOffsetInView - this.view.renderHeight / 2);
977978

@@ -992,10 +993,10 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
992993
}
993994
}
994995

995-
private async _revealRangeInCenterIfOutsideViewportInternalAsync(viewIndex: number, range: Range, revealType: CellEditorRevealType): Promise<void> {
996+
private async _revealRangeInCenterIfOutsideViewportInternalAsync(viewIndex: number, range: Selection | Range, revealType: CellEditorRevealType): Promise<void> {
996997
const reveal = (viewIndex: number, range: Range, revealType: CellEditorRevealType) => {
997998
const element = this.view.element(viewIndex);
998-
const positionOffset = element.getPositionScrollTopOffset(range.startLineNumber, range.startColumn);
999+
const positionOffset = element.getPositionScrollTopOffset(range);
9991000
const positionOffsetInView = this.view.elementTop(viewIndex) + positionOffset;
10001001
this.view.setScrollTop(positionOffsetInView - this.view.renderHeight / 2);
10011002

@@ -1009,14 +1010,14 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
10091010
const elementTop = this.view.elementTop(viewIndex);
10101011
const viewItemOffset = elementTop;
10111012
const element = this.view.element(viewIndex);
1012-
const positionOffset = viewItemOffset + element.getPositionScrollTopOffset(range.startLineNumber, range.startColumn);
1013+
const positionOffset = viewItemOffset + element.getPositionScrollTopOffset(range);
10131014

10141015
if (positionOffset < scrollTop || positionOffset > wrapperBottom) {
10151016
// let it render
10161017
this.view.setScrollTop(positionOffset - this.view.renderHeight / 2);
10171018

10181019
// after rendering, it might be pushed down due to markdown cell dynamic height
1019-
const newPositionOffset = this.view.elementTop(viewIndex) + element.getPositionScrollTopOffset(range.startLineNumber, range.startColumn);
1020+
const newPositionOffset = this.view.elementTop(viewIndex) + element.getPositionScrollTopOffset(range);
10201021
this.view.setScrollTop(newPositionOffset - this.view.renderHeight / 2);
10211022

10221023
// reveal editor
@@ -1035,11 +1036,11 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
10351036
}
10361037
}
10371038

1038-
private _revealRangeCommon(viewIndex: number, range: Range, revealType: CellEditorRevealType, newlyCreated: boolean, alignToBottom: boolean) {
1039+
private _revealRangeCommon(viewIndex: number, range: Selection | Range, revealType: CellEditorRevealType, newlyCreated: boolean, alignToBottom: boolean) {
10391040
const element = this.view.element(viewIndex);
10401041
const scrollTop = this.getViewScrollTop();
10411042
const wrapperBottom = this.getViewScrollBottom();
1042-
const positionOffset = element.getPositionScrollTopOffset(range.startLineNumber, range.startColumn);
1043+
const positionOffset = element.getPositionScrollTopOffset(range);
10431044
const elementOriginalHeight = this.view.elementHeight(viewIndex);
10441045
if (positionOffset >= elementOriginalHeight) {
10451046
// we are revealing a range that is beyond current element height

src/vs/workbench/contrib/notebook/browser/view/notebookRenderingCommon.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle';
1212
import { ScrollEvent } from 'vs/base/common/scrollable';
1313
import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
1414
import { Range } from 'vs/editor/common/core/range';
15+
import { Selection } from 'vs/editor/common/core/selection';
1516
import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
1617
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
1718
import { IWorkbenchListOptionsUpdate } from 'vs/platform/list/browser/listService';
@@ -65,7 +66,7 @@ export interface INotebookCellList {
6566
scrollToBottom(): void;
6667
revealCell(cell: ICellViewModel, revealType: CellRevealSyncType): void;
6768
revealCellAsync(cell: ICellViewModel, revealType: CellRevealType): Promise<void>;
68-
revealCellRangeAsync(cell: ICellViewModel, range: Range, revealType: CellRevealRangeType): Promise<void>;
69+
revealCellRangeAsync(cell: ICellViewModel, range: Selection | Range, revealType: CellRevealRangeType): Promise<void>;
6970
revealCellOffsetInCenterAsync(element: ICellViewModel, offset: number): Promise<void>;
7071
setHiddenAreas(_ranges: ICellRange[], triggerViewUpdate: boolean): boolean;
7172
domElementOfElement(element: ICellViewModel): HTMLElement | null;

src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -487,13 +487,16 @@ export abstract class BaseCellViewModel extends Disposable {
487487
return this._textEditor.getTopForLineNumber(line) + editorPadding.top;
488488
}
489489

490-
getPositionScrollTopOffset(line: number, column: number): number {
490+
getPositionScrollTopOffset(range: Selection | Range): number {
491491
if (!this._textEditor) {
492492
return 0;
493493
}
494494

495+
496+
const position = range instanceof Selection ? range.getPosition() : range.getStartPosition();
497+
495498
const editorPadding = this._viewContext.notebookOptions.computeEditorPadding(this.internalMetadata, this.uri);
496-
return this._textEditor.getTopForPosition(line, column) + editorPadding.top;
499+
return this._textEditor.getTopForPosition(position.lineNumber, position.column) + editorPadding.top;
497500
}
498501

499502
cursorAtBoundary(): CursorAtBoundary {

0 commit comments

Comments
 (0)