Merge "Make navigating cursor on file list circular"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 9dcb67e..5206fdb 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -952,7 +952,7 @@
return;
}
e.preventDefault();
- this.fileCursor.next();
+ this.fileCursor.next({circular: true});
this.selectedIndex = this.fileCursor.index;
}
}
@@ -972,7 +972,7 @@
return;
}
e.preventDefault();
- this.fileCursor.previous();
+ this.fileCursor.previous({circular: true});
this.selectedIndex = this.fileCursor.index;
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
index 0655721..79bc9f6 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
@@ -533,7 +533,7 @@
assert.equal(element.fileCursor.index, 2);
// up should not move the cursor.
- MockInteractions.pressAndReleaseKeyOn(element, 38, null, 'down');
+ MockInteractions.pressAndReleaseKeyOn(element, 38, null, 'up');
assert.equal(element.fileCursor.index, 2);
MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
@@ -548,8 +548,8 @@
MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
- assert.equal(element.fileCursor.index, 0);
- assert.equal(element.selectedIndex, 0);
+ assert.equal(element.fileCursor.index, 1);
+ assert.equal(element.selectedIndex, 1);
const createCommentInPlaceStub = sinon.stub(element.diffCursor,
'createCommentInPlace');
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
index 017ba50..9dce127 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
@@ -87,7 +87,7 @@
}
/**
- * Move the cursor forward. Clipped to the ends of the stop list.
+ * Move the cursor forward. Clipped to the end of the stop list.
*
* @param options.filter Skips any stops for which filter returns false.
* @param options.getTargetHeight Optional function to calculate the
@@ -95,22 +95,36 @@
* sometimes different, used by the diff cursor.
* @param options.clipToTop When none of the next indices match, move
* back to first instead of to last.
+ * @param options.circular When on last element, you get to first element.
* @return If a move was performed or why not.
- * @private
*/
next(
options: {
filter?: (stop: HTMLElement) => boolean;
getTargetHeight?: (target: HTMLElement) => number;
clipToTop?: boolean;
+ circular?: boolean;
} = {}
): CursorMoveResult {
return this._moveCursor(1, options);
}
+ /**
+ * Move the cursor backward. Clipped to the beginning of stop list.
+ *
+ * @param options.filter Skips any stops for which filter returns false.
+ * @param options.getTargetHeight Optional function to calculate the
+ * height of the target's 'section'. The height of the target itself is
+ * sometimes different, used by the diff cursor.
+ * @param options.clipToTop When none of the next indices match, move
+ * back to first instead of to last.
+ * @param options.circular When on first element, you get to last element.
+ * @return If a move was performed or why not.
+ */
previous(
options: {
filter?: (stop: HTMLElement) => boolean;
+ circular?: boolean;
} = {}
): CursorMoveResult {
return this._moveCursor(-1, options);
@@ -276,34 +290,18 @@
}
}
- /**
- * Move the cursor forward or backward by delta. Clipped to the beginning or
- * end of stop list.
- *
- * @param delta either -1 or 1.
- * @param options.abort Will abort moving the cursor when encountering a
- * stop for which this condition is met. Will abort even if the stop
- * would have been filtered
- * @param options.filter Will keep going and skip any stops for which this
- * condition is not met.
- * @param options.getTargetHeight Optional function to calculate the
- * height of the target's 'section'. The height of the target itself is
- * sometimes different, used by the diff cursor.
- * @param options.clipToTop When none of the next indices match, move
- * back to first instead of to last.
- * @return If a move was performed or why not.
- * @private
- */
_moveCursor(
delta: number,
{
filter,
getTargetHeight,
clipToTop,
+ circular,
}: {
filter?: (stop: HTMLElement) => boolean;
getTargetHeight?: (target: HTMLElement) => number;
clipToTop?: boolean;
+ circular?: boolean;
} = {}
): CursorMoveResult {
if (!this.stops.length) {
@@ -326,7 +324,10 @@
(delta > 0 && newIndex >= this.stops.length) ||
(delta < 0 && newIndex < 0)
) {
- newIndex = delta < 0 || clipToTop ? 0 : this.stops.length - 1;
+ newIndex =
+ (delta < 0 && !circular) || (delta > 0 && circular) || clipToTop
+ ? 0
+ : this.stops.length - 1;
newStop = this.stops[newIndex];
clipped = true;
break;
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js
index ba7e4f8..d0bd420 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js
@@ -255,6 +255,25 @@
assert.isTrue(cursor.target.focus.called);
});
+ suite('circular options', () => {
+ const options = {circular: true};
+ setup(() => {
+ cursor.stops = [...list.querySelectorAll('li')];
+ });
+
+ test('previous() on first element goes to last element', () => {
+ cursor.setCursor(list.children[0]);
+ cursor.previous(options);
+ assert.equal(cursor.index, list.children.length - 1);
+ });
+
+ test('next() on last element goes to first element', () => {
+ cursor.setCursor(list.children[list.children.length - 1]);
+ cursor.next(options);
+ assert.equal(cursor.index, 0);
+ });
+ });
+
suite('_scrollToTarget', () => {
let scrollStub;
setup(() => {
