opendev/gerrit
Code Issues Proposed changes

Merge "Support deep linking on tabs"

Browse Source
This commit is contained in:
Tao Zhou
2020-04-14 08:04:15 +00:00
committed by Gerrit Code Review
parent 7d8844564e 4fd32d5e7c
commit a67f79d9fd
11 changed files with 402 additions and 164 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
polygerrit-ui/app/constants/README.md Normal file
View File

@@ -0,0 +1,7 @@
`constants` folder should contain:
1. constants used across files
2. messages used across files, like toasters, notifications etc
For every constant defined, please add a `@desc` for it, once the list grows bigger,
we should consider grouping them with sub folders / files.

35
polygerrit-ui/app/constants/constants.js Normal file
View File

@@ -0,0 +1,35 @@
/**
* @license
* Copyright (C) 2020 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* @enum
* @desc Tab names for primary tabs on change view page.
*/
export const PrimaryTabs = {
FILES: '_files',
FINDINGS: '_findings',
};
/**
* @enum
* @desc Tab names for secondary tabs on change view page.
*/
export const SecondaryTabs = {
CHANGE_LOG: '_changeLog',
COMMENT_THREADS: '_commentThreads',
};

25
polygerrit-ui/app/constants/messages.js Normal file
View File

@@ -0,0 +1,25 @@
/**
* @license
* Copyright (C) 2020 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/** @desc Default message shown when no threads in gr-thread-list */
export const NO_THREADS_MSG =
'There are no inline comment threads on any diff for this change.';
/** @desc Message shown when no threads in gr-thread-list for robot comments */
export const NO_ROBOT_COMMENTS_THREADS_MSG =
'There are no findings for this patchset.';

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

