Fix next/prev page event handlers on change list view
There was a logic flaw that prevented "]" or "n" from working to
navigate to the next page with an offset greater than the items per
page, resulting in the keyboard shortcuts only working the first time.
This change fixes the issue and also adds some test coverage to this
element.
Bug: Issue 5537
Change-Id: I8893a8517eeb05c5228258b8efa3090bc076057e
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.html b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.html
index 91b2f07..9c9a31a 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.html
@@ -57,10 +57,12 @@
selected-index="{{viewState.selectedChangeIndex}}"
show-star="[[loggedIn]]"></gr-change-list>
<nav>
- <a href$="[[_computeNavLink(_query, _offset, -1, _changesPerPage)]]"
- hidden$="[[_hidePrevArrow(_offset)]]" hidden>← Prev</a>
- <a href$="[[_computeNavLink(_query, _offset, 1, _changesPerPage)]]"
- hidden$="[[_hideNextArrow(_loading, _changesPerPage)]]" hidden>
+ <a id="prevArrow"
+ href$="[[_computeNavLink(_query, _offset, -1, _changesPerPage)]]"
+ hidden$="[[_hidePrevArrow(_offset)]]" hidden>← Prev</a>
+ <a id="nextArrow"
+ href$="[[_computeNavLink(_query, _offset, 1, _changesPerPage)]]"
+ hidden$="[[_hideNextArrow(_loading, _changesPerPage)]]" hidden>
Next →</a>
</nav>
</div>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js
index 4831000..fa68bc8 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js
@@ -139,13 +139,13 @@
},
_handleNextPage: function() {
- if (this._hideNextArrow(this._offset)) { return; }
+ if (this.$.nextArrow.hidden) { return; }
page.show(this._computeNavLink(
this._query, this._offset, 1, this._changesPerPage));
},
_handlePreviousPage: function() {
- if (this._hidePrevArrow(this._offset)) { return; }
+ if (this.$.prevArrow.hidden) { return; }
page.show(this._computeNavLink(
this._query, this._offset, -1, this._changesPerPage));
},
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.html
index 944e963..376c6ea 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.html
@@ -18,6 +18,7 @@
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
<title>gr-change-list-view</title>
+<script src="../../../bower_components/page/page.js"></script>
<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
@@ -32,12 +33,18 @@
<script>
suite('gr-change-list-view tests', function() {
var element;
+ var sandbox;
setup(function() {
stub('gr-rest-api-interface', {
getLoggedIn: function() { return Promise.resolve(false); },
});
element = fixture('basic');
+ sandbox = sinon.sandbox.create();
+ });
+
+ teardown(function() {
+ sandbox.restore();
});
test('url is properly encoded', function() {
@@ -50,5 +57,65 @@
'/q/status:open+project:platform%252Fframeworks%252Fbase,25'
);
});
+
+ test('_computeNavLink', function() {
+ var query = 'status:open';
+ var offset = 0;
+ var direction = 1;
+ var changesPerPage = 5;
+ assert.equal(
+ element._computeNavLink(query, offset, direction, changesPerPage),
+ '/q/status:open,5');
+ direction = -1;
+ assert.equal(
+ element._computeNavLink(query, offset, direction, changesPerPage),
+ '/q/status:open');
+ offset = 5;
+ direction = 1;
+ assert.equal(
+ element._computeNavLink(query, offset, direction, changesPerPage),
+ '/q/status:open,10');
+ });
+
+ test('_hidePrevArrow', function() {
+ var offset = 0;
+ assert.isTrue(element._hidePrevArrow(offset));
+ offset = 5;
+ assert.isFalse(element._hidePrevArrow(offset));
+ });
+
+ test('_hideNextArrow', function() {
+ var loading = true;
+ var changesPerPage = 25;
+ assert.isTrue(element._hideNextArrow(loading, changesPerPage));
+ loading = false;
+ assert.isTrue(element._hideNextArrow(loading, changesPerPage));
+ element._changes =
+ Array.apply(null, Array(5)).map(Number.prototype.valueOf, 0);
+ assert.isTrue(element._hideNextArrow(loading, changesPerPage));
+ element._changes =
+ Array.apply(null, Array(25)).map(Number.prototype.valueOf, 0);
+ assert.isFalse(element._hideNextArrow(loading, changesPerPage));
+ });
+
+ test('_handleNextPage', function() {
+ var showStub = sandbox.stub(page, 'show');
+ element.$.nextArrow.hidden = true;
+ element._handleNextPage();
+ assert.isFalse(showStub.called);
+ element.$.nextArrow.hidden = false;
+ element._handleNextPage();
+ assert.isTrue(showStub.called);
+ });
+
+ test('_handlePreviousPage', function() {
+ var showStub = sandbox.stub(page, 'show');
+ element.$.prevArrow.hidden = true;
+ element._handlePreviousPage();
+ assert.isFalse(showStub.called);
+ element.$.prevArrow.hidden = false;
+ element._handlePreviousPage();
+ assert.isTrue(showStub.called);
+ });
});
</script>
