Merge "Wrap isAtStart/End() in GrDiffCursor"
diff --git a/polygerrit-ui/FE_Style_Guide.md b/polygerrit-ui/FE_Style_Guide.md
index c9a5d9b..a636119 100644
--- a/polygerrit-ui/FE_Style_Guide.md
+++ b/polygerrit-ui/FE_Style_Guide.md
@@ -10,11 +10,20 @@
Additionally to the rules above, Gerrit frontend uses the following rules (some of them have automated checks,
some don't):
+- [Prefer null over undefined](#prefer-null)
- [Use destructuring imports only](#destructuring-imports-only)
- [Use classes and services for storing and manipulating global state](#services-for-global-state)
- [Pass required services in the constructor for plain classes](#pass-dependencies-in-constructor)
- [Assign required services in a HTML/Polymer element constructor](#assign-dependencies-in-html-element-constructor)
+## <a name="prefer-undefined"></a>Prefer `undefined` over `null`
+
+It is more confusing than helpful to work with both `null` and `undefined`. We prefer to only use `undefined` in
+our code base. Try to avoid `null`.
+
+Some browser and library APIs are using `null`, so we cannot remove `null` completely from our code base. But even
+then try to convert return values and leak as few `nulls` as possible.
+
## <a name="destructuring-imports-only"></a>Use destructuring imports only
Always use destructuring import statement and specify all required names explicitly (e.g. `import {a,b,c} from '...'`)
where possible.
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', () => {
