Merge "Format mention suggestions with name + email"
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
index 9f8f930..62de868 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
@@ -47,10 +47,19 @@
adminViewModelToken,
AdminViewState,
} from '../../../models/views/admin';
-import {GroupDetailView, GroupViewState} from '../../../models/views/group';
-import {RepoDetailView, RepoViewState} from '../../../models/views/repo';
+import {
+ GroupDetailView,
+ groupViewModelToken,
+ GroupViewState,
+} from '../../../models/views/group';
+import {
+ RepoDetailView,
+ repoViewModelToken,
+ RepoViewState,
+} from '../../../models/views/repo';
import {resolve} from '../../../models/dependency';
import {subscribe} from '../../lit/subscription-controller';
+import {merge} from 'rxjs';
const INTERNAL_GROUP_REGEX = /^[\da-f]{40}$/;
@@ -123,13 +132,22 @@
private readonly restApiService = getAppContext().restApiService;
- private readonly getViewModel = resolve(this, adminViewModelToken);
+ private readonly getAdminViewModel = resolve(this, adminViewModelToken);
+
+ private readonly getGroupViewModel = resolve(this, groupViewModelToken);
+
+ private readonly getRepoViewModel = resolve(this, repoViewModelToken);
constructor() {
super();
subscribe(
this,
- () => this.getViewModel().state$,
+ () =>
+ merge(
+ this.getAdminViewModel().state$,
+ this.getGroupViewModel().state$,
+ this.getRepoViewModel().state$
+ ),
x => {
this.viewState = x;
if (this.needsReload()) this.reload();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index c20ca1d..d076439 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -181,7 +181,11 @@
import {getBaseUrl, prependOrigin} from '../../../utils/url-util';
import {CopyLink, GrCopyLinks} from '../gr-copy-links/gr-copy-links';
import {KnownExperimentId} from '../../../services/flags/flags';
-import {ChangeViewState, createChangeUrl} from '../../../models/views/change';
+import {
+ changeViewModelToken,
+ ChangeViewState,
+ createChangeUrl,
+} from '../../../models/views/change';
import {rootUrl} from '../../../utils/url-util';
import {createEditUrl} from '../../../models/views/edit';
@@ -275,23 +279,19 @@
@query('gr-copy-links') private copyLinksDropdown?: GrCopyLinks;
- /**
- * URL params passed from the router.
- * Use params getter/setter.
- */
- private _params?: ChangeViewState;
+ private _viewState?: ChangeViewState;
@property({type: Object})
- get params() {
- return this._params;
+ get viewState() {
+ return this._viewState;
}
- set params(params: ChangeViewState | undefined) {
- if (this._params === params) return;
- const oldParams = this._params;
- this._params = params;
- this.paramsChanged();
- this.requestUpdate('params', oldParams);
+ set viewState(viewState: ChangeViewState | undefined) {
+ if (this._viewState === viewState) return;
+ const oldViewState = this._viewState;
+ this._viewState = viewState;
+ this.viewStateChanged();
+ this.requestUpdate('viewState', oldViewState);
}
@property({type: String})
@@ -433,11 +433,11 @@
// Private but used in tests.
getEditMode() {
- if (!this.patchRange || !this.params) {
+ if (!this.patchRange || !this.viewState) {
return false;
}
- if (this.params.edit) {
+ if (this.viewState.edit) {
return true;
}
@@ -554,6 +554,8 @@
private readonly getFilesModel = resolve(this, filesModelToken);
+ private readonly getViewModel = resolve(this, changeViewModelToken);
+
private readonly getShortcutsService = resolve(this, shortcutsServiceToken);
private replyRefitTask?: DelayedTask;
@@ -693,6 +695,11 @@
private setupSubscriptions() {
subscribe(
this,
+ () => this.getViewModel().state$,
+ s => (this.viewState = s)
+ );
+ subscribe(
+ this,
() => this.getChecksModel().aPluginHasRegistered$,
b => {
this.showChecksTab = b;
@@ -2094,25 +2101,25 @@
*/
private isChangeObsolete() {
// While this.changeNum is undefined the change view is fresh and has just
- // not updated it to params.changeNum yet. Not obsolete in that case.
+ // not updated it to viewState.changeNum yet. Not obsolete in that case.
if (this.changeNum === undefined) return false;
- // this.params reflects the current state of the URL. If this.changeNum
+ // this.viewState reflects the current state of the URL. If this.changeNum
// does not match it anymore, then this view must be considered obsolete.
- return this.changeNum !== this.params?.changeNum;
+ return this.changeNum !== this.viewState?.changeNum;
}
// Private but used in tests.
- hasPatchRangeChanged(value: ChangeViewState) {
+ hasPatchRangeChanged(viewState: ChangeViewState) {
if (!this.patchRange) return false;
- if (this.patchRange.basePatchNum !== value.basePatchNum) return true;
- return this.hasPatchNumChanged(value);
+ if (this.patchRange.basePatchNum !== viewState.basePatchNum) return true;
+ return this.hasPatchNumChanged(viewState);
}
// Private but used in tests.
- hasPatchNumChanged(value: ChangeViewState) {
+ hasPatchNumChanged(viewState: ChangeViewState) {
if (!this.patchRange) return false;
- if (value.patchNum !== undefined) {
- return this.patchRange.patchNum !== value.patchNum;
+ if (viewState.patchNum !== undefined) {
+ return this.patchRange.patchNum !== viewState.patchNum;
} else {
// value.patchNum === undefined specifies the latest patchset
return (
@@ -2122,8 +2129,8 @@
}
// Private but used in tests.
- paramsChanged() {
- if (this.params?.view !== GerritView.CHANGE) {
+ viewStateChanged() {
+ if (this.viewState === undefined) {
this.initialLoadComplete = false;
querySelectorAll(this, 'gr-overlay').forEach(overlay =>
(overlay as GrOverlay).close()
@@ -2138,24 +2145,24 @@
return;
}
- if (this.params.changeNum && this.params.project) {
+ if (this.viewState.changeNum && this.viewState.project) {
this.restApiService.setInProjectLookup(
- this.params.changeNum,
- this.params.project
+ this.viewState.changeNum,
+ this.viewState.project
);
}
- if (this.params.basePatchNum === undefined)
- this.params.basePatchNum = PARENT;
+ if (this.viewState.basePatchNum === undefined)
+ this.viewState.basePatchNum = PARENT;
- const patchChanged = this.hasPatchRangeChanged(this.params);
- let patchNumChanged = this.hasPatchNumChanged(this.params);
+ const patchChanged = this.hasPatchRangeChanged(this.viewState);
+ let patchNumChanged = this.hasPatchNumChanged(this.viewState);
this.patchRange = {
- patchNum: this.params.patchNum,
- basePatchNum: this.params.basePatchNum,
+ patchNum: this.viewState.patchNum,
+ basePatchNum: this.viewState.basePatchNum,
};
- this.scrollCommentId = this.params.commentId;
+ this.scrollCommentId = this.viewState.commentId;
const patchKnown =
!this.patchRange.patchNum ||
@@ -2163,7 +2170,7 @@
ps => ps.num === this.patchRange!.patchNum
);
// _allPatchsets does not know value.patchNum so force a reload.
- const forceReload = this.params.forceReload || !patchKnown;
+ const forceReload = this.viewState.forceReload || !patchKnown;
// If changeNum is defined that means the change has already been
// rendered once before so a full reload is not required.
@@ -2176,7 +2183,7 @@
patchNumChanged = true;
}
if (patchChanged) {
- // We need to collapse all diffs when params change so that a non
+ // We need to collapse all diffs when viewState changes so that a non
// existing diff is not requested. See Issue 125270 for more details.
this.fileList?.resetFileState();
this.fileList?.collapseAllDiffs();
@@ -2198,8 +2205,8 @@
return;
}
- // We need to collapse all diffs when params change so that a non existing
- // diff is not requested. See Issue 125270 for more details.
+ // We need to collapse all diffs when viewState changes so that a non
+ // existing diff is not requested. See Issue 125270 for more details.
this.updateComplete.then(() => {
assertIsDefined(this.fileList);
this.fileList?.collapseAllDiffs();
@@ -2218,7 +2225,7 @@
}
this.initialLoadComplete = false;
- this.changeNum = this.params.changeNum;
+ this.changeNum = this.viewState.changeNum;
this.loadData(true).then(() => {
this.performPostLoadTasks();
});
@@ -2232,9 +2239,9 @@
private initActiveTab() {
let tab = Tab.FILES;
- if (this.params?.tab) {
- tab = this.params?.tab as Tab;
- } else if (this.params?.commentId) {
+ if (this.viewState?.tab) {
+ tab = this.viewState?.tab as Tab;
+ } else if (this.viewState?.commentId) {
tab = Tab.COMMENT_THREADS;
}
const detail: SwitchTabEventDetail = {
@@ -2243,8 +2250,8 @@
if (tab === Tab.CHECKS) {
const state: ChecksTabState = {};
detail.tabState = {checksTab: state};
- if (this.params?.filter) state.filter = this.params.filter;
- if (this.params?.attempt) state.attempt = this.params.attempt;
+ if (this.viewState?.filter) state.filter = this.viewState.filter;
+ if (this.viewState?.attempt) state.attempt = this.viewState.attempt;
}
this.setActiveTab(
@@ -2340,7 +2347,7 @@
private maybeShowReplyDialog() {
if (!this.loggedIn) return;
- if (this.params?.openReplyDialog) {
+ if (this.viewState?.openReplyDialog) {
this.openReplyDialog(FocusTarget.ANY);
}
}
@@ -2750,7 +2757,7 @@
private async untilModelLoaded() {
// NOTE: Wait until this page is connected before determining whether the
- // model is loaded. This can happen when params are changed when setting up
+ // model is loaded. This can happen when viewState changes when setting up
// this view. It's unclear whether this issue is related to Polymer
// specifically.
if (!this.isConnected) {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 8bb937f..0e6fb71 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -367,7 +367,7 @@
element = await fixture<GrChangeView>(
html`<gr-change-view></gr-change-view>`
);
- element.params = {
+ element.viewState = {
view: GerritView.CHANGE,
changeNum: TEST_NUMERIC_CHANGE_ID,
project: 'gerrit' as RepoName,
@@ -730,9 +730,9 @@
assert.equal(element.activeTab, Tab.FILES);
// view is required
element.changeNum = undefined;
- element.params = {
+ element.viewState = {
...createChangeViewState(),
- ...element.params,
+ ...element.viewState,
tab: Tab.COMMENT_THREADS,
};
await element.updateComplete;
@@ -742,9 +742,9 @@
test('invalid param change should not switch primary tab', async () => {
assert.equal(element.activeTab, Tab.FILES);
// view is required
- element.params = {
+ element.viewState = {
...createChangeViewState(),
- ...element.params,
+ ...element.viewState,
tab: 'random',
};
await element.updateComplete;
@@ -1054,8 +1054,8 @@
) as GrRelatedChangesList;
sinon.stub(relatedChanges, 'reload');
sinon.stub(element, 'loadData').returns(Promise.resolve());
- sinon.spy(element, 'paramsChanged');
- element.params = createChangeViewState();
+ sinon.spy(element, 'viewStateChanged');
+ element.viewState = createChangeViewState();
});
});
@@ -1549,7 +1549,7 @@
await element.updateComplete;
element.changeNum = 2 as NumericChangeId;
- element.params = {
+ element.viewState = {
...createChangeViewState(),
changeNum: 2 as NumericChangeId,
};
@@ -1574,7 +1574,7 @@
patchNum: 1 as RevisionPatchSetNum,
};
element.changeNum = undefined;
- element.params = value;
+ element.viewState = value;
await element.updateComplete;
assert.isTrue(reloadStub.calledOnce);
@@ -1590,7 +1590,7 @@
value.basePatchNum = 1 as BasePatchSetNum;
value.patchNum = 2 as RevisionPatchSetNum;
- element.params = {...value};
+ element.viewState = {...value};
await element.updateComplete;
await waitEventLoop();
assert.equal(element.fileList.selectedIndex, 0);
@@ -1619,7 +1619,7 @@
view: GerritView.CHANGE,
patchNum: 1 as RevisionPatchSetNum,
};
- element.params = value;
+ element.viewState = value;
await element.updateComplete;
element.initialLoadComplete = true;
@@ -1633,7 +1633,7 @@
value.basePatchNum = 1 as BasePatchSetNum;
value.patchNum = 2 as RevisionPatchSetNum;
- element.params = {...value};
+ element.viewState = {...value};
await element.updateComplete;
assert.isTrue(reloadPortedCommentsStub.calledOnce);
assert.isTrue(reloadPortedDraftsStub.calledOnce);
@@ -1646,13 +1646,13 @@
.callsFake(() => Promise.resolve());
const collapseStub = sinon.stub(element.fileList, 'collapseAllDiffs');
const value: ChangeViewState = createChangeViewState();
- element.params = value;
+ element.viewState = value;
// change already loaded
assert.isOk(element.changeNum);
await element.updateComplete;
assert.isFalse(reloadStub.calledOnce);
element.initialLoadComplete = true;
- element.params = {...value};
+ element.viewState = {...value};
await element.updateComplete;
assert.isFalse(reloadStub.calledTwice);
assert.isFalse(collapseStub.calledTwice);
@@ -1667,7 +1667,7 @@
.stub(element, 'loadData')
.callsFake(() => Promise.resolve());
const collapseStub = sinon.stub(element.fileList, 'collapseAllDiffs');
- element.params = {...createChangeViewState(), forceReload: true};
+ element.viewState = {...createChangeViewState(), forceReload: true};
await element.updateComplete;
assert.isTrue(getChangeStub.called);
assert.isTrue(loadDataStub.called);
@@ -1681,12 +1681,12 @@
element.addEventListener('recreate-change-view', recreateSpy);
const value: ChangeViewState = createChangeViewState();
- element.params = value;
+ element.viewState = value;
await element.updateComplete;
assert.isFalse(recreateSpy.calledOnce);
value.changeNum = 555111333 as NumericChangeId;
- element.params = {...value};
+ element.viewState = {...value};
await element.updateComplete;
assert.isTrue(recreateSpy.calledOnce);
});
@@ -1701,7 +1701,7 @@
Promise.resolve({...createMergeable(), mergeable: true})
);
- element.params = createChangeViewState();
+ element.viewState = createChangeViewState();
element.getChangeModel().setState({
loadingStatus: LoadingStatus.LOADED,
change: {
@@ -2019,8 +2019,8 @@
test('header class computation', () => {
assert.equal(element.computeHeaderClass(), 'header');
- assertIsDefined(element.params);
- element.params.edit = true;
+ assertIsDefined(element.viewState);
+ element.viewState.edit = true;
assert.equal(element.computeHeaderClass(), 'header editMode');
});
@@ -2038,8 +2038,8 @@
});
test('computeEditMode', async () => {
- const callCompute = async (params: ChangeViewState) => {
- element.params = params;
+ const callCompute = async (viewState: ChangeViewState) => {
+ element.viewState = viewState;
await element.updateComplete;
return element.getEditMode();
};
@@ -2256,30 +2256,30 @@
element.change.current_revision = '1' as CommitId;
element.change = {...element.change};
- const params = createChangeViewState();
+ const viewState = createChangeViewState();
- assert.isFalse(element.hasPatchRangeChanged(params));
- assert.isFalse(element.hasPatchNumChanged(params));
+ assert.isFalse(element.hasPatchRangeChanged(viewState));
+ assert.isFalse(element.hasPatchNumChanged(viewState));
- params.basePatchNum = PARENT;
+ viewState.basePatchNum = PARENT;
// undefined means navigate to latest patchset
- params.patchNum = undefined;
+ viewState.patchNum = undefined;
element.patchRange = {
patchNum: 2 as RevisionPatchSetNum,
basePatchNum: PARENT,
};
- assert.isTrue(element.hasPatchRangeChanged(params));
- assert.isTrue(element.hasPatchNumChanged(params));
+ assert.isTrue(element.hasPatchRangeChanged(viewState));
+ assert.isTrue(element.hasPatchNumChanged(viewState));
element.patchRange = {
patchNum: 4 as RevisionPatchSetNum,
basePatchNum: PARENT,
};
- assert.isFalse(element.hasPatchRangeChanged(params));
- assert.isFalse(element.hasPatchNumChanged(params));
+ assert.isFalse(element.hasPatchRangeChanged(viewState));
+ assert.isFalse(element.hasPatchNumChanged(viewState));
});
suite('handleEditTap', () => {
@@ -2485,7 +2485,7 @@
assert.isFalse(changeFullyLoadedStub.called);
});
- test('report changeDisplayed on paramsChanged', async () => {
+ test('report changeDisplayed on viewStateChanged', async () => {
stubRestApi('getChangeOrEditFiles').resolves({
'a-file.js': {},
});
@@ -2499,7 +2499,7 @@
);
// reset so reload is triggered
element.changeNum = undefined;
- element.params = {
+ element.viewState = {
...createChangeViewState(),
changeNum: TEST_NUMERIC_CHANGE_ID,
project: TEST_PROJECT_NAME,
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index a0fec50..354d596 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -457,10 +457,7 @@
// Template as not to create duplicates, for renderChangeView() only.
private changeViewTemplate() {
return html`
- <gr-change-view
- .params=${this.params}
- .backPage=${this.lastSearchPage}
- ></gr-change-view>
+ <gr-change-view .backPage=${this.lastSearchPage}></gr-change-view>
`;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
index 3887ee5b..eab6638 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
@@ -34,7 +34,7 @@
export const MERGE_CONFLICT_TOOLTIP =
'This change has merge conflicts. ' +
- 'Download the patch and run "git rebase". ' +
+ 'Rebase on the upstream branch (e.g. "git pull --rebase"). ' +
'Upload a new patchset after resolving all merge conflicts.';
export const GIT_CONFLICT_TOOLTIP =
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index f5e8a1a..cc76150 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -501,6 +501,9 @@
.draft gr-account-label {
width: unset;
}
+ .draft gr-formatted-text.message {
+ margin-bottom: var(--spacing-m);
+ }
.portedMessage {
margin: 0 var(--spacing-m);
}
@@ -723,7 +726,6 @@
class="message"
.content=${this.comment?.message}
.config=${this.commentLinks}
- ?noTrailingMargin=${!isDraftOrUnsaved(this.comment)}
></gr-formatted-text>
`;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 2e4383e..5686c6b 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -141,10 +141,7 @@
</div>
</div>
<div class="body">
- <gr-formatted-text
- class="message"
- notrailingmargin=""
- ></gr-formatted-text>
+ <gr-formatted-text class="message"></gr-formatted-text>
</div>
</div>
`
@@ -178,10 +175,7 @@
</div>
<div class="body">
<div class="robotId"></div>
- <gr-formatted-text
- class="message"
- notrailingmargin=""
- ></gr-formatted-text>
+ <gr-formatted-text class="message"></gr-formatted-text>
<div class="robotActions">
<gr-icon
icon="link"
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts
index 51b1862..7f75d83 100644
--- a/polygerrit-ui/app/models/views/change.ts
+++ b/polygerrit-ui/app/models/views/change.ts
@@ -41,10 +41,6 @@
usp?: string;
}
-const DEFAULT_STATE: ChangeViewState = {
- view: GerritView.CHANGE,
-};
-
export function createChangeUrl(state: Omit<ChangeViewState, 'view'>) {
let range = getPatchRangeExpression(state);
if (range.length) {
@@ -84,8 +80,8 @@
export const changeViewModelToken =
define<ChangeViewModel>('change-view-model');
-export class ChangeViewModel extends Model<ChangeViewState> {
+export class ChangeViewModel extends Model<ChangeViewState | undefined> {
constructor() {
- super(DEFAULT_STATE);
+ super(undefined);
}
}
diff --git a/polygerrit-ui/app/models/views/repo.ts b/polygerrit-ui/app/models/views/repo.ts
index 93064fd..02fd17d 100644
--- a/polygerrit-ui/app/models/views/repo.ts
+++ b/polygerrit-ui/app/models/views/repo.ts
@@ -27,10 +27,6 @@
offset?: number | string;
}
-const DEFAULT_STATE: RepoViewState = {
- view: GerritView.REPO,
-};
-
export function createRepoUrl(state: Omit<RepoViewState, 'view'>) {
let url = `/admin/repos/${encodeURL(`${state.repo}`, true)}`;
if (state.detail === RepoDetailView.GENERAL) {
@@ -51,8 +47,8 @@
export const repoViewModelToken = define<RepoViewModel>('repo-view-model');
-export class RepoViewModel extends Model<RepoViewState> {
+export class RepoViewModel extends Model<RepoViewState | undefined> {
constructor() {
- super(DEFAULT_STATE);
+ super(undefined);
}
}
