opendev/gerrit
Code Issues Proposed changes

Merge "Expose whether moving the cursor was successful"

Browse Source
This commit is contained in:
Ole Rehmsen
2020-09-25 08:38:29 +00:00
committed by Gerrit Code Review
parent 921b948d9f 38f8066d03
commit 64f8c794ff
2 changed files with 70 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.ts
View File

@@ -34,6 +34,18 @@ declare global {
// Time in which pressing n key again after the toast navigates to next file
const NAVIGATE_TO_NEXT_FILE_TIMEOUT_MS = 5000;
/**
* Return type for cursor moves, that indicate whether a move was possible.
*/
export enum CursorMoveResult {
/** The cursor was successfully moved. */
MOVED,
/** There were no stops - the cursor was reset. */
NO_STOPS,
/** There was no more stop to move to - the cursor was clipped to the end. */
CLIPPED,
}
@customElement('gr-cursor-manager')
export class GrCursorManager extends GestureEventListeners(
LegacyElementMixin(PolymerElement)
@@ -104,16 +116,16 @@ export class GrCursorManager extends GestureEventListeners(
* back to first instead of to last.
* @param navigateToNextFile Navigate to next unreviewed file
* if user presses next on the last diff chunk
* @return If a move was performed or why not.
* @private
*/
next(
condition?: Function,
getTargetHeight?: (target: HTMLElement) => number,
clipToTop?: boolean,
navigateToNextFile?: boolean
) {
this._moveCursor(
): CursorMoveResult {
return this._moveCursor(
1,
condition,
getTargetHeight,
@@ -122,8 +134,8 @@ export class GrCursorManager extends GestureEventListeners(
);
}
previous(condition?: Function) {
this._moveCursor(-1, condition);
previous(condition?: Function): CursorMoveResult {
return this._moveCursor(-1, condition);
}
/**
@@ -269,6 +281,7 @@ export class GrCursorManager extends GestureEventListeners(
* back to first instead of to last.
* @param navigateToNextFile Navigate to next unreviewed file
* if user presses next on the last diff chunk
* @return If a move was performed or why not.
* @private
*/
_moveCursor(
@@ -277,27 +290,25 @@ export class GrCursorManager extends GestureEventListeners(
getTargetHeight?: (target: HTMLElement) => number,
clipToTop?: boolean,
navigateToNextFile?: boolean
) {
): CursorMoveResult {
if (!this.stops.length) {
this.unsetCursor();
return;
return CursorMoveResult.NO_STOPS;
}
this._unDecorateTarget();
const newIndex = this._getNextindex(delta, condition, clipToTop);
const newTarget = newIndex !== -1 ? this.stops[newIndex] : null;
let newTarget = null;
if (newIndex !== -1) {
newTarget = this.stops[newIndex];
}
const clipped = this.index === newIndex;
/*
* If user presses n on the last diff chunk, show a toast informing user
* that pressing n again will navigate them to next unreviewed file.
* If click happens within the time limit, then navigate to next file
*/
if (navigateToNextFile && this.index === newIndex && this.isAtEnd()) {
if (navigateToNextFile && clipped && this.isAtEnd()) {
if (
this._lastDisplayedNavigateToNextFileToast &&
Date.now() - this._lastDisplayedNavigateToNextFileToast <=
@@ -311,7 +322,7 @@ export class GrCursorManager extends GestureEventListeners(
bubbles: true,
})
);
return;
return CursorMoveResult.CLIPPED;
}
this._lastDisplayedNavigateToNextFileToast = Date.now();
this.dispatchEvent(
@@ -323,14 +334,14 @@ export class GrCursorManager extends GestureEventListeners(
bubbles: true,
})
);
return;
return CursorMoveResult.CLIPPED;
}
this.index = newIndex;
this.target = newTarget as HTMLElement;
if (!newTarget) {
return;
return CursorMoveResult.NO_STOPS;
}
if (getTargetHeight) {
@@ -344,6 +355,8 @@ export class GrCursorManager extends GestureEventListeners(
}
this._decorateTarget();
return clipped ? CursorMoveResult.CLIPPED : CursorMoveResult.MOVED;
}
_decorateTarget() {

52
polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.js
View File

@@ -18,6 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-cursor-manager.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
import {CursorMoveResult} from './gr-cursor-manager.js';
const basicTestFixutre = fixtureFromTemplate(html`
<gr-cursor-manager cursor-target-class="targeted"></gr-cursor-manager>
@@ -66,10 +67,11 @@ suite('gr-cursor-manager tests', () => {
assert.isFalse(element.isAtEnd());
// Progress the cursor.
element.next();
let result = element.next();
// Confirm that the next stop is selected and that the previous stop is
// unselected.
assert.equal(result, CursorMoveResult.MOVED);
assert.equal(element.index, 3);
assert.equal(element.target, list.children[3]);
assert.isTrue(element.isAtEnd());
@@ -77,19 +79,23 @@ suite('gr-cursor-manager tests', () => {
assert.isTrue(list.children[3].classList.contains('targeted'));
// Progress the cursor.
element.next();
result = element.next();
// We should still be at the end.
assert.equal(result, CursorMoveResult.CLIPPED);
assert.equal(element.index, 3);
assert.equal(element.target, list.children[3]);
assert.isTrue(element.isAtEnd());
// Wind the cursor all the way back to the first stop.
element.previous();
element.previous();
element.previous();
result = element.previous();
assert.equal(result, CursorMoveResult.MOVED);
result = element.previous();
assert.equal(result, CursorMoveResult.MOVED);
result = element.previous();
assert.equal(result, CursorMoveResult.MOVED);
// The element state should reflect the end of the list.
// The element state should reflect the start of the list.
assert.equal(element.index, 0);
assert.equal(element.target, list.children[0]);
assert.isTrue(element.isAtStart());
@@ -113,8 +119,9 @@ suite('gr-cursor-manager tests', () => {
test('next() goes to first element when no cursor is set', () => {
element.stops = list.querySelectorAll('li');
element.next();
const result = element.next();
assert.equal(result, CursorMoveResult.MOVED);
assert.equal(element.index, 0);
assert.equal(element.target, list.children[0]);
assert.isTrue(list.children[0].classList.contains('targeted'));
@@ -122,10 +129,23 @@ suite('gr-cursor-manager tests', () => {
assert.isFalse(element.isAtEnd());
});
test('next() goes to first element when no cursor is set', () => {
element.stops = list.querySelectorAll('li');
element.previous();
test('next() resets the cursor when there are no stops', () => {
element.stops = [];
const result = element.next();
assert.equal(result, CursorMoveResult.NO_STOPS);
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', () => {
element.stops = list.querySelectorAll('li');
const result = element.previous();
assert.equal(result, CursorMoveResult.MOVED);
const lastIndex = list.children.length - 1;
assert.equal(element.index, lastIndex);
assert.equal(element.target, list.children[lastIndex]);
@@ -134,6 +154,18 @@ suite('gr-cursor-manager tests', () => {
assert.isTrue(element.isAtEnd());
});
test('previous() resets the cursor when there are no stops', () => {
element.stops = [];
const result = element.previous();
assert.equal(result, CursorMoveResult.NO_STOPS);
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', () => {
// Initialize the cursor with its stops.
element.stops = list.querySelectorAll('li');