Merge "Update cursor stops after partial renders"

Itiviti · Apr 7, 2022 · 0beddf9 · 0beddf9
2 parents 3bf27ea + a52f6bb
commit 0beddf9
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 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
@@ -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
@@ -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
@@ -485,10 +485,16 @@ export class GrDiff extends PolymerElement implements GrDiffApi {
   getCursorStops(): Array<HTMLElement | AbortStop> {
     if (this.hidden && this.noAutoRender) return [];
 
+    // Get rendered stops.
+    const stops: Array<HTMLElement | AbortStop> =
+      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
@@ -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', () => {