From 38f2a92f435f585ac0b2ff7d01feb6232c696190 Mon Sep 17 00:00:00 2001 From: Dhruv Srivastava Date: Mon, 22 Jun 2020 11:59:13 +0200 Subject: [PATCH] Navigate to next unreviewed file after moving to last diff chunk Show toast to users telling them pressing n on the last diff chunk of a file will navigate them to next unreviewed file. Pressing n after displaying the toast does the navigation. Change-Id: I8c1248b43ceb88cd464b8ab92eecb57e2d57b39d --- .../diff/gr-diff-cursor/gr-diff-cursor.js | 5 ++- .../gr-diff-cursor/gr-diff-cursor_test.html | 15 +++++++ .../diff/gr-diff-view/gr-diff-view.js | 3 +- .../diff/gr-diff-view/gr-diff-view_html.js | 1 + .../gr-cursor-manager/gr-cursor-manager.js | 39 +++++++++++++++++-- 5 files changed, 57 insertions(+), 6 deletions(-) diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js index acb8f0c778..9d68ba3235 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js @@ -202,9 +202,10 @@ class GrDiffCursor extends GestureEventListeners( } } - moveToNextChunk(opt_clipToTop) { + moveToNextChunk(opt_clipToTop, opt_navigateToNextFile) { this.$.cursorManager.next(this._isFirstRowOfChunk.bind(this), - target => target.parentNode.scrollHeight, opt_clipToTop); + target => target.parentNode.scrollHeight, opt_clipToTop, + opt_navigateToNextFile); this._fixSide(); } diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html index 71fc3410c9..440b233c52 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html @@ -244,6 +244,21 @@ suite('gr-diff-cursor tests', () => { assert.equal(cursorElement.side, 'left'); }); + test('navigate to next unreviewed file via moveToNextChunk', () => { + const cursor = cursorElement.shadowRoot.querySelector('#cursorManager'); + cursor.index = cursor.stops.length - 1; + const dispatchEventStub = sandbox.stub(cursor, 'dispatchEvent'); + cursorElement.moveToNextChunk(/* opt_clipToTop = */false, + /* opt_navigateToNextFile = */true); + assert.isTrue(dispatchEventStub.called); + assert.equal(dispatchEventStub.getCall(0).args[0].type, 'show-alert'); + + cursorElement.moveToNextChunk(/* opt_clipToTop = */false, + /* opt_navigateToNextFile = */true); + assert.equal(dispatchEventStub.getCall(1).args[0].type, + 'navigate-to-next-unreviewed-file'); + }); + test('initialLineNumber not provided', done => { let scrollBehaviorDuringMove; const moveToNumStub = sandbox.stub(cursorElement, 'moveToLineNumber'); diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js index 19abdaf22e..cf89c6308a 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js @@ -556,7 +556,8 @@ class GrDiffView extends mixinBehaviors( [ this.$.cursor.moveToNextCommentThread(); } else { if (this.modifierPressed(e)) { return; } - this.$.cursor.moveToNextChunk(); + this.$.cursor.moveToNextChunk(/* opt_clipToTop = */false, + /* opt_navigateToNextFile = */true); } } diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.js index 3d30f77757..2b7d363e9a 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.js @@ -422,6 +422,7 @@ export const htmlTemplate = html` `; diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js index 647f41b4e0..4bb79c9b58 100644 --- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js +++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js @@ -119,11 +119,15 @@ class GrCursorManager extends GestureEventListeners( * sometimes different, used by the diff cursor. * @param {boolean=} opt_clipToTop When none of the next indices match, move * back to first instead of to last. + * @param {boolean=} opt_navigateToNextFile Navigate to next unreviewed file + * if user presses next on the last diff chunk * @private */ - next(opt_condition, opt_getTargetHeight, opt_clipToTop) { - this._moveCursor(1, opt_condition, opt_getTargetHeight, opt_clipToTop); + next(opt_condition, opt_getTargetHeight, opt_clipToTop, + opt_navigateToNextFile) { + this._moveCursor(1, opt_condition, opt_getTargetHeight, opt_clipToTop, + opt_navigateToNextFile); } previous(opt_condition) { @@ -265,9 +269,12 @@ class GrCursorManager extends GestureEventListeners( * sometimes different, used by the diff cursor. * @param {boolean=} opt_clipToTop When none of the next indices match, move * back to first instead of to last. + * @param {boolean=} opt_navigateToNextFile Navigate to next unreviewed file + * if user presses next on the last diff chunk * @private */ - _moveCursor(delta, opt_condition, opt_getTargetHeight, opt_clipToTop) { + _moveCursor(delta, opt_condition, opt_getTargetHeight, opt_clipToTop, + opt_navigateToNextFile) { if (!this.stops.length) { this.unsetCursor(); return; @@ -282,6 +289,32 @@ class GrCursorManager extends GestureEventListeners( newTarget = this.stops[newIndex]; } + /* + * If user presses n on the last diff chunk, show a toast informing user + * that pressing n again will navigate them to next unreviewed file + */ + if (opt_navigateToNextFile && this.index === newIndex) { + if (newIndex === this.stops.length - 1) { + if (this._displayedNavigateToNextFileToast) { + // reset for next file + this._displayedNavigateToNextFileToast = false; + this.dispatchEvent(new CustomEvent( + 'navigate-to-next-unreviewed-file', { + composed: true, bubbles: true, + })); + return; + } + this._displayedNavigateToNextFileToast = true; + this.dispatchEvent(new CustomEvent('show-alert', { + detail: { + message: 'Press n again to navigate to next unreviewed file', + }, + composed: true, bubbles: true, + })); + return; + } + } + this.index = newIndex; this.target = newTarget;