Merge "Implement generic matching for integer fields in ChangePredicate"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
index 049bb19..69c6139 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -35,7 +35,6 @@
import {PolymerDomWrapper} from '../../../types/types';
import {GrDiffGroupType} from '../gr-diff/gr-diff-group';
import {GrDiff} from '../gr-diff/gr-diff';
-import {fireAlert, fireEvent} from '../../../utils/event-util';
import {Subscription} from 'rxjs';
import {toggleClass} from '../../../utils/dom-util';
@@ -44,9 +43,6 @@
const LEFT_SIDE_CLASS = 'target-side-left';
const RIGHT_SIDE_CLASS = 'target-side-right';
-// Time in which pressing n key again after the toast navigates to next file
-const NAVIGATE_TO_NEXT_FILE_TIMEOUT_MS = 5000;
-
export interface GrDiffCursor {
$: {};
}
@@ -59,8 +55,6 @@
private preventAutoScrollOnManualScroll = false;
- private lastDisplayedNavigateToFileToast: Map<string, number> = new Map();
-
@property({type: String})
side = Side.RIGHT;
@@ -190,65 +184,27 @@
}
}
- private showToastAndFireEvent(direction: string, shortcut: string) {
- /*
- * If user presses p/n on the first/last diff chunk, show a toast informing
- * user that pressing it again will navigate them to previous/next
- * unreviewedfile if click happens within the time limit
- */
- if (
- this.lastDisplayedNavigateToFileToast.get(direction) &&
- Date.now() - this.lastDisplayedNavigateToFileToast.get(direction)! <=
- NAVIGATE_TO_NEXT_FILE_TIMEOUT_MS
- ) {
- // reset for next file
- this.lastDisplayedNavigateToFileToast.delete(direction);
- fireEvent(this, `navigate-to-${direction}-unreviewed-file`);
- } else {
- this.lastDisplayedNavigateToFileToast.set(direction, Date.now());
- fireAlert(
- this,
- `Press ${shortcut} again to navigate to ${direction} unreviewed file`
- );
- }
- }
-
- moveToNextChunk(
- clipToTop?: boolean,
- navigateToNextFile?: boolean
- ): CursorMoveResult {
+ moveToNextChunk(clipToTop?: boolean): CursorMoveResult {
const result = this.cursorManager.next({
filter: (row: HTMLElement) => this._isFirstRowOfChunk(row),
getTargetHeight: target =>
(target?.parentNode as HTMLElement)?.scrollHeight || 0,
clipToTop,
});
- if (
- navigateToNextFile &&
- result === CursorMoveResult.CLIPPED &&
- this.isAtEnd()
- ) {
- this.showToastAndFireEvent('next', 'n');
- }
-
this._fixSide();
return result;
}
- moveToPreviousChunk(navigateToPreviousFile?: boolean): CursorMoveResult {
+ moveToPreviousChunk(): CursorMoveResult {
const result = this.cursorManager.previous({
filter: (row: HTMLElement) => this._isFirstRowOfChunk(row),
});
- if (navigateToPreviousFile && this.isAtStart()) {
- this.showToastAndFireEvent('previous', 'p');
- }
this._fixSide();
return result;
}
moveToNextCommentThread(): CursorMoveResult {
if (this.isAtEnd()) {
- fireEvent(this, 'navigate-to-next-file-with-comments');
return CursorMoveResult.CLIPPED;
}
const result = this.cursorManager.next({
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
index 901f72a..7b72da8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
@@ -445,21 +445,6 @@
});
});
- test('navigate to next unreviewed file via moveToNextChunk', () => {
- const cursorManager = cursorElement.cursorManager;
- cursorManager.index = cursorManager.stops.length - 1;
- const dispatchEventStub = sinon.stub(cursorElement, 'dispatchEvent');
- cursorElement.moveToNextChunk(/* opt_clipToTop = */false,
- /* opt_navigateToNextFile = */true);
- assert.isTrue(dispatchEventStub.called);
- assert.equal(dispatchEventStub.getCall(1).args[0].type, 'show-alert');
-
- cursorElement.moveToNextChunk(/* opt_clipToTop = */false,
- /* opt_navigateToNextFile = */true);
- assert.equal(dispatchEventStub.getCall(2).args[0].type,
- 'navigate-to-next-unreviewed-file');
- });
-
test('initialLineNumber not provided', done => {
let scrollBehaviorDuringMove;
const moveToNumStub = sinon.stub(cursorElement, 'moveToLineNumber');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 638b49e..b7ac0de 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -106,10 +106,15 @@
import {GerritView} from '../../../services/router/router-model';
import {assertIsDefined} from '../../../utils/common-util';
import {toggleClass, getKeyboardEvent} from '../../../utils/dom-util';
+import {CursorMoveResult} from '../../shared/gr-cursor-manager/gr-cursor-manager';
+
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const MSG_LOADING_BLAME = 'Loading blame...';
const MSG_LOADED_BLAME = 'Blame loaded';
+// Time in which pressing n key again after the toast navigates to next file
+const NAVIGATE_TO_NEXT_FILE_TIMEOUT_MS = 5000;
+
interface Files {
sortedFileList: string[];
changeFilesByPath: {[path: string]: FileInfo};
@@ -623,17 +628,62 @@
e.preventDefault();
if (e.detail.keyboardEvent?.shiftKey) {
- this.$.cursor.moveToNextCommentThread();
+ const result = this.$.cursor.moveToNextCommentThread();
+ if (result === CursorMoveResult.CLIPPED) {
+ this._navigateToNextFileWithCommentThread();
+ }
} else {
if (this.modifierPressed(e)) return;
+ const result = this.$.cursor.moveToNextChunk();
// navigate to next file if key is not being held down
- this.$.cursor.moveToNextChunk(
- /* opt_clipToTop = */ false,
- /* opt_navigateToNextFile = */ !e.detail.keyboardEvent?.repeat
+ if (
+ !e.detail.keyboardEvent?.repeat &&
+ result === CursorMoveResult.CLIPPED &&
+ this.$.cursor.isAtEnd()
+ ) {
+ this.showToastAndNavigateFile('next', 'n');
+ }
+ }
+ }
+
+ private lastDisplayedNavigateToFileToast: Map<string, number> = new Map();
+
+ private showToastAndNavigateFile(direction: string, shortcut: string) {
+ /*
+ * If user presses p/n on the first/last diff chunk, show a toast informing
+ * user that pressing it again will navigate them to previous/next
+ * unreviewedfile if click happens within the time limit
+ */
+ if (
+ this.lastDisplayedNavigateToFileToast.get(direction) &&
+ Date.now() - this.lastDisplayedNavigateToFileToast.get(direction)! <=
+ NAVIGATE_TO_NEXT_FILE_TIMEOUT_MS
+ ) {
+ // reset for next file
+ this.lastDisplayedNavigateToFileToast.delete(direction);
+ this.navigateToUnreviewedFile(direction);
+ } else {
+ this.lastDisplayedNavigateToFileToast.set(direction, Date.now());
+ fireAlert(
+ this,
+ `Press ${shortcut} again to navigate to ${direction} unreviewed file`
);
}
}
+ private navigateToUnreviewedFile(direction: string) {
+ if (!this._path) return;
+ if (!this._fileList) return;
+ if (!this._reviewedFiles) return;
+ // Ensure that the currently viewed file always appears in unreviewedFiles
+ // so we resolve the right "next" file.
+ const unreviewedFiles = this._fileList.filter(
+ file => file === this._path || !this._reviewedFiles.has(file)
+ );
+
+ this._navToFile(this._path, unreviewedFiles, direction === 'next' ? 1 : -1);
+ }
+
_handlePrevChunkOrCommentThread(e: CustomKeyboardEvent) {
if (this.shouldSuppressKeyboardShortcut(e)) return;
@@ -642,7 +692,10 @@
this.$.cursor.moveToPreviousCommentThread();
} else {
if (this.modifierPressed(e)) return;
- this.$.cursor.moveToPreviousChunk(!e.detail.keyboardEvent?.repeat);
+ this.$.cursor.moveToPreviousChunk();
+ if (!e.detail.keyboardEvent?.repeat && this.$.cursor.isAtStart()) {
+ this.showToastAndNavigateFile('previous', 'p');
+ }
}
}
@@ -1753,34 +1806,10 @@
return disableDiffPrefs || !loggedIn;
}
- _navigateToNextUnreviewedFile() {
- if (!this._path) return;
- if (!this._fileList) return;
- if (!this._reviewedFiles) return;
- // Ensure that the currently viewed file always appears in unreviewedFiles
- // so we resolve the right "next" file.
- const unreviewedFiles = this._fileList.filter(
- file => file === this._path || !this._reviewedFiles.has(file)
- );
- this._navToFile(this._path, unreviewedFiles, 1);
- }
-
- _navigateToPreviousUnreviewedFile() {
- if (!this._path) return;
- if (!this._fileList) return;
- if (!this._reviewedFiles) return;
- // Ensure that the currently viewed file always appears in unreviewedFiles
- // so we resolve the right "next" file.
- const unreviewedFiles = this._fileList.filter(
- file => file === this._path || !this._reviewedFiles.has(file)
- );
- this._navToFile(this._path, unreviewedFiles, -1);
- }
-
_handleNextUnreviewedFile(e: CustomKeyboardEvent) {
if (this.shouldSuppressKeyboardShortcut(e)) return;
this._setReviewed(true);
- this._navigateToNextUnreviewedFile();
+ this.navigateToUnreviewedFile('next');
}
_navigateToNextFileWithCommentThread() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
index 63bf74e..7897a4a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
@@ -429,11 +429,6 @@
on-reload-diff-preference="_handleReloadingDiffPreference"
>
</gr-diff-preferences-dialog>
- <gr-diff-cursor
- id="cursor"
- on-navigate-to-next-unreviewed-file="_navigateToNextUnreviewedFile"
- on-navigate-to-previous-unreviewed-file="_navigateToPreviousUnreviewedFile"
- on-navigate-to-next-file-with-comments="_navigateToNextFileWithCommentThread"
- ></gr-diff-cursor>
+ <gr-diff-cursor id="cursor"></gr-diff-cursor>
<gr-comment-api id="commentAPI"></gr-comment-api>
`;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 244bafb..39ec384 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -29,6 +29,8 @@
createComment,
} from '../../../test/test-data-generators.js';
import {EditPatchSetNum} from '../../../types/common.js';
+import sinon from 'sinon/pkg/sinon-esm';
+import {CursorMoveResult} from '../../shared/gr-cursor-manager/gr-cursor-manager.js';
const basicFixture = fixtureFromElement('gr-diff-view');
@@ -1735,6 +1737,133 @@
});
});
+ suite('switching files', () => {
+ let dispatchEventStub;
+ let navToFileStub;
+ let moveToPreviousChunkStub;
+ let moveToNextChunkStub;
+ let isAtStartStub;
+ let isAtEndStub;
+ let nowStub;
+
+ setup(() => {
+ dispatchEventStub = sinon.stub(
+ element, 'dispatchEvent').callThrough();
+ navToFileStub = sinon.stub(element, '_navToFile');
+ moveToPreviousChunkStub =
+ sinon.stub(element.$.cursor, 'moveToPreviousChunk');
+ moveToNextChunkStub =
+ sinon.stub(element.$.cursor, 'moveToNextChunk');
+ isAtStartStub = sinon.stub(element.$.cursor, 'isAtStart');
+ isAtEndStub = sinon.stub(element.$.cursor, 'isAtEnd');
+ nowStub = sinon.stub(Date, 'now');
+ });
+
+ test('shows toast when at the end of file', () => {
+ moveToNextChunkStub.returns(CursorMoveResult.CLIPPED);
+ isAtEndStub.returns(true);
+
+ MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
+
+ assert.isTrue(moveToNextChunkStub.called);
+ assert.equal(dispatchEventStub.lastCall.args[0].type, 'show-alert');
+ assert.isFalse(navToFileStub.called);
+ });
+
+ test('navigates to next file when n is tapped again', () => {
+ moveToNextChunkStub.returns(CursorMoveResult.CLIPPED);
+ isAtEndStub.returns(true);
+
+ element._files = getFilesFromFileList(['file1', 'file2', 'file3']);
+ element._reviewedFiles = new Set(['file2']);
+ element._path = 'file1';
+
+ nowStub.returns(5);
+ MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
+ nowStub.returns(10);
+ MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
+
+ assert.isTrue(navToFileStub.called);
+ assert.deepEqual(navToFileStub.lastCall.args, [
+ 'file1',
+ ['file1', 'file3'],
+ 1,
+ ]);
+ });
+
+ test('does not navigate if n is tapped twice too slow', () => {
+ moveToNextChunkStub.returns(CursorMoveResult.CLIPPED);
+ isAtEndStub.returns(true);
+
+ nowStub.returns(5);
+ MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
+ nowStub.returns(6000);
+ MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
+
+ assert.isFalse(navToFileStub.called);
+ });
+
+ test('shows toast when at the start of file', () => {
+ moveToPreviousChunkStub.returns(CursorMoveResult.CLIPPED);
+ isAtStartStub.returns(true);
+
+ MockInteractions.pressAndReleaseKeyOn(element, 80, null, 'p');
+
+ assert.isTrue(moveToPreviousChunkStub.called);
+ assert.equal(dispatchEventStub.lastCall.args[0].type, 'show-alert');
+ assert.isFalse(navToFileStub.called);
+ });
+
+ test('navigates to prev file when p is tapped again', () => {
+ moveToPreviousChunkStub.returns(CursorMoveResult.CLIPPED);
+ isAtStartStub.returns(true);
+
+ element._files = getFilesFromFileList(['file1', 'file2', 'file3']);
+ element._reviewedFiles = new Set(['file2']);
+ element._path = 'file3';
+
+ nowStub.returns(5);
+ MockInteractions.pressAndReleaseKeyOn(element, 80, null, 'p');
+ nowStub.returns(10);
+ MockInteractions.pressAndReleaseKeyOn(element, 80, null, 'p');
+
+ assert.isTrue(navToFileStub.called);
+ assert.deepEqual(navToFileStub.lastCall.args, [
+ 'file3',
+ ['file1', 'file3'],
+ -1,
+ ]);
+ });
+
+ test('does not navigate if p is tapped twice too slow', () => {
+ moveToPreviousChunkStub.returns(CursorMoveResult.CLIPPED);
+ isAtStartStub.returns(true);
+
+ nowStub.returns(5);
+ MockInteractions.pressAndReleaseKeyOn(element, 80, null, 'p');
+ nowStub.returns(6000);
+ MockInteractions.pressAndReleaseKeyOn(element, 80, null, 'p');
+
+ assert.isFalse(navToFileStub.called);
+ });
+
+ test('does not navigate when tapping n then p', () => {
+ moveToNextChunkStub.returns(CursorMoveResult.CLIPPED);
+ isAtEndStub.returns(true);
+
+ nowStub.returns(5);
+ MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
+
+ moveToPreviousChunkStub.returns(CursorMoveResult.CLIPPED);
+ isAtStartStub.returns(true);
+
+ nowStub.returns(10);
+ MockInteractions.pressAndReleaseKeyOn(element, 80, null, 'p');
+
+ assert.isFalse(navToFileStub.called);
+ });
+ });
+
test('shift+m navigates to next unreviewed file', () => {
element._files = getFilesFromFileList(['file1', 'file2', 'file3']);
element._reviewedFiles = new Set(['file1', 'file2']);
@@ -1894,7 +2023,7 @@
});
});
- suite('gr-diff-view tests unmodified files with comments', () => {
+ suite('unmodified files with comments', () => {
let element;
setup(() => {
const changedFiles = {
