From f09ff816d4bff68a23e0e4928e775d1323cb9e37 Mon Sep 17 00:00:00 2001 From: Dhruv Srivastava Date: Wed, 13 May 2020 18:40:11 +0200 Subject: [PATCH] Add link icon to messages in change log. Clicking the link icon will change the url. Users can copy the link by using the right click button of the browser. Screenshot: https://imgur.com/a/qnfOmQz Change-Id: I7b5cda5833835a296146b0f1add5255c044243ef --- .../change/gr-change-view/gr-change-view.js | 13 ++++-------- .../gr-change-view/gr-change-view_html.js | 2 -- .../gr-change-view/gr-change-view_test.js | 14 ------------- .../elements/change/gr-message/gr-message.js | 16 ++++++++------- .../change/gr-message/gr-message_html.js | 12 ++++++++++- .../change/gr-message/gr-message_test.html | 20 ------------------- .../core/gr-navigation/gr-navigation.js | 3 ++- .../app/elements/shared/gr-icons/gr-icons.js | 2 ++ 8 files changed, 28 insertions(+), 54 deletions(-) diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js index a7b49a7f55..b4b2d8b3a0 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js @@ -1020,7 +1020,10 @@ class GrChangeView extends mixinBehaviors( [ this.$.fileList.collapseAllDiffs(); } - _paramsChanged(value) { + _paramsChanged(value, oldValue) { + const paramsChanged = JSON.stringify(oldValue) !== JSON.stringify(value); + if (!paramsChanged) return; + if (value.view !== GerritNav.View.CHANGE) { this._initialLoadComplete = false; return; @@ -1138,14 +1141,6 @@ class GrChangeView extends mixinBehaviors( [ this.viewState.numFilesShown = numFilesShown; } - _handleMessageAnchorTap(e) { - const hash = MSG_PREFIX + e.detail.id; - const url = GerritNav.getUrlForChange(this._change, - this._patchRange.patchNum, this._patchRange.basePatchNum, - this._editMode, hash); - history.replaceState(null, '', url); - } - _maybeScrollToMessage(hash) { if (hash.startsWith(MSG_PREFIX)) { this.messagesList.scrollToMessage(hash.substr(MSG_PREFIX.length)); diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js index f5a04633cb..50b6d2d844 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js @@ -711,7 +711,6 @@ export const htmlTemplate = html` change-comments="[[_changeComments]]" project-name="[[_change.project]]" show-reply-buttons="[[_loggedIn]]" - on-message-anchor-tap="_handleMessageAnchorTap" on-reply="_handleMessageReply" > @@ -726,7 +725,6 @@ export const htmlTemplate = html` change-comments="[[_changeComments]]" project-name="[[_change.project]]" show-reply-buttons="[[_loggedIn]]" - on-message-anchor-tap="_handleMessageAnchorTap" on-reply="_handleMessageReply" > diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.js index 1362fe36d2..192ba8f047 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.js +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.js @@ -322,20 +322,6 @@ suite('gr-change-view tests', () => { const getCustomCssValue = cssParam => util.getComputedStyleValue(cssParam, element); - test('_handleMessageAnchorTap', () => { - element._changeNum = '1'; - element._patchRange = { - basePatchNum: 'PARENT', - patchNum: 1, - }; - const getUrlStub = sandbox.stub(GerritNav, '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('plugins adding to file tab', () => { setup(done => { // Resolving it here instead of during setup() as other tests depend diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.js b/polygerrit-ui/app/elements/change/gr-message/gr-message.js index 8c9d2de4f5..cc06beb98e 100644 --- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js +++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js @@ -35,6 +35,7 @@ import {SpecialFilePath} from '../../../constants/constants.js'; const PATCH_SET_PREFIX_PATTERN = /^Patch Set \d+:\s*(.*)/; const LABEL_TITLE_SCORE_PATTERN = /^(-?)([A-Za-z0-9-]+?)([+-]\d+)?$/; +const MSG_PREFIX = '#message-'; /** * @extends PolymerElement @@ -152,6 +153,10 @@ class GrMessage extends GestureEventListeners( value: false, }, _isCleanerLogExperimentEnabled: Boolean, + _changeMessageUrl: { + type: String, + computed: '_computeChangeMessageUrl(message)', + }, }; } @@ -407,13 +412,10 @@ class GrMessage extends GestureEventListeners( return classes.join(' '); } - _handleAnchorClick(e) { - e.preventDefault(); - this.dispatchEvent(new CustomEvent('message-anchor-tap', { - bubbles: true, - composed: true, - detail: {id: this.message.id}, - })); + _computeChangeMessageUrl(message) { + if (!message) return ''; + const hash = MSG_PREFIX + message.id; + return hash; } _handleReplyTap(e) { diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js index de4a72a407..baaf5a3a97 100644 --- a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js +++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js @@ -181,6 +181,9 @@ export const htmlTemplate = html` content: 'PS '; } } + .message-link { + text-decoration: none; + }
@@ -300,13 +303,20 @@ export const htmlTemplate = html` { }); }); - test('clicking on date link fires event', () => { - element.message = { - type: 'REVIEWER_UPDATE', - updated: '2016-01-12 20:24:49.448000000', - reviewer: {}, - id: '47c43261_55aa2c41', - expanded: false, - }; - flushAsynchronousOperations(); - const stub = sinon.stub(); - element.addEventListener('message-anchor-tap', stub); - const dateEl = element.shadowRoot - .querySelector('.date'); - assert.ok(dateEl); - MockInteractions.tap(dateEl); - - assert.isTrue(stub.called); - assert.deepEqual(stub.lastCall.args[0].detail, {id: element.message.id}); - }); - suite('compute messages', () => { test('empty', () => { assert.equal(element._computeMessageContent('', '', true), ''); diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.js b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.js index 69e2989522..8308942baa 100644 --- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.js +++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.js @@ -393,12 +393,13 @@ export const GerritNav = { * @param {number=} opt_patchNum * @return {string} */ - getUrlForChangeById(changeNum, project, opt_patchNum) { + getUrlForChangeById(changeNum, project, opt_patchNum, opt_messageHash) { return this._getUrlFor({ view: GerritNav.View.CHANGE, changeNum, project, patchNum: opt_patchNum, + messageHash: opt_messageHash, }); }, diff --git a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.js b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.js index 85caa61aaf..254fd25fd4 100644 --- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.js +++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.js @@ -102,6 +102,8 @@ $_documentContainer.innerHTML = ` + + `;