opendev/gerrit
Code Issues Proposed changes

Merge "Handle message timestamp taps with Nav API"

Browse Source
This commit is contained in:
Logan Hanks 2018-10-31 22:33:18 +00:00 committed by Gerrit Code Review
commit f7f5c175b8
11 changed files with 77 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
View File

@ -587,6 +587,7 @@ limitations under the License.
change-comments="[[_changeComments]]"
project-name="[[_change.project]]"
show-reply-buttons="[[_loggedIn]]"
on-message-anchor-tap="_handleMessageAnchorTap"
on-reply="_handleMessageReply"></gr-messages-list>
</template>
<template is="dom-if" if="[[!_showMessagesView]]">

15
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
View File

@ -44,6 +44,8 @@
const TRAILING_WHITESPACE_REGEX = /[ \t]+$/gm;
const MSG_PREFIX = '#message-';
const ReloadToastMessage = {
NEWER_REVISION: 'A newer patch set has been uploaded',
RESTORED: 'This change has been restored',
@ -720,10 +722,17 @@
this.viewState.numFilesShown = numFilesShown;
},
_handleMessageAnchorTap(e) {
const hash = MSG_PREFIX + e.detail.id;
const url = Gerrit.Nav.getUrlForChange(this._change,
this._patchRange.patchNum, this._patchRange.basePatchNum,
this._editMode, hash);
history.replaceState(null, '', url);
},
_maybeScrollToMessage(hash) {
const msgPrefix = '#message-';
if (hash.startsWith(msgPrefix)) {
this.messagesList.scrollToMessage(hash.substr(msgPrefix.length));
if (hash.startsWith(MSG_PREFIX)) {
this.messagesList.scrollToMessage(hash.substr(MSG_PREFIX.length));
}
},

14
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
View File

@ -94,6 +94,20 @@ limitations under the License.
return element.getComputedStyleValue(cssParam);
};
test('_handleMessageAnchorTap', () => {
element._changeNum = '1';
element._patchRange = {
basePatchNum: 'PARENT',
patchNum: 1,
};
const getUrlStub = sandbox.stub(Gerrit.Nav, 'getUrlForChange');
const replaceStateStub = sandbox.stub(history, 'replaceState');
element._handleMessageAnchorTap({detail: {id: 'a12345'}});
assert.equal(getUrlStub.lastCall.args[4], '#message-a12345');
assert.isTrue(replaceStateStub.called);
});
suite('keyboard shortcuts', () => {
test('S should toggle the CL star', () => {
const starStub = sandbox.stub(element.$.changeStar, 'toggleStar');

9
polygerrit-ui/app/elements/change/gr-message/gr-message.html
View File

@ -132,9 +132,12 @@ limitations under the License.
right: var(--default-horizontal-margin);
top: 10px;
}
.date {
span.date {
color: var(--deemphasized-text-color);
}
span.date:hover {
text-decoration: underline;
}
.dateContainer iron-icon {
cursor: pointer;
}
@ -227,12 +230,12 @@ limitations under the License.
</span>
</template>
<template is="dom-if" if="[[message.id]]">
<a class="date" href$="[[_computeMessageHash(message)]]" on-tap="_handleLinkTap">
<span class="date" on-tap="_handleAnchorTap">
<gr-date-formatter
has-tooltip
show-date-and-time
date-str="[[message.date]]"></gr-date-formatter>
</a>
</span>
</template>
<iron-icon
id="expandToggle"

32
polygerrit-ui/app/elements/change/gr-message/gr-message.js
View File

@ -23,18 +23,18 @@
Polymer({
is: 'gr-message',
/**
* Fired when this message's permalink is tapped.
*
* @event scroll-to
*/
/**
* Fired when this message's reply link is tapped.
*
* @event reply
*/
/**
* Fired when the message's timestamp is tapped.
*
* @event message-anchor-tap
*/
listeners: {
tap: '_handleTap',
},
@ -223,22 +223,12 @@
return classes.join(' ');
},
_computeMessageHash(message) {
return '#message-' + message.id;
},
_handleLinkTap(e) {
_handleAnchorTap(e) {
e.preventDefault();
this.fire('scroll-to', {message: this.message}, {bubbles: false});
const hash = this._computeMessageHash(this.message);
// Don't add the hash to the window history if it's already there.
// Otherwise you mess up expected back button behavior.
if (window.location.hash == hash) { return; }
// Change the URL but don’t trigger a nav event. Otherwise it will
// reload the page.
page.show(window.location.pathname + hash, null, false);
this.dispatchEvent(new CustomEvent('message-anchor-tap', {
bubbles: true,
detail: {id: this.message.id},
}));
},
_handleReplyTap(e) {

18
polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
View File

@ -164,6 +164,24 @@ limitations under the License.
});
});
test('clicking on date link fires event', () => {
element.message = {
type: 'REVIEWER_UPDATE',
updated: '2016-01-12 20:24:49.448000000',
reviewer: {},
id: '47c43261_55aa2c41',
};
flushAsynchronousOperations();
const stub = sinon.stub();
element.addEventListener('message-anchor-tap', stub);
const dateEl = element.$$('.date');
assert.ok(dateEl);
MockInteractions.tap(dateEl);
assert.isTrue(stub.called);
assert.deepEqual(stub.lastCall.args[0].detail, {id: element.message.id});
});
test('votes', () => {
element.message = {
author: {},

2
polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
View File

@ -109,7 +109,7 @@ limitations under the License.
hide-automated="[[_hideAutomated]]"
project-name="[[projectName]]"
show-reply-button="[[showReplyButtons]]"
on-scroll-to="_handleScrollTo"
on-message-anchor-tap="_handleAnchorTap"
label-extremes="[[_labelExtremes]]"
data-message-id$="[[message.id]]"></gr-message>
</template>

4
polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
View File

@ -184,8 +184,8 @@
this.handleExpandCollapse(!this._expanded);
},
_handleScrollTo(e) {
this.scrollToMessage(e.detail.message.id);
_handleAnchorTap(e) {
this.scrollToMessage(e.detail.id);
},
_hasAutomatedMessages(messages) {

7
polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
View File

@ -33,6 +33,8 @@ limitations under the License.
// also be provided.
// - `edit`, optional, Boolean: whether or not to load the file list with
// edit controls.
// - `messageHash`, optional, String: the hash of the change message to
// scroll to.
//
// - Gerrit.Nav.View.SEARCH:
// - `query`, optional, String: the literal search query. If provided,
@ -352,9 +354,11 @@ limitations under the License.
* @param {number|string=} opt_basePatchNum The string 'PARENT' can be
* used for none.
* @param {boolean=} opt_isEdit
* @param {string=} opt_messageHash
* @return {string}
*/
getUrlForChange(change, opt_patchNum, opt_basePatchNum, opt_isEdit) {
getUrlForChange(change, opt_patchNum, opt_basePatchNum, opt_isEdit,
opt_messageHash) {
if (opt_basePatchNum === PARENT_PATCHNUM) {
opt_basePatchNum = undefined;
}
@ -368,6 +372,7 @@ limitations under the License.
basePatchNum: opt_basePatchNum,
edit: opt_isEdit,
host: change.internalHost || undefined,
messageHash: opt_messageHash,
});
},

3
polygerrit-ui/app/elements/core/gr-router/gr-router.js
View File

@ -395,6 +395,9 @@
} else if (params.edit) {
suffix += ',edit';
}
if (params.messageHash) {
suffix += params.messageHash;
}
if (params.project) {
const encodedProject = this.encodeURL(params.project, true);
return `/c/${encodedProject}/+/${params.changeNum}${suffix}`;

3
polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
View File

@ -281,6 +281,9 @@ limitations under the License.
paramsWithQuery.basePatchNum = 5;
assert.equal(element._generateUrl(paramsWithQuery),
'/c/test/+/1234/5..10?revert&foo=bar');
params.messageHash = '#123';
assert.equal(element._generateUrl(params), '/c/test/+/1234/5..10#123');
});
test('change with repo name encoding', () => {