Skip to content

Commit c128c6f

Browse files
alexdimaalexdima
committed
Clean up horizontal reveal sequence, do not touch the DOM in onScrollHeightChanged
1 parent 78569ff commit c128c6f

4 files changed

Lines changed: 63 additions & 37 deletions

File tree

‎src/vs/base/browser/styleMutator.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,13 +226,17 @@ export const StyleMutator = {
226226
let desiredValue = top + 'px';
227227
if (domNode.style.top !== desiredValue) {
228228
domNode.style.top = desiredValue;
229+
return true;
229230
}
231+
return false;
230232
},
231233
setLeft: (domNode: HTMLElement, left: number) => {
232234
let desiredValue = left + 'px';
233235
if (domNode.style.left !== desiredValue) {
234236
domNode.style.left = desiredValue;
237+
return true;
235238
}
239+
return false;
236240
},
237241
setBottom: (domNode: HTMLElement, bottom: number) => {
238242
let desiredValue = bottom + 'px';
@@ -272,17 +276,21 @@ export const StyleMutator = {
272276
};
273277

274278
// Define setTransform
275-
function setWebkitTransform(domNode: HTMLElement, desiredValue: string): void {
279+
function setWebkitTransform(domNode: HTMLElement, desiredValue: string): boolean {
276280
if (domNode.getAttribute('data-transform') !== desiredValue) {
277281
domNode.setAttribute('data-transform', desiredValue);
278282
(<any>domNode.style).webkitTransform = desiredValue;
283+
return true;
279284
}
285+
return false;
280286
}
281-
function setTransform(domNode: HTMLElement, desiredValue: string): void {
287+
function setTransform(domNode: HTMLElement, desiredValue: string): boolean {
282288
if (domNode.getAttribute('data-transform') !== desiredValue) {
283289
domNode.setAttribute('data-transform', desiredValue);
284290
domNode.style.transform = desiredValue;
291+
return true;
285292
}
293+
return false;
286294
}
287295
(function() {
288296
let testDomNode = document.createElement('div');

‎src/vs/editor/browser/view/viewImpl.ts‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -868,19 +868,24 @@ export class View extends ViewEventHandler implements editorBrowser.IView, IDisp
868868
return r;
869869
}
870870

871+
private _getViewPartsToRender(): ViewPart[] {
872+
let result:ViewPart[] = [];
873+
for (let i = 0, len = this.viewParts.length; i < len; i++) {
874+
let viewPart = this.viewParts[i];
875+
if (viewPart.shouldRender()) {
876+
result.push(viewPart);
877+
}
878+
}
879+
return result;
880+
}
881+
871882
private _actualRender(): void {
872883
if (!dom.isInDOM(this.domNode)) {
873884
return;
874885
}
875886
let t = timer.start(timer.Topic.EDITOR, 'View.render');
876887

877-
let viewPartsToRender:ViewPart[] = [];
878-
for (let i = 0, len = this.viewParts.length; i < len; i++) {
879-
let viewPart = this.viewParts[i];
880-
if (viewPart.shouldRender()) {
881-
viewPartsToRender.push(viewPart);
882-
}
883-
}
888+
let viewPartsToRender = this._getViewPartsToRender();
884889

885890
if (!this.viewLines.shouldRender() && viewPartsToRender.length === 0) {
886891
// Nothing to render
@@ -893,6 +898,9 @@ export class View extends ViewEventHandler implements editorBrowser.IView, IDisp
893898
if (this.viewLines.shouldRender()) {
894899
this.viewLines.renderText(linesViewportData);
895900
this.viewLines.onDidRender();
901+
902+
// Rendering of viewLines might cause scroll events to occur, so collect view parts to render again
903+
viewPartsToRender = this._getViewPartsToRender();
896904
}
897905

898906
let renderingContext = this.createRenderingContext(linesViewportData);

‎src/vs/editor/browser/view/viewOverlays.ts‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -219,13 +219,15 @@ export class MarginViewOverlays extends ViewOverlays {
219219
private _glyphMarginLeft:number;
220220
private _glyphMarginWidth:number;
221221
private _scrollHeight:number;
222+
private _contentLeft: number;
222223

223224
constructor(context:editorBrowser.IViewContext, layoutProvider:editorBrowser.ILayoutProvider) {
224225
super(context, layoutProvider);
225226

226-
this._glyphMarginLeft = 0;
227-
this._glyphMarginWidth = 0;
227+
this._glyphMarginLeft = context.configuration.editor.layoutInfo.glyphMarginLeft;
228+
this._glyphMarginWidth = context.configuration.editor.layoutInfo.glyphMarginWidth;
228229
this._scrollHeight = layoutProvider.getScrollHeight();
230+
this._contentLeft = context.configuration.editor.layoutInfo.contentLeft;
229231

230232
this.domNode.setClassName(editorBrowser.ClassNames.MARGIN_VIEW_OVERLAYS + ' monaco-editor-background');
231233
this.domNode.setWidth(1);
@@ -251,26 +253,14 @@ export class MarginViewOverlays extends ViewOverlays {
251253

252254
public onScrollHeightChanged(scrollHeight:number): boolean {
253255
this._scrollHeight = scrollHeight;
254-
var glyphMargin = this._getGlyphMarginDomNode();
255-
if (glyphMargin) {
256-
StyleMutator.setHeight(glyphMargin, this._scrollHeight);
257-
}
258256
return super.onScrollHeightChanged(scrollHeight) || true;
259257
}
260258

261259
public onLayoutChanged(layoutInfo:IEditorLayoutInfo): boolean {
262260
this._glyphMarginLeft = layoutInfo.glyphMarginLeft;
263261
this._glyphMarginWidth = layoutInfo.glyphMarginWidth;
264262
this._scrollHeight = this._layoutProvider.getScrollHeight();
265-
266-
this.domNode.setWidth(layoutInfo.contentLeft);
267-
268-
var glyphMargin = this._getGlyphMarginDomNode();
269-
if (glyphMargin) {
270-
StyleMutator.setLeft(glyphMargin, layoutInfo.glyphMarginLeft);
271-
StyleMutator.setWidth(glyphMargin, layoutInfo.glyphMarginWidth);
272-
}
273-
263+
this._contentLeft = layoutInfo.contentLeft;
274264
return super.onLayoutChanged(layoutInfo) || true;
275265
}
276266

@@ -284,5 +274,13 @@ export class MarginViewOverlays extends ViewOverlays {
284274
}
285275
var height = Math.min(this._layoutProvider.getTotalHeight(), 1000000);
286276
this.domNode.setHeight(height);
277+
this.domNode.setWidth(this._contentLeft);
278+
279+
var glyphMargin = this._getGlyphMarginDomNode();
280+
if (glyphMargin) {
281+
StyleMutator.setHeight(glyphMargin, this._scrollHeight);
282+
StyleMutator.setLeft(glyphMargin, this._glyphMarginLeft);
283+
StyleMutator.setWidth(glyphMargin, this._glyphMarginWidth);
284+
}
287285
}
288286
}

‎src/vs/editor/browser/viewParts/lines/viewLines.ts‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ export class ViewLines extends ViewLayer {
140140
public onLayoutChanged(layoutInfo:editorCommon.IEditorLayoutInfo): boolean {
141141
var shouldRender = super.onLayoutChanged(layoutInfo);
142142
this._maxLineWidth = 0;
143+
this._lastRenderedData.resetDomNodeClientRectLeft();
143144
return shouldRender;
144145
}
145146

@@ -379,39 +380,50 @@ export class ViewLines extends ViewLayer {
379380
throw new Error('I did not ask to render!');
380381
}
381382

383+
// (1) render lines - ensures lines are in the DOM
382384
super._renderLines(linesViewportData);
383-
this.onDidRender();
384-
385385
this._lastRenderedData.setBigNumbersDelta(linesViewportData.bigNumbersDelta);
386386
this._lastRenderedData.setCurrentVisibleRange(linesViewportData.visibleRange);
387-
this._lastRenderedData.resetDomNodeClientRectLeft();
387+
this.domNode.setWidth(this._layoutProvider.getScrollWidth());
388+
this.domNode.setHeight(Math.min(this._layoutProvider.getTotalHeight(), 1000000));
388389

390+
// (2) compute horizontal scroll position:
391+
// - this must happen after the lines are in the DOM since it might need a line that rendered just now
392+
// - it might change `scrollWidth` and `scrollLeft`
389393
if (this._lastCursorRevealRangeHorizontallyEvent) {
390-
var newScrollLeft = this._computeScrollLeftToRevealRange(this._lastCursorRevealRangeHorizontallyEvent.range);
394+
let revealHorizontalRange = this._lastCursorRevealRangeHorizontallyEvent.range;
391395
this._lastCursorRevealRangeHorizontallyEvent = null;
392396

397+
// allow `visibleRangesForRange2` to work
398+
this.onDidRender();
399+
400+
// compute new scroll position
401+
var newScrollLeft = this._computeScrollLeftToRevealRange(revealHorizontalRange);
402+
393403
var isViewportWrapping = this._isViewportWrapping;
394404
if (!isViewportWrapping) {
405+
// ensure `scrollWidth` is large enough
395406
this._ensureMaxLineWidth(newScrollLeft.maxHorizontalOffset);
396407
}
397408

409+
// set `scrollLeft`
398410
this._layoutProvider.setScrollLeft(newScrollLeft.scrollLeft);
399411
}
400412

401-
// Update max line width (not so important, it is just so the horizontal scrollbar doesn't get too small)
402-
this._asyncUpdateLineWidths.schedule();
403-
413+
let somethingChanged = false;
404414
if (browser.canUseTranslate3d) {
405415
var transform = 'translate3d(' + -this._layoutProvider.getScrollLeft() + 'px, ' + linesViewportData.visibleRangesDeltaTop + 'px, 0px)';
406-
StyleMutator.setTransform(<HTMLElement>this.domNode.domNode.parentNode, transform); // TODO@Alex
416+
somethingChanged = StyleMutator.setTransform(<HTMLElement>this.domNode.domNode.parentNode, transform) || somethingChanged; // TODO@Alex
407417
} else {
408-
StyleMutator.setTop(<HTMLElement>this.domNode.domNode.parentNode, linesViewportData.visibleRangesDeltaTop); // TODO@Alex
409-
StyleMutator.setLeft(<HTMLElement>this.domNode.domNode.parentNode, -this._layoutProvider.getScrollLeft()); // TODO@Alex
418+
somethingChanged = StyleMutator.setTop(<HTMLElement>this.domNode.domNode.parentNode, linesViewportData.visibleRangesDeltaTop) || somethingChanged; // TODO@Alex
419+
somethingChanged = StyleMutator.setLeft(<HTMLElement>this.domNode.domNode.parentNode, -this._layoutProvider.getScrollLeft()) || somethingChanged; // TODO@Alex
420+
}
421+
if (somethingChanged) {
422+
this._lastRenderedData.resetDomNodeClientRectLeft();
410423
}
411-
this._lastRenderedData.resetDomNodeClientRectLeft();
412424

413-
this.domNode.setWidth(this._layoutProvider.getScrollWidth());
414-
this.domNode.setHeight(Math.min(this._layoutProvider.getTotalHeight(), 1000000));
425+
// Update max line width (not so important, it is just so the horizontal scrollbar doesn't get too small)
426+
this._asyncUpdateLineWidths.schedule();
415427
}
416428

417429
// --- width

0 commit comments

Comments
 (0)