From 230e5fdaed79941eba06c119ba82cd29c728b5e5 Mon Sep 17 00:00:00 2001 From: Wyatt Allen Date: Mon, 11 Dec 2017 18:40:50 -0800 Subject: [PATCH] Setup gr-change-list to use gr-cursor-manager Cursing in the change list predates the standard cursor manager and thus handled cursing manually. When the [enter] key handler was reworked to defer to the browser's native focus (issue 7294) the enter key behavior stopped working in change lists because the focus was not being set by the cursor. With this change, change lists use the cursor manager that is configured to set focus, allowing the [enter] key to safely work as before without interfering with issue 7294. Section query link URL generation is upgraded to use Gerrit.Nav. Bug: Issue 7493 Change-Id: Ia70cf75a555b6aee54e2740fa653c1568eba1525 --- .../gr-change-list/gr-change-list.html | 7 +++++++ .../gr-change-list/gr-change-list.js | 21 ++++++++++++------- .../gr-change-list/gr-change-list_test.html | 10 --------- .../core/gr-navigation/gr-navigation.html | 10 +++++++++ .../app/elements/core/gr-router/gr-router.js | 4 ++++ .../core/gr-router/gr-router_test.html | 4 ++++ 6 files changed, 39 insertions(+), 17 deletions(-) diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html index 152ef3d2f4..27d3a429a7 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html +++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html @@ -22,6 +22,7 @@ limitations under the License. + @@ -101,11 +102,17 @@ limitations under the License. visible-change-table-columns="[[visibleChangeTableColumns]]" show-number="[[showNumber]]" show-star="[[showStar]]" + tabindex="0" label-names="[[labelNames]]"> + diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js index 262d1dc0ec..f66d8c8a89 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js +++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js @@ -113,6 +113,10 @@ keydown: '_scopedKeydownHandler', }, + observers: [ + '_sectionsChanged(sections.*)', + ], + /** * Iron-a11y-keys-behavior catches keyboard events globally. Some keyboard * events must be scoped to a component level (e.g. `enter`) in order to not @@ -194,7 +198,7 @@ }, _sectionHref(query) { - return `${this.getBaseUrl()}/q/${this.encodeURL(query, true)}`; + return Gerrit.Nav.getUrlForSearchQuery(query); }, /** @@ -234,10 +238,7 @@ this.modifierPressed(e)) { return; } e.preventDefault(); - // Compute absolute index of item that would come after final item. - const len = this._computeItemAbsoluteIndex(this.sections.length, 0); - if (this.selectedIndex === len - 1) { return; } - this.selectedIndex += 1; + this.$.cursor.next(); }, _handleKKey(e) { @@ -245,8 +246,7 @@ this.modifierPressed(e)) { return; } e.preventDefault(); - if (this.selectedIndex === 0) { return; } - this.selectedIndex -= 1; + this.$.cursor.previous(); }, _handleOKey(e) { @@ -317,5 +317,12 @@ _getListItems() { return Polymer.dom(this.root).querySelectorAll('gr-change-list-item'); }, + + _sectionsChanged() { + // Flush DOM operations so that the list item elements will be loaded. + Polymer.dom.flush(); + this.$.cursor.stops = this._getListItems(); + this.$.cursor.moveToStart(); + }, }); })(); diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html index bded5f6242..7b9fadf31c 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html +++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html @@ -473,16 +473,6 @@ limitations under the License. } }); - test('_sectionHref', () => { - assert.equal( - element._sectionHref('is:open owner:self'), - '/q/is:open+owner:self'); - assert.equal( - element._sectionHref( - 'is:open ((reviewer:self -is:ignored) OR assignee:self)'), - '/q/is:open+((reviewer:self+-is:ignored)+OR+assignee:self)'); - }); - test('_computeItemAbsoluteIndex', () => { sandbox.stub(element, '_computeLabelNames'); element.sections = [ diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html index f30cc1ca4d..b59ad9dc9b 100644 --- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html +++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html @@ -26,6 +26,9 @@ limitations under the License. // - `changeNum`, required, String: the numeric ID of the change. // // - Gerrit.Nav.View.SEARCH: + // - `query`, optional, String: the literal search query. If provided, + // the string will be used as the query, and all other params will be + // ignored. // - `owner`, optional, String: the owner name. // - `project`, optional, String: the project name. // - `branch`, optional, String: the branch name. @@ -136,6 +139,13 @@ limitations under the License. return this._generateUrl(params); }, + getUrlForSearchQuery(query) { + return this._getUrlFor({ + view: Gerrit.Nav.View.SEARCH, + query, + }); + }, + /** * @param {!string} project The name of the project. * @param {boolean=} opt_openOnly When true, only search open changes in diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js index 60fd109ab9..5d26cb63cf 100644 --- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js +++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js @@ -322,6 +322,10 @@ * @return {string} */ _generateSearchUrl(params) { + if (params.query) { + return '/q/' + this.encodeURL(params.query, true); + } + const operators = []; if (params.owner) { operators.push('owner:' + this.encodeURL(params.owner, false)); diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html index 909235b840..7159e0aebc 100644 --- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html +++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html @@ -226,6 +226,10 @@ limitations under the License. '/q/owner:a%2525b+project:c%2525d+branch:e%2525f+' + 'topic:"g%2525h"+status:op%2525en'); + // The presence of the query param overrides other params. + params.query = 'foo$bar'; + assert.equal(element._generateUrl(params), '/q/foo%2524bar'); + params = { view: Gerrit.Nav.View.SEARCH, statuses: ['a', 'b', 'c'],