@@ -56,13 +56,15 @@ import '../gr-reply-dialog/gr-reply-dialog.js';
import '../gr-thread-list/gr-thread-list.js';
import '../gr-upload-help-dialog/gr-upload-help-dialog.js';
import {flush} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {beforeNextRender} from '@polymer/polymer/lib/utils/render-status.js';
import {mixinBehaviors} from '@polymer/polymer/lib/legacy/class.js';
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
import {htmlTemplate} from './gr-change-view_html.js';
import {PrimaryTabs, SecondaryTabs} from '../../../constants/constants.js';
import {NO_ROBOT_COMMENTS_THREADS_MSG} from '../../../constants/messages.js';
const CHANGE_ID_ERROR = {
MISMATCH: 'mismatch',
MISSING: 'missing',
@@ -105,20 +107,22 @@ const DiffViewMode = {
UNIFIED: 'UNIFIED_DIFF',
};
const CommentTabs = {
CHANGE_LOG: 0,
COMMENT_THREADS: 1,
ROBOT_COMMENTS: 2,
};
const CHANGE_DATA_TIMING_LABEL = 'ChangeDataLoaded';
const CHANGE_RELOAD_TIMING_LABEL = 'ChangeReloaded';
const SEND_REPLY_TIMING_LABEL = 'SendReply';
// Making the tab names more unique in case a plugin adds one with same name
const FILES_TAB_NAME = '__gerrit_internal_files';
const FINDINGS_TAB_NAME = '__gerrit_internal_findings';
const ROBOT_COMMENTS_LIMIT = 10;
// types used in this file
/**
* Type for the custom event to switch tab.
*
* @typedef {Object} SwitchTabEventDetail
* @property {?string} tab - name of the tab to set as active, from custom event
* @property {?boolean} scrollIntoView - scroll into the tab afterwards, from custom event
* @property {?number} value - index of tab to set as active, from paper-tabs event
*/
/**
* @appliesMixin Gerrit.FireMixin
* @appliesMixin Gerrit.KeyboardShortcutMixin
@@ -263,9 +267,18 @@ class GrChangeView extends mixinBehaviors( [
type: String,
value: '',
},
_commentTabs: {
_constants: {
type: Object,
value: CommentTabs,
value: {
SecondaryTabs,
PrimaryTabs,
},
},
_messages: {
type: Object,
value: {
NO_ROBOT_COMMENTS_THREADS_MSG,
},
},
_lineHeight: Number,
_changeIdCommitMessageError: {
@@ -356,10 +369,6 @@ class GrChangeView extends mixinBehaviors( [
type: Boolean,
value: undefined,
},
_currentView: {
type: Number,
value: CommentTabs.CHANGE_LOG,
},
_showFileTabContent: {
type: Boolean,
value: true,
@@ -389,17 +398,14 @@ class GrChangeView extends mixinBehaviors( [
_currentRobotCommentsPatchSet: {
type: Number,
},
_files_tab_name: {
type: String,
value: FILES_TAB_NAME,
},
_findings_tab_name: {
type: String,
value: FINDINGS_TAB_NAME,
},
_currentTabName: {
type: String,
value: FILES_TAB_NAME,
/**
* @type {Array<string>} this is a two-element tuple to always
* hold the current active tab for both primary and secondary tabs
*/
_activeTabs: {
type: Array,
value: [PrimaryTabs.FILES, SecondaryTabs.CHANGE_LOG],
},
_showAllRobotComments: {
type: Boolean,
@@ -489,7 +495,7 @@ class GrChangeView extends mixinBehaviors( [
console.warn('Different number of tab headers and tab content.');
}
})
.then(() => this._setPrimaryTab());
.then(() => this._initActiveTabs(this.params));
this.addEventListener('comment-save', this._handleCommentSave.bind(this));
this.addEventListener('comment-refresh', this._reloadDrafts.bind(this));
@@ -507,6 +513,11 @@ class GrChangeView extends mixinBehaviors( [
this._onCloseFixPreview.bind(this));
this.listen(window, 'scroll', '_handleScroll');
this.listen(document, 'visibilitychange', '_handleVisibilityChange');
this.addEventListener('show-primary-tab',
e => this._setActivePrimaryTab(e));
this.addEventListener('show-secondary-tab',
e => this._setActiveSecondaryTab(e));
}
/** @override */
@@ -567,30 +578,64 @@ class GrChangeView extends mixinBehaviors( [
}
}
_handleCommentTabChange() {
this._currentView = this.$.commentTabs.selected;
const type = Object.keys(CommentTabs).find(key => CommentTabs[key] ===
this._currentView);
this.$.reporting.reportInteraction('comment-tab-changed', {tabName:
type});
_isTabActive(tab, activeTabs) {
return activeTabs.includes(tab);
}
_isSelectedView(currentView, view) {
return currentView === view;
/**
* Actual implementation of switching a tab
*
* @param {!HTMLElement} paperTabs - the parent tabs container
* @param {!SwitchTabEventDetail} activeDetails
*/
_setActiveTab(paperTabs, activeDetails) {
const {activeTabName, activeTabIndex, scrollIntoView} = activeDetails;
const tabs = paperTabs.querySelectorAll('paper-tab');
let activeIndex = -1;
if (activeTabIndex !== undefined) {
activeIndex = activeTabIndex;
} else {
for (let i = 0; i <= tabs.length; i++) {
const tab = tabs[i];
if (tab.dataset.name === activeTabName) {
activeIndex = i;
break;
}
}
}
if (activeIndex === -1) {
console.warn('tab not found with given info', activeTab);
return;
}
const tabName = tabs[activeIndex].dataset.name;
if (scrollIntoView) {
paperTabs.scrollIntoView();
}
if (paperTabs.selected !== activeIndex) {
paperTabs.selected = activeIndex;
this.$.reporting.reportInteraction('show-tab', {tabName});
}
return tabName;
}
_findIfTabMatches(currentTab, tab) {
return currentTab === tab;
}
/**
* Changes active primary tab.
*
* @param {CustomEvent<SwitchTabEventDetail>} e
*/
_setActivePrimaryTab(e) {
const primaryTabs = this.shadowRoot.querySelector('#primaryTabs');
const activeTabName = this._setActiveTab(primaryTabs, {
activeTabName: e.detail.tab,
activeTabIndex: e.detail.value,
scrollIntoView: e.detail.scrollIntoView,
});
if (activeTabName) {
this._activeTabs = [activeTabName, this._activeTabs[1]];
_handleFileTabChange(e) {
const selectedIndex = e.target.selected;
const tabs = e.target.querySelectorAll('paper-tab');
this._currentTabName = tabs[selectedIndex] &&
tabs[selectedIndex].dataset.name;
const source = e && e.type ? e.type : '';
// update plugin endpoint if its a plugin tab
const pluginIndex = (this._dynamicTabHeaderEndpoints || []).indexOf(
this._currentTabName);
activeTabName);
if (pluginIndex !== -1) {
this._selectedTabPluginEndpoint = this._dynamicTabContentEndpoints[
pluginIndex];
@@ -600,27 +645,27 @@ class GrChangeView extends mixinBehaviors( [
this._selectedTabPluginEndpoint = '';
this._selectedTabPluginHeader = '';
}
this.$.reporting.reportInteraction('tab-changed',
{tabName: this._currentTabName, source});
}
}
_handleShowTab(e) {
const primaryTabs = this.shadowRoot.querySelector('#primaryTabs');
const tabs = primaryTabs.querySelectorAll('paper-tab');
let idx = -1;
tabs.forEach((tab, index) => {
if (tab.dataset.name === e.detail.tab) idx = index;
/**
* Changes active secondary tab.
*
* @param {CustomEvent<SwitchTabEventDetail>} e
*/
_setActiveSecondaryTab(e) {
const secondaryTabs = this.shadowRoot.querySelector('#secondaryTabs');
const activeTabName = this._setActiveTab(secondaryTabs, {
activeTabName: e.detail.tab,
activeTabIndex: e.detail.value,
scrollIntoView: e.detail.scrollIntoView,
});
if (idx === -1) {
console.error(e.detail.tab + ' tab not found');
return;
if (activeTabName) {
this._activeTabs = [this._activeTabs[0], activeTabName];
}
primaryTabs.selected = idx;
primaryTabs.scrollIntoView();
this.$.reporting.reportInteraction('show-tab', {tabName: e.detail.tab});
}
_handleEditCommitMessage(e) {
_handleEditCommitMessage() {
this._editingCommitMessage = true;
this.$.commitMessageEditor.focusTextarea();
}
@@ -633,7 +678,8 @@ class GrChangeView extends mixinBehaviors( [
this.$.commitMessageEditor.disabled = true;
this.$.restAPI.putChangeCommitMessage(
this._changeNum, message).then(resp => {
this._changeNum, message)
.then(resp => {
this.$.commitMessageEditor.disabled = false;
if (!resp.ok) { return; }
@@ -963,8 +1009,6 @@ class GrChangeView extends mixinBehaviors( [
}
_paramsChanged(value) {
this._currentView = CommentTabs.CHANGE_LOG;
this._setPrimaryTab();
if (value.view !== Gerrit.Nav.View.CHANGE) {
this._initialLoadComplete = false;
return;
@@ -1009,6 +1053,34 @@ class GrChangeView extends mixinBehaviors( [
this._reload(true).then(() => {
this._performPostLoadTasks();
});
Gerrit.awaitPluginsLoaded().then(() => {
this._initActiveTabs(value);
});
}
_initActiveTabs(params = {}) {
let primaryTab = PrimaryTabs.FILES;
if (params.queryMap && params.queryMap.has('tab')) {
primaryTab = params.queryMap.get('tab');
}
this._setActivePrimaryTab({
detail: {
tab: primaryTab,
},
});
// TODO: should drop this once we move CommentThreads tab
// to primary as well
let secondaryTab = SecondaryTabs.CHANGE_LOG;
if (params.queryMap && params.queryMap.has('secondaryTab')) {
secondaryTab = params.queryMap.get('secondaryTab');
}
this._setActiveSecondaryTab({
detail: {
tab: secondaryTab,
},
});
}
_sendShowChangeEvent() {
@@ -1019,28 +1091,6 @@ class GrChangeView extends mixinBehaviors( [
});
}
_setPrimaryTab() {
// Selected has to be set after the paper-tabs are visible, because
// the selected underline depends on calculations made by the browser.
// paper-tabs depends on iron-resizable-behavior, which only fires on
// attached() without using RenderStatus.beforeNextRender. Not changing
// this when migrating from Polymer 1 to 2 was probably an oversight by
// the paper component maintainers.
// https://polymer-library.polymer-project.org/2.0/docs/upgrade#attach-time-attached-connectedcallback
// By calling _onTabSizingChanged() we are reaching into the private API
// of paper-tabs, but we believe this workaround is acceptable for the
// time being.
beforeNextRender(this, () => {
this.$.commentTabs.selected = 0;
this.$.commentTabs._onTabSizingChanged();
const primaryTabs = this.shadowRoot.querySelector('#primaryTabs');
if (primaryTabs) {
primaryTabs.selected = 0;
primaryTabs._onTabSizingChanged();
}
});
}
_performPostLoadTasks() {
this._maybeShowReplyDialog();
this._maybeShowRevertDialog();

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

@@ -337,7 +337,14 @@ export const htmlTemplate = html`
}
</style>
<div class="container loading" hidden\$="[[!_loading]]">Loading...</div>
<div id="mainContent" class="container" on-show-checks-table="_handleShowTab" hidden\$="{{_loading}}">
<!-- TODO(taoalpha): remove on-show-checks-table,
Gerrit should not have any thing too special for a plugin,
replace with a generic event: show-primary-tab. -->
<div
id="mainContent"
class="container"
on-show-checks-table="_setActivePrimaryTab"
hidden\$="{{_loading}}">
<section class="changeInfoSection">
<div class\$="[[_computeHeaderClass(_editMode)]]">
<div class="headerTitle">
@@ -415,8 +422,8 @@ export const htmlTemplate = html`
</div>
</section>
<paper-tabs id="primaryTabs" on-selected-changed="_handleFileTabChange">
<paper-tab data-name\$="[[_files_tab_name]]">Files</paper-tab>
<paper-tabs id="primaryTabs" on-selected-changed="_setActivePrimaryTab">
<paper-tab data-name\$="[[_constants.PrimaryTabs.FILES]]">Files</paper-tab>
<template is="dom-repeat" items="[[_dynamicTabHeaderEndpoints]]" as="tabHeader">
<paper-tab data-name\$="[[tabHeader]]">
<gr-endpoint-decorator name\$="[[tabHeader]]">
@@ -427,23 +434,31 @@ export const htmlTemplate = html`
</gr-endpoint-decorator>
</paper-tab>
</template>
<paper-tab data-name\$="[[_findings_tab_name]]">
<paper-tab data-name\$="[[_constants.PrimaryTabs.FINDINGS]]">
Findings
</paper-tab>
</paper-tabs>
<section class="patchInfo">
<div hidden\$="[[!_findIfTabMatches(_currentTabName, _files_tab_name)]]">
<div hidden\$="[[!_isTabActive(_constants.PrimaryTabs.FILES, _activeTabs)]]">
<gr-file-list-header id="fileListHeader" account="[[_account]]" all-patch-sets="[[_allPatchSets]]" change="[[_change]]" change-num="[[_changeNum]]" revision-info="[[_revisionInfo]]" change-comments="[[_changeComments]]" commit-info="[[_commitInfo]]" change-url="[[_computeChangeUrl(_change)]]" edit-mode="[[_editMode]]" logged-in="[[_loggedIn]]" server-config="[[_serverConfig]]" shown-file-count="[[_shownFileCount]]" diff-prefs="[[_diffPrefs]]" diff-view-mode="{{viewState.diffMode}}" patch-num="{{_patchRange.patchNum}}" base-patch-num="{{_patchRange.basePatchNum}}" files-expanded="[[_filesExpanded]]" diff-prefs-disabled="[[_diffPrefsDisabled]]" on-open-diff-prefs="_handleOpenDiffPrefs" on-open-download-dialog="_handleOpenDownloadDialog" on-open-upload-help-dialog="_handleOpenUploadHelpDialog" on-open-included-in-dialog="_handleOpenIncludedInDialog" on-expand-diffs="_expandAllDiffs" on-collapse-diffs="_collapseAllDiffs">
</gr-file-list-header>
<gr-file-list id="fileList" class="hideOnMobileOverlay" diff-prefs="{{_diffPrefs}}" change="[[_change]]" change-num="[[_changeNum]]" patch-range="{{_patchRange}}" change-comments="[[_changeComments]]" drafts="[[_diffDrafts]]" revisions="[[_change.revisions]]" project-config="[[_projectConfig]]" selected-index="{{viewState.selectedFileIndex}}" diff-view-mode="[[viewState.diffMode]]" edit-mode="[[_editMode]]" num-files-shown="{{_numFilesShown}}" files-expanded="{{_filesExpanded}}" file-list-increment="{{_numFilesShown}}" on-files-shown-changed="_setShownFiles" on-file-action-tap="_handleFileActionTap" on-reload-drafts="_reloadDraftsWithCallback">
</gr-file-list>
</div>
<template is="dom-if" if="[[_findIfTabMatches(_currentTabName, _findings_tab_name)]]">
<template is="dom-if" if="[[_isTabActive(_constants.PrimaryTabs.FINDINGS, _activeTabs)]]">
<gr-dropdown-list class="patch-set-dropdown" items="[[_robotCommentsPatchSetDropdownItems]]" on-value-change="_handleRobotCommentPatchSetChanged" value="[[_currentRobotCommentsPatchSet]]">
</gr-dropdown-list>
<gr-thread-list threads="[[_robotCommentThreads]]" change="[[_change]]" change-num="[[_changeNum]]" logged-in="[[_loggedIn]]" tab="[[_findings_tab_name]]" hide-toggle-buttons="" on-thread-list-modified="_handleReloadDiffComments"></gr-thread-list>
<gr-thread-list
threads="[[_robotCommentThreads]]"
change="[[_change]]"
change-num="[[_changeNum]]"
logged-in="[[_loggedIn]]"
hide-toggle-buttons
empty-thread-msg="[[_messages.NO_ROBOT_COMMENTS_THREADS_MSG]]"
on-thread-list-modified="_handleReloadDiffComments">
</gr-thread-list>
<template is="dom-if" if="[[_showRobotCommentsButton]]">
<gr-button class="show-robot-comments" on-click="_toggleShowRobotComments">
[[_computeShowText(_showAllRobotComments)]]
@@ -451,7 +466,7 @@ export const htmlTemplate = html`
</template>
</template>
<template is="dom-if" if="[[_findIfTabMatches(_currentTabName, _selectedTabPluginHeader)]]">
<template is="dom-if" if="[[_isTabActive(_selectedTabPluginHeader, _activeTabs)]]">
<gr-endpoint-decorator name\$="[[_selectedTabPluginEndpoint]]">
<gr-endpoint-param name="change" value="[[_change]]">
</gr-endpoint-param>
@@ -468,20 +483,24 @@ export const htmlTemplate = html`
</gr-endpoint-param>
</gr-endpoint-decorator>
<paper-tabs id="commentTabs" on-selected-changed="_handleCommentTabChange">
<paper-tab class="changeLog">Change Log</paper-tab>
<paper-tab class="commentThreads">
<paper-tabs id="secondaryTabs" on-selected-changed="_setActiveSecondaryTab">
<paper-tab
data-name\$="[[_constants.SecondaryTabs.CHANGE_LOG]]"
class="changeLog">
Change Log
</paper-tab>
<paper-tab
data-name\$="[[_constants.SecondaryTabs.COMMENT_THREADS]]"
class="commentThreads">
<gr-tooltip-content has-tooltip="" title\$="[[_computeTotalCommentCounts(_change.unresolved_comment_count, _changeComments)]]">
<span>Comment Threads</span></gr-tooltip-content>
</paper-tab>
</paper-tabs>
<section class="changeLog">
<template is="dom-if" if="[[_isSelectedView(_currentView,
_commentTabs.CHANGE_LOG)]]">
<template is="dom-if" if="[[_isTabActive(_constants.SecondaryTabs.CHANGE_LOG, _activeTabs)]]">
<gr-messages-list class="hideOnMobileOverlay" change-num="[[_changeNum]]" labels="[[_change.labels]]" messages="[[_change.messages]]" reviewer-updates="[[_change.reviewer_updates]]" 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="[[_isSelectedView(_currentView,
_commentTabs.COMMENT_THREADS)]]">
<template is="dom-if" if="[[_isTabActive(_constants.SecondaryTabs.COMMENT_THREADS, _activeTabs)]]">
<gr-thread-list threads="[[_commentThreads]]" change="[[_change]]" change-num="[[_changeNum]]" logged-in="[[_loggedIn]]" only-show-robot-comments-with-human-reply="" on-thread-list-modified="_handleReloadDiffComments"></gr-thread-list>
</template>
</section>

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

@@ -41,6 +41,8 @@ limitations under the License.
import '../../../test/common-test-setup.js';
import '../../edit/gr-edit-constants.js';
import './gr-change-view.js';
import {PrimaryTabs, SecondaryTabs} from '../../../constants/constants.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
suite('gr-change-view tests', () => {
const kb = window.Gerrit.KeyboardShortcutBinder;
@@ -61,10 +63,6 @@ suite('gr-change-view tests', () => {
let navigateToChangeStub;
const TEST_SCROLL_TOP_PX = 100;
const CommentTabs = {
CHANGE_LOG: 0,
COMMENT_THREADS: 1,
};
const ROBOT_COMMENTS_LIMIT = 10;
const THREADS = [
@@ -364,13 +362,51 @@ suite('gr-change-view tests', () => {
'change-view-tab-header-url');
});
test('handleShowTab switched tab correctly', done => {
const paperTabs = element.shadowRoot.querySelector('#primaryTabs');
assert.equal(paperTabs.selected, 0);
element._handleShowTab({detail:
test('_setActivePrimaryTab switched tab correctly', done => {
element._setActivePrimaryTab({detail:
{tab: 'change-view-tab-header-url'}});
flush(() => {
assert.equal(paperTabs.selected, 1);
assert.equal(element._activeTabs[0], 'change-view-tab-header-url');
done();
});
});
test('show-primary-tab switched primary tab correctly', done => {
element.fire('show-primary-tab', {tab: 'change-view-tab-header-url'});
flush(() => {
assert.equal(element._activeTabs[0], 'change-view-tab-header-url');
done();
});
});
test('param change should switch primary tab correctly', done => {
assert.equal(element._activeTabs[0], PrimaryTabs.FILES);
const queryMap = new Map();
queryMap.set('tab', PrimaryTabs.FINDINGS);
// view is required
element.params = Object.assign(
{
view: Gerrit.Nav.View.CHANGE,
},
element.params, {queryMap});
flush(() => {
assert.equal(element._activeTabs[0], PrimaryTabs.FINDINGS);
done();
});
});
test('invalid param change should not switch primary tab', done => {
assert.equal(element._activeTabs[0], PrimaryTabs.FILES);
const queryMap = new Map();
queryMap.set('tab', 'random');
// view is required
element.params = Object.assign(
{
view: Gerrit.Nav.View.CHANGE,
},
element.params, {queryMap});
flush(() => {
assert.equal(element._activeTabs[0], PrimaryTabs.FILES);
done();
});
});
@@ -680,7 +716,7 @@ suite('gr-change-view tests', () => {
test('thread list modified', () => {
sandbox.spy(element, '_handleReloadDiffComments');
element._currentView = CommentTabs.COMMENT_THREADS;
element._activeTabs = [PrimaryTabs.FILES, SecondaryTabs.COMMENT_THREADS];
flushAsynchronousOperations();
return element._reloadComments().then(() => {
@@ -746,25 +782,55 @@ suite('gr-change-view tests', () => {
test('tab switch works correctly', done => {
assert.isTrue(element._paramsChanged.called);
assert.equal(element.$.commentTabs.selected, CommentTabs.CHANGE_LOG);
assert.equal(element._currentView, CommentTabs.CHANGE_LOG);
assert.equal(element._activeTabs[1], SecondaryTabs.CHANGE_LOG);
const commentTab = element.shadowRoot.querySelector(
'paper-tab.commentThreads'
);
// Switch to comment thread tab
MockInteractions.tap(commentTab);
const commentTabs = element.$.commentTabs;
assert.equal(commentTabs.selected,
CommentTabs.COMMENT_THREADS);
assert.equal(element._currentView, CommentTabs.COMMENT_THREADS);
assert.equal(element._activeTabs[1], SecondaryTabs.COMMENT_THREADS);
// Switch back to 'Change Log' tab
element._paramsChanged(element.params);
flush(() => {
assert.equal(commentTabs.selected,
CommentTabs.CHANGE_LOG);
assert.equal(element._currentView, CommentTabs.CHANGE_LOG);
assert.equal(element._activeTabs[1], SecondaryTabs.CHANGE_LOG);
done();
});
});
test('show-secondary-tab event works', () => {
assert.equal(element._activeTabs[1], SecondaryTabs.CHANGE_LOG);
// Switch to comment thread tab
element.fire('show-secondary-tab', {tab: SecondaryTabs.COMMENT_THREADS});
assert.equal(element._activeTabs[1], SecondaryTabs.COMMENT_THREADS);
});
test('param change should switched secondary tab correctly', done => {
assert.equal(element._activeTabs[1], SecondaryTabs.CHANGE_LOG);
const queryMap = new Map();
queryMap.set('secondaryTab', SecondaryTabs.COMMENT_THREADS);
// view is required
element.params = Object.assign(
{view: Gerrit.Nav.View.CHANGE},
element.params, {queryMap}
);
flush(() => {
assert.equal(element._activeTabs[1], SecondaryTabs.COMMENT_THREADS);
done();
});
});
test('invalid secondaryTab should not switch tab', done => {
assert.equal(element._activeTabs[1], SecondaryTabs.CHANGE_LOG);
const queryMap = new Map();
queryMap.set('secondaryTab', 'random');
// view is required
element.params = Object.assign({
view: Gerrit.Nav.View.CHANGE,
}, element.params, {queryMap});
flush(() => {
assert.equal(element._activeTabs[1], SecondaryTabs.CHANGE_LOG);
done();
});
});

19
polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
View File

@@ -25,18 +25,14 @@ import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mix
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
import {htmlTemplate} from './gr-thread-list_html.js';
import {NO_THREADS_MSG} from '../../../constants/messages.js';
/**
* Fired when a comment is saved or deleted
*
* @event thread-list-modified
* @extends Polymer.Element
*/
const NO_THREADS_MESSAGE = 'There are no inline comment threads on any diff '
+ 'for this change.';
const NO_ROBOT_COMMENTS_THREADS_MESSAGE = 'There are no findings for this ' +
'patchset.';
const FINDINGS_TAB_NAME = '__gerrit_internal_findings';
class GrThreadList extends GestureEventListeners(
LegacyElementMixin(
PolymerElement)) {
@@ -78,9 +74,9 @@ class GrThreadList extends GestureEventListeners(
type: Boolean,
value: false,
},
tab: {
emptyThreadMsg: {
type: String,
value: '',
value: NO_THREADS_MSG,
},
};
}
@@ -91,13 +87,6 @@ class GrThreadList extends GestureEventListeners(
return loggedIn ? 'show' : '';
}
_computeNoThreadsMessage(tab) {
if (tab === FINDINGS_TAB_NAME) {
return NO_ROBOT_COMMENTS_THREADS_MESSAGE;
}
return NO_THREADS_MESSAGE;
}
/**
* Order as follows:
* - Unresolved threads with drafts (reverse chronological)

2
polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.js
View File

@@ -66,7 +66,7 @@ export const htmlTemplate = html`
</template>
<div id="threads">
<template is="dom-if" if="[[!threads.length]]">
[[_computeNoThreadsMessage(tab)]]
[[emptyThreadMsg]]
</template>
<template is="dom-repeat" items="[[_filteredThreads]]" as="thread" initial-count="5" target-framerate="60">
<gr-comment-thread show-file-path="" change-num="[[changeNum]]" comments="[[thread.comments]]" comment-side="[[thread.commentSide]]" project-name="[[change.project]]" is-on-parent="[[_isOnParent(thread.commentSide)]]" line-num="[[thread.line]]" patch-num="[[thread.patchNum]]" path="[[thread.path]]" root-id="{{thread.rootId}}" on-thread-changed="_handleCommentsChanged" on-thread-discard="_handleThreadDiscard"></gr-comment-thread>

28
polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
View File

@@ -34,6 +34,7 @@ limitations under the License.
import '../../../test/common-test-setup.js';
import './gr-thread-list.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {NO_THREADS_MSG} from '../../../constants/messages.js';
suite('gr-thread-list tests', () => {
let element;
let sandbox;
@@ -361,17 +362,42 @@ suite('gr-thread-list tests', () => {
assert.equal(dispatchSpy.lastCall.args[0].detail.path, '/COMMIT_MSG');
});
suite('findings tab', () => {
suite('hideToggleButtons', () => {
setup(done => {
element.hideToggleButtons = true;
flush(() => {
done();
});
});
test('toggle buttons are hidden', () => {
assert.equal(element.shadowRoot.querySelector('.header').style.display,
'none');
});
});
suite('empty thread', () => {
setup(done => {
element.threads = [];
flush(() => {
done();
});
});
test('default empty message should show', () => {
assert.equal(
element.shadowRoot.querySelector('#threads').textContent.trim(),
NO_THREADS_MSG
);
});
test('can override empty message', () => {
element.emptyThreadMsg = 'test';
assert.equal(
element.shadowRoot.querySelector('#threads').textContent.trim(),
'test'
);
});
});
});
</script>

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

@@ -682,6 +682,21 @@ class GrRouter extends mixinBehaviors( [
this.$.restAPI.getLoggedIn().then(() => { next(); });
}
/** Page.js middleware that try parse the querystring into queryMap. */
_queryStringMiddleware(ctx, next) {
let queryMap = new Map();
if (ctx.querystring) {
// https://caniuse.com/#search=URLSearchParams
if (window.URLSearchParams) {
queryMap = new URLSearchParams(ctx.querystring);
} else {
queryMap = new Map(this._parseQueryString(ctx.querystring));
}
}
ctx.queryMap = queryMap;
next();
}
/**
* Map a route to a method on the router.
*
@@ -701,7 +716,10 @@ class GrRouter extends mixinBehaviors( [
handlerName);
return;
}
page(pattern, this._loadUserMiddleware.bind(this), data => {
page(pattern,
(ctx, next) => this._loadUserMiddleware(ctx, next),
(ctx, next) => this._queryStringMiddleware(ctx, next),
data => {
this.$.reporting.locationChanged(handlerName);
const promise = opt_authRedirect ?
this._redirectIfNotLoggedIn(data) : Promise.resolve();
@@ -1342,6 +1360,7 @@ class GrRouter extends mixinBehaviors( [
basePatchNum: ctx.params[4],
patchNum: ctx.params[6],
view: Gerrit.Nav.View.CHANGE,
queryMap: ctx.queryMap,
};
this.$.reporting.setRepoName(params.project);

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

@@ -1427,6 +1427,7 @@ suite('gr-router tests', () => {
null, // 5 Unused
7, // 6 Patch number
],
queryMap: new Map(),
};
}
@@ -1457,6 +1458,7 @@ suite('gr-router tests', () => {
changeNum: 1234,
basePatchNum: 4,
patchNum: 7,
queryMap: new Map(),
});
assert.isFalse(redirectStub.called);
assert.isTrue(normalizeRangeStub.called);
@@ -1639,8 +1641,8 @@ suite('gr-router tests', () => {
element._parseQueryString('a=b&c=d&e=f'),
[['a', 'b'], ['c', 'd'], ['e', 'f']]);
assert.deepEqual(
element._parseQueryString('&a=b&&&e=f&'),
[['a', 'b'], ['e', 'f']]);
element._parseQueryString('&a=b&&&e=f&c'),
[['a', 'b'], ['e', 'f'], ['c', '']]);
});
});
});