From a52f6bbcc3594206ed957612907bb1f753e3d0fc Mon Sep 17 00:00:00 2001 From: Ole Date: Thu, 31 Mar 2022 12:14:29 +0200 Subject: [PATCH] Update cursor stops after partial renders Large diffs are rendered in chunks of a predefined size, across multiple tasks, to avoid the browser freezing while rendering large files. This change adds a new even "render-progress" whenever such a chunk has been rendered synchronously, and updates the cursor to then query the cursor stops so that users can start moving the cursor on the already rendered lines, even when the browser is still rendering more lines in the background. An AbortStop is still added at the end while loading, to avoid skipping over cursor stops that have not yet been rendered. Release-Notes: skip Change-Id: I2f735231770287fae1432b1e2aa565aa8f422817 --- .../gr-diff-builder-element.ts | 7 ++++++ .../diff/gr-diff-cursor/gr-diff-cursor.ts | 12 ++++++++++ .../app/embed/diff/gr-diff/gr-diff.ts | 10 +++++++-- .../app/embed/diff/gr-diff/gr-diff_test.js | 22 ++++++++++++++++--- 4 files changed, 46 insertions(+), 5 deletions(-) diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts index ed3d43a2cd33..aabdf57d9335 100644 --- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts +++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts @@ -105,6 +105,12 @@ export class GrDiffBuilderElement extends PolymerElement { * @event render-start */ + /** + * Fired whenever a new chunk of lines has been rendered synchronously. + * + * @event render-progress + */ + /** * Fired when the diff finishes rendering text content. * @@ -506,6 +512,7 @@ export class GrDiffBuilderElement extends PolymerElement { ); this._builder.addGroups(added); } + fireEvent(this, 'render-progress'); } _createIntralineLayer(): DiffLayer { diff --git a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts index 35b89ecaa0e4..dfe8a15d69d0 100644 --- a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts +++ b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts @@ -335,6 +335,10 @@ export class GrDiffCursor implements GrDiffCursorApi { this.preventAutoScrollOnManualScroll = true; }; + private _boundHandleDiffRenderProgress = () => { + this._updateStops(); + }; + private _boundHandleDiffRenderContent = () => { this._updateStops(); // When done rendering, turn focus on move and automatic scrolling back on @@ -545,6 +549,10 @@ export class GrDiffCursor implements GrDiffCursorApi { this.boundHandleDiffLoadingChanged ); diff.removeEventListener('render-start', this._boundHandleDiffRenderStart); + diff.removeEventListener( + 'render-progress', + this._boundHandleDiffRenderProgress + ); diff.removeEventListener( 'render-content', this._boundHandleDiffRenderContent @@ -561,6 +569,10 @@ export class GrDiffCursor implements GrDiffCursorApi { this.boundHandleDiffLoadingChanged ); diff.addEventListener('render-start', this._boundHandleDiffRenderStart); + diff.addEventListener( + 'render-progress', + this._boundHandleDiffRenderProgress + ); diff.addEventListener('render-content', this._boundHandleDiffRenderContent); diff.addEventListener('line-selected', this._boundHandleDiffLineSelected); } diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts index 2762c74d810f..a38ec91854a1 100644 --- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts +++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts @@ -485,10 +485,16 @@ export class GrDiff extends PolymerElement implements GrDiffApi { getCursorStops(): Array { if (this.hidden && this.noAutoRender) return []; + // Get rendered stops. + const stops: Array = + this.$.diffBuilder.getLineNumberRows(); + + // If we are still loading this diff, abort after the rendered stops to + // avoid skipping over to e.g. the next file. if (this.loading) { - return [new AbortStop()]; + stops.push(new AbortStop()); } - return this.$.diffBuilder.getLineNumberRows(); + return stops; } isRangeSelected() { diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.js b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.js index 36b5f293aa0d..714005e33a2f 100644 --- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.js +++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.js @@ -25,6 +25,7 @@ import {runA11yAudit} from '../../../test/a11y-test-utils.js'; import '@polymer/paper-button/paper-button.js'; import {Side} from '../../../api/diff.js'; import {mockPromise, stubRestApi} from '../../../test/test-utils.js'; +import {AbortStop} from '../../../api/core.js'; const basicFixture = fixtureFromElement('gr-diff'); @@ -543,23 +544,38 @@ suite('gr-diff tests', () => { }; element._renderDiffTable(); - element._setLoading(false); + flush(); } - test('getCursorStops returns [] when hidden and noAutoRender', () => { + test('returns [] when hidden and noAutoRender', () => { element.noAutoRender = true; setupDiff(); + element._setLoading(false); + flush(); element.hidden = true; assert.equal(element.getCursorStops().length, 0); }); - test('getCursorStops', () => { + test('returns one stop per line and one for the file row', () => { setupDiff(); + element._setLoading(false); + flush(); const ROWS = 48; const FILE_ROW = 1; assert.equal(element.getCursorStops().length, ROWS + FILE_ROW); }); + + test('returns an additional AbortStop when still loading', () => { + setupDiff(); + element._setLoading(true); + flush(); + const ROWS = 48; + const FILE_ROW = 1; + const actual = element.getCursorStops(); + assert.equal(actual.length, ROWS + FILE_ROW + 1); + assert.isTrue(actual[actual.length -1] instanceof AbortStop); + }); }); test('adds .hiddenscroll', () => {