Make isAtStart/End() return true when no stops
Historically it would return true for "at end" and false for "at start".
Change 282278 made this consistently return false, because at the time
empty stops could have indicated that the diff was still loading. This
is no longer the case, as it is now handled with an explicit AbortStop.
On the contrary, when gr-diff is configured to not render the file line,
fully loaded diffs empty, binary or image diffs would have empty stop,
and it makes more sense in this case to consider the cursor "at end"
e.g. for the purpose of showing an "tap x again to advance to next
file" toast.
Change-Id: I4c88e4ec47a1619e14081ba424d8056b754de125
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 352bc58..2fbbd7c 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
@@ -261,14 +261,14 @@
this._targetHeight = null;
}
- isAtStart() {
- return this.index === 0;
+ /** Returns true if there are no stops, or we are on the first stop. */
+ isAtStart(): boolean {
+ return this.stops.length === 0 || this.index === 0;
}
- isAtEnd() {
- // Unset cursor should not be considered "at end", even when there are no
- // cursor stops.
- return this.index !== -1 && this.index === this.stops.length - 1;
+ /** Returns true if there are no stops, or we are on the last stop. */
+ isAtEnd(): boolean {
+ return this.stops.length === 0 || this.index === this.stops.length - 1;
}
moveToStart() {
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 33aeafc..6f74d6b 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
@@ -117,6 +117,16 @@
assert.equal(element.index, -1);
});
+ test('isAtStart() returns true when there are no stops', () => {
+ element.stops = [];
+ assert.isTrue(element.isAtStart());
+ });
+
+ test('isAtEnd() returns true when there are no stops', () => {
+ element.stops = [];
+ assert.isTrue(element.isAtEnd());
+ });
+
test('next() goes to first element when no cursor is set', () => {
element.stops = [...list.querySelectorAll('li')];
const result = element.next();
@@ -137,8 +147,6 @@
assert.equal(element.index, -1);
assert.isNotOk(element.target);
assert.isFalse(list.children[1].classList.contains('targeted'));
- assert.isFalse(element.isAtStart());
- assert.isFalse(element.isAtEnd());
});
test('previous() goes to last element when no cursor is set', () => {
@@ -162,8 +170,6 @@
assert.equal(element.index, -1);
assert.isNotOk(element.target);
assert.isFalse(list.children[1].classList.contains('targeted'));
- assert.isFalse(element.isAtStart());
- assert.isFalse(element.isAtEnd());
});
test('_moveCursor', () => {
