Prevent skipping chunks on first or last line of file
moveToStart() / moveToEnd() move the cursor to the first / last cursor
stop in the diff, which in Gerrit is the file comment line / last line
of the file, and for users of gr-diff that do not show file comment
buttons, the first line / last line of the file.
If the cursor is already on a chunk at the beginning or end of the file,
it should not unconditionally move to the next / previous chunk, as that
will mean skipping those chunks e.g. when navigating in large files with
the n and p shortcuts.
Change-Id: I45192812a3e56ef69ef0a3b2dafbf3b28573211d
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 52ac7cc..27d8c57 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
@@ -292,12 +292,16 @@
moveToFirstChunk() {
this.$.cursorManager.moveToStart();
- this.moveToNextChunk(true);
+ if (this.diffRow && !this._isFirstRowOfChunk(this.diffRow)) {
+ this.moveToNextChunk(true);
+ }
}
moveToLastChunk() {
this.$.cursorManager.moveToEnd();
- this.moveToPreviousChunk();
+ if (this.diffRow && !this._isFirstRowOfChunk(this.diffRow)) {
+ this.moveToPreviousChunk();
+ }
}
/**
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 3b366ce..d462f51 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
@@ -82,16 +82,101 @@
assert.equal(cursorElement.diffRow, firstDeltaRow);
});
- test('moveToLastChunk', () => {
+ test('moveToFirstChunk', async () => {
+ const diff = {
+ meta_a: {
+ name: 'lorem-ipsum.txt',
+ content_type: 'text/plain',
+ lines: 4,
+ },
+ meta_b: {
+ name: 'lorem-ipsum.txt',
+ content_type: 'text/plain',
+ lines: 4,
+ },
+ intraline_status: 'OK',
+ change_type: 'MODIFIED',
+ diff_header: [
+ 'diff --git a/lorem-ipsum.txt b/lorem-ipsum.txt',
+ 'index b2adcf4..554ae49 100644',
+ '--- a/lorem-ipsum.txt',
+ '+++ b/lorem-ipsum.txt',
+ ],
+ content: [
+ {a: ['old line 1'], b: ['new line 1']},
+ {ab: ['unchanged line 2']},
+ {a: ['old line 3'], b: ['new line 3']},
+ {ab: ['more unchanged lines']},
+ ],
+ };
+
+ diffElement.diff = diff;
+ // The file comment button, if present, is a cursor stop. Ensure
+ // moveToFirstChunk() works correctly even if the button is not shown.
+ diffElement.prefs.show_file_comment_button = false;
+ await flush();
+ cursorElement._updateStops();
+
const chunks = Array.from(diffElement.root.querySelectorAll(
'.section.delta'));
- assert.isAbove(chunks.length, 1);
+ assert.equal(chunks.length, 2);
+
+ // Verify it works on fresh diff.
+ cursorElement.moveToFirstChunk();
assert.equal(chunks.indexOf(cursorElement.diffRow.parentElement), 0);
+ // Verify it works from other cursor positions.
cursorElement.moveToLastChunk();
+ assert.notEqual(chunks.indexOf(cursorElement.diffRow.parentElement), 0);
+ cursorElement.moveToFirstChunk();
+ assert.equal(chunks.indexOf(cursorElement.diffRow.parentElement), 0);
+ });
- assert.equal(chunks.indexOf(cursorElement.diffRow.parentElement),
- chunks.length - 1);
+ test('moveToLastChunk', async () => {
+ const diff = {
+ meta_a: {
+ name: 'lorem-ipsum.txt',
+ content_type: 'text/plain',
+ lines: 4,
+ },
+ meta_b: {
+ name: 'lorem-ipsum.txt',
+ content_type: 'text/plain',
+ lines: 4,
+ },
+ intraline_status: 'OK',
+ change_type: 'MODIFIED',
+ diff_header: [
+ 'diff --git a/lorem-ipsum.txt b/lorem-ipsum.txt',
+ 'index b2adcf4..554ae49 100644',
+ '--- a/lorem-ipsum.txt',
+ '+++ b/lorem-ipsum.txt',
+ ],
+ content: [
+ {ab: ['unchanged line 1']},
+ {a: ['old line 2'], b: ['new line 2']},
+ {ab: ['more unchanged line 3']},
+ {a: ['old line 4'], b: ['new line 4']},
+ ],
+ };
+
+ diffElement.diff = diff;
+ await flush();
+ cursorElement._updateStops();
+
+ const chunks = Array.from(diffElement.root.querySelectorAll(
+ '.section.delta'));
+ assert.equal(chunks.length, 2);
+
+ // Verify it works on fresh diff.
+ cursorElement.moveToLastChunk();
+ assert.equal(chunks.indexOf(cursorElement.diffRow.parentElement), 1);
+
+ // Verify it works from other cursor positions.
+ cursorElement.moveToFirstChunk();
+ assert.notEqual(chunks.indexOf(cursorElement.diffRow.parentElement), 1);
+ cursorElement.moveToLastChunk();
+ assert.equal(chunks.indexOf(cursorElement.diffRow.parentElement), 1);
});
test('cursor scroll behavior', () => {
