Merge "Revert "Add lit element based gr-diff elements""
diff --git a/java/com/google/gerrit/extensions/webui/PatchSetWebLink.java b/java/com/google/gerrit/extensions/webui/PatchSetWebLink.java
index 0e8e28e..74bccbd 100644
--- a/java/com/google/gerrit/extensions/webui/PatchSetWebLink.java
+++ b/java/com/google/gerrit/extensions/webui/PatchSetWebLink.java
@@ -37,6 +37,34 @@
* @return WebLinkInfo that links to patch set in external service, null if there should be no
* link.
*/
+ @Deprecated
WebLinkInfo getPatchSetWebLink(
String projectName, String commit, String commitMessage, String branchName);
+
+ /**
+ * {@link com.google.gerrit.extensions.common.WebLinkInfo} describing a link from a patch set to
+ * an external service.
+ *
+ * <p>In order for the web link to be visible {@link
+ * com.google.gerrit.extensions.common.WebLinkInfo#url} and {@link
+ * com.google.gerrit.extensions.common.WebLinkInfo#name} must be set.
+ *
+ * <p>
+ *
+ * @param projectName name of the project
+ * @param commit commit of the patch set
+ * @param commitMessage the commit message of the change
+ * @param branchName target branch of the change
+ * @param changeKey the changeID for this change
+ * @return WebLinkInfo that links to patch set in external service, null if there should be no
+ * link.
+ */
+ default WebLinkInfo getPatchSetWebLink(
+ String projectName,
+ String commit,
+ String commitMessage,
+ String branchName,
+ String changeKey) {
+ return getPatchSetWebLink(projectName, commit, commitMessage, branchName);
+ }
}
diff --git a/java/com/google/gerrit/server/WebLinks.java b/java/com/google/gerrit/server/WebLinks.java
index 3a82694..dd36066 100644
--- a/java/com/google/gerrit/server/WebLinks.java
+++ b/java/com/google/gerrit/server/WebLinks.java
@@ -86,12 +86,19 @@
* @param commit SHA1 of commit.
* @param commitMessage the commit message of the commit.
* @param branchName branch of the commit.
+ * @param changeKey change Identifier for this change
*/
public ImmutableList<WebLinkInfo> getPatchSetLinks(
- Project.NameKey project, String commit, String commitMessage, String branchName) {
+ Project.NameKey project,
+ String commit,
+ String commitMessage,
+ String branchName,
+ String changeKey) {
return filterLinks(
patchSetLinks,
- webLink -> webLink.getPatchSetWebLink(project.get(), commit, commitMessage, branchName));
+ webLink ->
+ webLink.getPatchSetWebLink(
+ project.get(), commit, commitMessage, branchName, changeKey));
}
/**
diff --git a/java/com/google/gerrit/server/change/RevisionJson.java b/java/com/google/gerrit/server/change/RevisionJson.java
index 0321fcb..7d40f06 100644
--- a/java/com/google/gerrit/server/change/RevisionJson.java
+++ b/java/com/google/gerrit/server/change/RevisionJson.java
@@ -168,7 +168,8 @@
RevCommit commit,
boolean addLinks,
boolean fillCommit,
- String branchName)
+ String branchName,
+ String changeKey)
throws IOException {
CommitInfo info = new CommitInfo();
if (fillCommit) {
@@ -182,7 +183,8 @@
if (addLinks) {
ImmutableList<WebLinkInfo> patchSetLinks =
- webLinks.getPatchSetLinks(project, commit.name(), commit.getFullMessage(), branchName);
+ webLinks.getPatchSetLinks(
+ project, commit.name(), commit.getFullMessage(), branchName, changeKey);
info.webLinks = patchSetLinks.isEmpty() ? null : patchSetLinks;
ImmutableList<WebLinkInfo> resolveConflictsLinks =
webLinks.getResolveConflictsLinks(
@@ -301,7 +303,9 @@
rw.parseBody(commit);
String branchName = cd.change().getDest().branch();
if (setCommit) {
- out.commit = getCommitInfo(project, rw, commit, has(WEB_LINKS), fillCommit, branchName);
+ out.commit =
+ getCommitInfo(
+ project, rw, commit, has(WEB_LINKS), fillCommit, branchName, c.getKey().get());
}
if (addFooters) {
Ref ref = repo.exactRef(branchName);
diff --git a/java/com/google/gerrit/server/restapi/change/GetCommit.java b/java/com/google/gerrit/server/restapi/change/GetCommit.java
index d76ce04..5193501 100644
--- a/java/com/google/gerrit/server/restapi/change/GetCommit.java
+++ b/java/com/google/gerrit/server/restapi/change/GetCommit.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.restapi.change;
+import static java.util.concurrent.TimeUnit.DAYS;
+
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.common.CommitInfo;
@@ -25,7 +27,6 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
import java.io.IOException;
-import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -64,10 +65,11 @@
commit,
addLinks,
/* fillCommit= */ true,
- rsrc.getChange().getDest().branch());
+ rsrc.getChange().getDest().branch(),
+ rsrc.getChange().getKey().get());
Response<CommitInfo> r = Response.ok(info);
if (rsrc.isCacheable()) {
- r.caching(CacheControl.PRIVATE(7, TimeUnit.DAYS));
+ r.caching(CacheControl.PRIVATE(7, DAYS));
}
return r;
}
diff --git a/java/com/google/gerrit/server/restapi/change/GetMergeList.java b/java/com/google/gerrit/server/restapi/change/GetMergeList.java
index f0639b5..551b50f 100644
--- a/java/com/google/gerrit/server/restapi/change/GetMergeList.java
+++ b/java/com/google/gerrit/server/restapi/change/GetMergeList.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.restapi.change;
+import static java.util.concurrent.TimeUnit.DAYS;
+
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Project;
@@ -30,7 +32,6 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
-import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -77,7 +78,7 @@
return createResponse(rsrc, ImmutableList.of());
}
- List<RevCommit> commits = MergeListBuilder.build(rw, commit, uninterestingParent);
+ ImmutableList<RevCommit> commits = MergeListBuilder.build(rw, commit, uninterestingParent);
List<CommitInfo> result = new ArrayList<>(commits.size());
RevisionJson changeJson = json.create(ImmutableSet.of());
for (RevCommit c : commits) {
@@ -88,7 +89,8 @@
c,
addLinks,
/* fillCommit= */ true,
- rsrc.getChange().getDest().branch()));
+ rsrc.getChange().getDest().branch(),
+ rsrc.getChange().getKey().get()));
}
return createResponse(rsrc, result);
}
@@ -98,7 +100,7 @@
RevisionResource rsrc, List<CommitInfo> result) {
Response<List<CommitInfo>> r = Response.ok(result);
if (rsrc.isCacheable()) {
- r.caching(CacheControl.PRIVATE(7, TimeUnit.DAYS));
+ r.caching(CacheControl.PRIVATE(7, DAYS));
}
return r;
}
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 2dc401f..e8448b7 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
@@ -2555,8 +2555,9 @@
_resetReplyOverlayFocusStops() {
const dialog = query<GrReplyDialog>(this, '#replyDialog');
- if (!dialog) return;
- this.$.replyOverlay.setFocusStops(dialog.getFocusStops());
+ const focusStops = dialog?.getFocusStops();
+ if (!focusStops) return;
+ this.$.replyOverlay.setFocusStops(focusStops);
}
_handleToggleStar(e: CustomEvent<ChangeStarToggleStarDetail>) {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.ts
index d7b4ef3..2972d24 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.ts
@@ -101,12 +101,12 @@
test('submit blocked when invalid email is supplied to ccs', () => {
const sendStub = sinon.stub(element, 'send').returns(Promise.resolve());
- element.$.ccs.$.entry.setText('test');
+ element.$.ccs.entry!.setText('test');
MockInteractions.tap(queryAndAssert(element, 'gr-button.send'));
assert.isFalse(sendStub.called);
flush();
- element.$.ccs.$.entry.setText('test@test.test');
+ element.$.ccs.entry!.setText('test@test.test');
MockInteractions.tap(queryAndAssert(element, 'gr-button.send'));
assert.isTrue(sendStub.called);
});
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index b91538b..c7b1982 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -116,6 +116,7 @@
import {resolve, DIPolymerElement} from '../../../models/dependency';
import {changeModelToken} from '../../../models/change/change-model';
import {LabelNameToValuesMap} from '../../../api/rest-api';
+import {ValueChangedEvent} from '../../../types/events';
const STORAGE_DEBOUNCE_INTERVAL_MS = 400;
@@ -477,6 +478,7 @@
getFocusStops() {
const end = this._sendDisabled ? this.$.cancelButton : this.$.sendButton;
+ if (!this.$.reviewers.focusStart) return undefined;
return {
start: this.$.reviewers.focusStart,
end,
@@ -666,10 +668,10 @@
setTimeout(() => textarea.getNativeTextarea().focus());
} else if (section === FocusTarget.REVIEWERS) {
const reviewerEntry = this.$.reviewers.focusStart;
- setTimeout(() => reviewerEntry.focus());
+ setTimeout(() => reviewerEntry?.focus());
} else if (section === FocusTarget.CCS) {
const ccEntry = this.$.ccs.focusStart;
- setTimeout(() => ccEntry.focus());
+ setTimeout(() => ccEntry?.focus());
}
}
@@ -1265,6 +1267,33 @@
Object.keys(this.getLabelScores().getLabelValues(false)).length !== 0;
}
+ // To decouple account-list and reply dialog
+ _getAccountListCopy(list: (AccountInfo | GroupInfo)[]) {
+ return list.slice();
+ }
+
+ _handleReviewersChanged(e: ValueChangedEvent<(AccountInfo | GroupInfo)[]>) {
+ this._reviewers = e.detail.value.slice();
+ this._reviewersMutated = true;
+ }
+
+ _handleCcsChanged(e: ValueChangedEvent<(AccountInfo | GroupInfo)[]>) {
+ this._ccs = e.detail.value.slice();
+ this._reviewersMutated = true;
+ }
+
+ _handleReviewersConfirmationChanged(
+ e: ValueChangedEvent<SuggestedReviewerGroupInfo | null>
+ ) {
+ this._reviewerPendingConfirmation = e.detail.value;
+ }
+
+ _handleCcsConfirmationChanged(
+ e: ValueChangedEvent<SuggestedReviewerGroupInfo | null>
+ ) {
+ this._ccPendingConfirmation = e.detail.value;
+ }
+
_isState(knownLatestState?: LatestPatchState, value?: LatestPatchState) {
return knownLatestState === value;
}
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
index c1d7cb1..c4b3578 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
@@ -266,11 +266,13 @@
<div class="peopleListLabel">Reviewers</div>
<gr-account-list
id="reviewers"
- accounts="{{_reviewers}}"
+ accounts="[[_getAccountListCopy(_reviewers)]]"
on-account-added="accountAdded"
+ on-accounts-changed="_handleReviewersChanged"
removable-values="[[change.removable_reviewers]]"
filter="[[filterReviewerSuggestion]]"
- pending-confirmation="{{_reviewerPendingConfirmation}}"
+ pending-confirmation="[[_reviewerPendingConfirmation]]"
+ on-pending-confirmation-changed="_handleReviewersConfirmationChanged"
placeholder="Add reviewer..."
on-account-text-changed="_handleAccountTextEntry"
suggestions-provider="[[_getReviewerSuggestionsProvider(change)]]"
@@ -284,10 +286,12 @@
<div class="peopleListLabel">CC</div>
<gr-account-list
id="ccs"
- accounts="{{_ccs}}"
+ accounts="[[_getAccountListCopy(_ccs)]]"
on-account-added="accountAdded"
+ on-accounts-changed="_handleCcsChanged"
filter="[[filterCCSuggestion]]"
- pending-confirmation="{{_ccPendingConfirmation}}"
+ pending-confirmation="[[_ccPendingConfirmation]]"
+ pending-confirmation-changed="_handleCcsConfirmationChanged"
allow-any-input=""
placeholder="Add CC..."
on-account-text-changed="_handleAccountTextEntry"
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
index e324cf2d..883fbfa 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
@@ -1184,7 +1184,7 @@
// We should be focused on account entry input.
assert.isTrue(
isFocusInsideElement(
- queryAndAssert<GrAccountList>(element, '#reviewers').$.entry.$.input.$
+ queryAndAssert<GrAccountList>(element, '#reviewers').entry!.$.input.$
.input
)
);
@@ -1245,13 +1245,13 @@
if (cc) {
assert.isTrue(
isFocusInsideElement(
- queryAndAssert<GrAccountList>(element, '#ccs').$.entry.$.input.$.input
+ queryAndAssert<GrAccountList>(element, '#ccs').entry!.$.input.$.input
)
);
} else {
assert.isTrue(
isFocusInsideElement(
- queryAndAssert<GrAccountList>(element, '#reviewers').$.entry.$.input.$
+ queryAndAssert<GrAccountList>(element, '#reviewers').entry!.$.input.$
.input
)
);
@@ -1683,13 +1683,14 @@
const cc1 = makeAccount();
const cc2 = makeAccount();
const cc3 = makeAccount();
- element._reviewers = [reviewer1, reviewer2];
- element._ccs = [cc1, cc2, cc3];
-
element.change!.reviewers = {
[ReviewerState.CC]: [],
[ReviewerState.REVIEWER]: [{_account_id: 33 as AccountId}],
};
+ await flush();
+
+ element._reviewers = [reviewer1, reviewer2];
+ element._ccs = [cc1, cc2, cc3];
const mutations: ReviewerInput[] = [];
@@ -1697,6 +1698,8 @@
mutations.push(...review.reviewers!);
});
+ assert.isFalse(element._reviewersMutated);
+
// Remove and add to other field.
reviewers.dispatchEvent(
new CustomEvent('remove', {
@@ -1705,7 +1708,10 @@
bubbles: true,
})
);
- ccs.$.entry.dispatchEvent(
+
+ await flush();
+ assert.isTrue(element._reviewersMutated);
+ ccs.entry!.dispatchEvent(
new CustomEvent('add', {
detail: {value: {account: reviewer1}},
composed: true,
@@ -1726,7 +1732,7 @@
bubbles: true,
})
);
- reviewers.$.entry.dispatchEvent(
+ reviewers.entry!.dispatchEvent(
new CustomEvent('add', {
detail: {value: {account: cc1}},
composed: true,
@@ -1736,14 +1742,14 @@
// Add to other field without removing from former field.
// (Currently not possible in UI, but this is a good consistency check).
- reviewers.$.entry.dispatchEvent(
+ reviewers.entry!.dispatchEvent(
new CustomEvent('add', {
detail: {value: {account: cc2}},
composed: true,
bubbles: true,
})
);
- ccs.$.entry.dispatchEvent(
+ ccs.entry!.dispatchEvent(
new CustomEvent('add', {
detail: {value: {account: reviewer2}},
composed: true,
@@ -1766,6 +1772,7 @@
// Send and purge and verify moves, delete cc3.
await element.send(false, false);
+ await flush();
expect(mutations).to.have.lengthOf(5);
expect(mutations[0]).to.deep.equal(
mapReviewer(cc1, ReviewerState.REVIEWER)
@@ -1814,7 +1821,7 @@
bubbles: true,
})
);
- ccs.$.entry.dispatchEvent(
+ ccs.entry!.dispatchEvent(
new CustomEvent('add', {
detail: {value: {account: reviewer1}},
composed: true,
@@ -2216,7 +2223,7 @@
await flush();
assert.equal(
- element.getFocusStops().end,
+ element.getFocusStops()!.end,
queryAndAssert(element, '#cancelButton')
);
element.draftCommentThreads = [
@@ -2234,7 +2241,7 @@
await flush();
assert.equal(
- element.getFocusStops().end,
+ element.getFocusStops()!.end,
queryAndAssert(element, '#sendButton')
);
});
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
index 1553eef..cf24bcd 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
@@ -16,11 +16,7 @@
*/
import '../gr-account-chip/gr-account-chip';
import '../gr-account-entry/gr-account-entry';
-import '../../../styles/shared-styles';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-account-list_html';
import {getAppContext} from '../../../services/app-context';
-import {customElement, property} from '@polymer/decorators';
import {
ChangeInfo,
Suggestion,
@@ -30,20 +26,29 @@
SuggestedReviewerGroupInfo,
SuggestedReviewerAccountInfo,
} from '../../../types/common';
-import {
- ReviewerSuggestionsProvider,
- SuggestionItem,
-} from '../../../scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider';
+import {ReviewerSuggestionsProvider} from '../../../scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider';
import {GrAccountEntry} from '../gr-account-entry/gr-account-entry';
import {GrAccountChip} from '../gr-account-chip/gr-account-chip';
-import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {PaperInputElementExt} from '../../../types/types';
-import {fireAlert, fire} from '../../../utils/event-util';
+import {fire, fireAlert} from '../../../utils/event-util';
import {accountOrGroupKey} from '../../../utils/account-util';
+import {LitElement, css, html, PropertyValues} from 'lit';
+import {customElement, property, query, state} from 'lit/decorators';
+import {sharedStyles} from '../../../styles/shared-styles';
+import {classMap} from 'lit/directives/class-map';
+import {
+ AutocompleteQuery,
+ AutocompleteSuggestion,
+} from '../gr-autocomplete/gr-autocomplete';
+import {ValueChangedEvent} from '../../../types/events';
const VALID_EMAIL_ALERT = 'Please input a valid email.';
declare global {
+ interface HTMLElementEventMap {
+ 'accounts-changed': ValueChangedEvent<(AccountInfo | GroupInfo)[]>;
+ 'pending-confirmation-changed': ValueChangedEvent<SuggestedReviewerGroupInfo | null>;
+ }
interface HTMLElementTagNameMap {
'gr-account-list': GrAccountList;
}
@@ -51,13 +56,6 @@
'account-added': CustomEvent<AccountInputDetail>;
}
}
-
-export interface GrAccountList {
- $: {
- entry: GrAccountEntry;
- };
-}
-
export interface AccountInputDetail {
account: AccountInput;
}
@@ -115,18 +113,15 @@
}
@customElement('gr-account-list')
-export class GrAccountList extends PolymerElement {
- static get template() {
- return htmlTemplate;
- }
-
+export class GrAccountList extends LitElement {
/**
* Fired when user inputs an invalid email address.
*
* @event show-alert
*/
+ @query('#entry') entry?: GrAccountEntry;
- @property({type: Array, notify: true})
+ @property({type: Array})
accounts: AccountInput[] = [];
@property({type: Object})
@@ -135,7 +130,7 @@
@property({type: Object})
filter?: (input: Suggestion) => boolean;
- @property({type: String})
+ @property()
placeholder = '';
@property({type: Boolean})
@@ -150,7 +145,7 @@
/**
* Needed for template checking since value is initially set to null.
*/
- @property({type: Object, notify: true})
+ @property({type: Object})
pendingConfirmation: SuggestedReviewerGroupInfo | null = null;
@property({type: Boolean})
@@ -159,7 +154,7 @@
/**
* When true, allows for non-suggested inputs to be added.
*/
- @property({type: Boolean})
+ @property({type: Boolean, attribute: 'allow-any-input'})
allowAnyInput = false;
/**
@@ -175,8 +170,7 @@
/**
* Returns suggestion items
*/
- @property({type: Object})
- _querySuggestions: (input: string) => Promise<SuggestionItem[]>;
+ @state() private querySuggestions: AutocompleteQuery;
private readonly reporting = getAppContext().reportingService;
@@ -184,21 +178,92 @@
constructor() {
super();
- this._querySuggestions = input => this._getSuggestions(input);
+ this.querySuggestions = input => this.getSuggestions(input);
this.addEventListener('remove', e =>
- this._handleRemove(e as CustomEvent<{account: AccountInput}>)
+ this.handleRemove(e as CustomEvent<{account: AccountInput}>)
);
}
- get accountChips() {
- return Array.from(this.root?.querySelectorAll('gr-account-chip') || []);
+ static override styles = [
+ sharedStyles,
+ css`
+ gr-account-chip {
+ display: inline-block;
+ margin: var(--spacing-xs) var(--spacing-xs) var(--spacing-xs) 0;
+ }
+ gr-account-entry {
+ display: flex;
+ flex: 1;
+ min-width: 10em;
+ margin: var(--spacing-xs) var(--spacing-xs) var(--spacing-xs) 0;
+ }
+ .group {
+ --account-label-suffix: ' (group)';
+ }
+ .pending-add {
+ font-style: italic;
+ }
+ .list {
+ align-items: center;
+ display: flex;
+ flex-wrap: wrap;
+ }
+ `,
+ ];
+
+ override render() {
+ return html`<div class="list">
+ ${this.accounts.map(
+ account => html`
+ <gr-account-chip
+ .account=${account}
+ class=${classMap({
+ group: !!account._group,
+ pendingAdd: !!account._pendingAdd,
+ })}
+ ?removable=${this.computeRemovable(account)}
+ @keydown=${this.handleChipKeydown}
+ tabindex="-1"
+ >
+ </gr-account-chip>
+ `
+ )}
+ </div>
+ <gr-account-entry
+ borderless=""
+ ?hidden=${(this.maxCount && this.maxCount <= this.accounts.length) ||
+ this.readonly}
+ id="entry"
+ .placeholder=${this.placeholder}
+ @add=${this.handleAdd}
+ @keydown=${this.handleInputKeydown}
+ .allowAnyInput=${this.allowAnyInput}
+ .querySuggestions=${this.querySuggestions}
+ >
+ </gr-account-entry>
+ <slot></slot>`;
+ }
+
+ override willUpdate(changedProperties: PropertyValues) {
+ if (changedProperties.has('pendingConfirmation')) {
+ fire(this, 'pending-confirmation-changed', {
+ value: this.pendingConfirmation,
+ });
+ }
+ }
+
+ get accountChips(): GrAccountChip[] {
+ return Array.from(
+ this.shadowRoot?.querySelectorAll('gr-account-chip') || []
+ );
}
get focusStart() {
- return this.$.entry.focusStart;
+ // Entry is always defined and we cannot return undefined.
+ return this.entry?.focusStart;
}
- _getSuggestions(input: string) {
+ getSuggestions(input: string): Promise<AutocompleteSuggestion[]> {
const provider = this.suggestionsProvider;
if (!provider) return Promise.resolve([]);
return provider.getSuggestions(input).then(suggestions => {
@@ -212,8 +277,11 @@
});
}
- _handleAdd(e: CustomEvent<{value: RawAccountInput}>) {
- this.addAccountItem(e.detail.value);
+ // private but used in test
+ handleAdd(e: ValueChangedEvent<string>) {
+ // TODO(TS) this is temporary hack to avoid cascade of ts issues
+ const item = e.detail.value as RawAccountInput;
+ this.addAccountItem(item);
}
addAccountItem(item: RawAccountInput) {
@@ -226,7 +294,7 @@
if (isAccountObject(item)) {
account = {...item.account, _pendingAdd: true};
this.removeFromPendingRemoval(account);
- this.push('accounts', account);
+ this.accounts.push(account);
itemTypeAdded = 'account';
} else if (isSuggestedReviewerGroupInfo(item)) {
if (item.confirm) {
@@ -234,41 +302,45 @@
return;
}
group = {...item.group, _pendingAdd: true, _group: true};
- this.push('accounts', group);
+ this.accounts.push(group);
this.removeFromPendingRemoval(group);
itemTypeAdded = 'group';
} else if (this.allowAnyInput) {
if (!item.includes('@')) {
// Repopulate the input with what the user tried to enter and have
// a toast tell them why they can't enter it.
- this.$.entry.setText(item);
+ this.entry?.setText(item);
fireAlert(this, VALID_EMAIL_ALERT);
return false;
} else {
account = {email: item as EmailAddress, _pendingAdd: true};
- this.push('accounts', account);
+ this.accounts.push(account);
this.removeFromPendingRemoval(account);
itemTypeAdded = 'email';
}
}
-
+ fire(this, 'accounts-changed', {value: this.accounts.slice()});
fire(this, 'account-added', {account: (account ?? group)! as AccountInput});
this.reporting.reportInteraction(`Add to ${this.id}`, {itemTypeAdded});
this.pendingConfirmation = null;
+ this.requestUpdate();
return true;
}
confirmGroup(group: GroupInfo) {
- this.push('accounts', {
+ this.accounts.push({
...group,
confirmed: true,
_pendingAdd: true,
_group: true,
});
this.pendingConfirmation = null;
+ fire(this, 'accounts-changed', {value: this.accounts});
+ this.requestUpdate();
}
- _computeChipClass(account: AccountInput) {
+ // private but used in test
+ computeChipClass(account: AccountInput) {
const classes = [];
if (account._group) {
classes.push('group');
@@ -279,8 +351,9 @@
return classes.join(' ');
}
- _computeRemovable(account: AccountInput, readonly: boolean) {
- if (readonly) {
+ // private but used in test
+ computeRemovable(account: AccountInput) {
+ if (this.readonly) {
return false;
}
if (this.removableValues) {
@@ -297,21 +370,23 @@
return true;
}
- _handleRemove(e: CustomEvent<{account: AccountInput}>) {
+ private handleRemove(e: CustomEvent<{account: AccountInput}>) {
const toRemove = e.detail.account;
this.removeAccount(toRemove);
- this.$.entry.focus();
+ this.entry?.focus();
}
removeAccount(toRemove?: AccountInput) {
- if (!toRemove || !this._computeRemovable(toRemove, this.readonly)) {
+ if (!toRemove || !this.computeRemovable(toRemove)) {
return;
}
for (let i = 0; i < this.accounts.length; i++) {
if (accountOrGroupKey(toRemove) === accountOrGroupKey(this.accounts[i])) {
- this.splice('accounts', i, 1);
+ this.accounts.splice(i, 1);
this.pendingRemoval.add(toRemove);
this.reporting.reportInteraction(`Remove from ${this.id}`);
+ this.requestUpdate();
+ fire(this, 'accounts-changed', {value: this.accounts.slice()});
return;
}
}
@@ -320,23 +395,23 @@
);
}
- _getNativeInput(paperInput: PaperInputElementExt) {
+ // private but used in test
+ getOwnNativeInput(paperInput: PaperInputElementExt) {
// In Polymer 2 inputElement isn't nativeInput anymore
return (paperInput.$.nativeInput ||
paperInput.inputElement) as HTMLTextAreaElement;
}
- _handleInputKeydown(
- e: CustomEvent<{input: PaperInputElementExt; keyCode: number}>
- ) {
- const input = this._getNativeInput(e.detail.input);
+ private handleInputKeydown(e: KeyboardEvent) {
+ const target = e.target as GrAccountEntry;
+ const input = this.getOwnNativeInput(target.$.input.$.input);
if (
input.selectionStart !== input.selectionEnd ||
input.selectionStart !== 0
) {
return;
}
- switch (e.detail.keyCode) {
+ switch (e.keyCode) {
case 8: // Backspace
this.removeAccount(this.accounts[this.accounts.length - 1]);
break;
@@ -348,7 +423,7 @@
}
}
- _handleChipKeydown(e: KeyboardEvent) {
+ private handleChipKeydown(e: KeyboardEvent) {
const chip = e.target as GrAccountChip;
const chips = this.accountChips;
const index = chips.indexOf(chip);
@@ -366,7 +441,7 @@
} else if (index > 0) {
chips[index - 1].focus();
} else {
- this.$.entry.focus();
+ this.entry?.focus();
}
break;
case 37: // Left arrow
@@ -380,7 +455,7 @@
if (index < chips.length - 1) {
chips[index + 1].focus();
} else {
- this.$.entry.focus();
+ this.entry?.focus();
}
break;
}
@@ -395,13 +470,13 @@
* return true.
*/
submitEntryText() {
- const text = this.$.entry.getText();
- if (!text.length) {
+ const text = this.entry?.getText();
+ if (!text?.length) {
return true;
}
const wasSubmitted = this.addAccountItem(text);
if (wasSubmitted) {
- this.$.entry.clear();
+ this.entry?.clear();
}
return wasSubmitted;
}
@@ -432,19 +507,11 @@
});
}
- removeFromPendingRemoval(account: AccountInput) {
+ private removeFromPendingRemoval(account: AccountInput) {
this.pendingRemoval.delete(account);
}
clearPendingRemovals() {
this.pendingRemoval.clear();
}
-
- _computeEntryHidden(
- maxCount: number,
- accountsRecord: PolymerDeepPropertyChange<AccountInput[], AccountInput[]>,
- readonly: boolean
- ) {
- return (maxCount && maxCount <= accountsRecord.base.length) || readonly;
- }
}
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_html.ts b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_html.ts
deleted file mode 100644
index 7a47e29..0000000
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_html.ts
+++ /dev/null
@@ -1,72 +0,0 @@
-/**
- * @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.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style include="shared-styles">
- gr-account-chip {
- display: inline-block;
- margin: var(--spacing-xs) var(--spacing-xs) var(--spacing-xs) 0;
- }
- gr-account-entry {
- display: flex;
- flex: 1;
- min-width: 10em;
- margin: var(--spacing-xs) var(--spacing-xs) var(--spacing-xs) 0;
- }
- .group {
- --account-label-suffix: ' (group)';
- }
- .pending-add {
- font-style: italic;
- }
- .list {
- align-items: center;
- display: flex;
- flex-wrap: wrap;
- }
- </style>
- <!--
- NOTE(Issue 6419): Nest the inner dom-repeat template in a div rather than
- as a direct child of the dom-module's template.
- -->
- <div class="list">
- <template id="chips" is="dom-repeat" items="[[accounts]]" as="account">
- <gr-account-chip
- account="[[account]]"
- class$="[[_computeChipClass(account)]]"
- data-account-id$="[[account._account_id]]"
- removable="[[_computeRemovable(account, readonly)]]"
- on-keydown="_handleChipKeydown"
- tabindex="-1"
- >
- </gr-account-chip>
- </template>
- </div>
- <gr-account-entry
- borderless=""
- hidden$="[[_computeEntryHidden(maxCount, accounts.*, readonly)]]"
- id="entry"
- placeholder="[[placeholder]]"
- on-add="_handleAdd"
- on-input-keydown="_handleInputKeydown"
- allow-any-input="[[allowAnyInput]]"
- query-suggestions="[[_querySuggestions]]"
- >
- </gr-account-entry>
- <slot></slot>
-`;
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts
index d77dec3..7509023 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts
@@ -28,7 +28,6 @@
GroupBaseInfo,
GroupId,
GroupName,
- SuggestedReviewerAccountInfo,
Suggestion,
} from '../../../types/common';
import {queryAll} from '../../../test/test-utils';
@@ -52,7 +51,7 @@
_account_id: 1 as AccountId,
} as AccountInfo,
count: 1,
- } as SuggestedReviewerAccountInfo,
+ } as unknown as string,
};
}
}
@@ -85,12 +84,14 @@
}
function handleAdd(value: RawAccountInput) {
- element._handleAdd(
- new CustomEvent<{value: RawAccountInput}>('add', {detail: {value}})
+ element.handleAdd(
+ new CustomEvent<{value: string}>('add', {
+ detail: {value: value as unknown as string},
+ })
);
}
- setup(() => {
+ setup(async () => {
existingAccount1 = makeAccount();
existingAccount2 = makeAccount();
@@ -98,18 +99,33 @@
element.accounts = [existingAccount1, existingAccount2];
suggestionsProvider = new MockSuggestionsProvider();
element.suggestionsProvider = suggestionsProvider;
+ await element.updateComplete;
});
- test('account entry only appears when editable', () => {
+ test('renders', () => {
+ expect(element).shadowDom.to.equal(
+ /* HTML */
+ `<div class="list">
+ <gr-account-chip removable="" tabindex="-1"> </gr-account-chip>
+ <gr-account-chip removable="" tabindex="-1"> </gr-account-chip>
+ </div>
+ <gr-account-entry borderless="" id="entry"></gr-account-entry>
+ <slot></slot>`
+ );
+ });
+
+ test('account entry only appears when editable', async () => {
element.readonly = false;
- assert.isFalse(element.$.entry.hasAttribute('hidden'));
+ await element.updateComplete;
+ assert.isFalse(element.entry!.hasAttribute('hidden'));
element.readonly = true;
- assert.isTrue(element.$.entry.hasAttribute('hidden'));
+ await element.updateComplete;
+ assert.isTrue(element.entry!.hasAttribute('hidden'));
});
- test('addition and removal of account/group chips', () => {
- flush();
- sinon.stub(element, '_computeRemovable').returns(true);
+ test('addition and removal of account/group chips', async () => {
+ await element.updateComplete;
+ sinon.stub(element, 'computeRemovable').returns(true);
// Existing accounts are listed.
let chips = getChips();
assert.equal(chips.length, 2);
@@ -119,7 +135,7 @@
// New accounts are added to end with pendingAdd class.
const newAccount = makeAccount();
handleAdd({account: newAccount, count: 1});
- flush();
+ await element.updateComplete;
chips = getChips();
assert.equal(chips.length, 3);
assert.isFalse(chips[0].classList.contains('pendingAdd'));
@@ -134,7 +150,7 @@
bubbles: true,
})
);
- flush();
+ await element.updateComplete;
chips = getChips();
assert.equal(chips.length, 2);
assert.isFalse(chips[0].classList.contains('pendingAdd'));
@@ -155,7 +171,7 @@
bubbles: true,
})
);
- flush();
+ await element.updateComplete;
chips = getChips();
assert.equal(chips.length, 1);
assert.isFalse(chips[0].classList.contains('pendingAdd'));
@@ -163,7 +179,7 @@
// New groups are added to end with pendingAdd and group classes.
const newGroup = makeGroup();
handleAdd({group: newGroup, confirm: false, count: 1});
- flush();
+ await element.updateComplete;
chips = getChips();
assert.equal(chips.length, 2);
assert.isTrue(chips[1].classList.contains('group'));
@@ -177,13 +193,13 @@
bubbles: true,
})
);
- flush();
+ await element.updateComplete;
chips = getChips();
assert.equal(chips.length, 1);
assert.isFalse(chips[0].classList.contains('pendingAdd'));
});
- test('_getSuggestions uses filter correctly', () => {
+ test('getSuggestions uses filter correctly', () => {
const originalSuggestions: Suggestion[] = [
{
email: 'abc@example.com' as EmailAddress,
@@ -212,12 +228,12 @@
value: {
account: suggestion as AccountInfo,
count: 1,
- },
+ } as unknown as string,
};
});
return element
- ._getSuggestions('')
+ .getSuggestions('')
.then(suggestions => {
// Default is no filtering.
assert.equal(suggestions.length, 3);
@@ -228,7 +244,7 @@
return (suggestion as AccountInfo)._account_id === accountId;
};
- return element._getSuggestions('');
+ return element.getSuggestions('');
})
.then(suggestions => {
assert.deepEqual(suggestions, [
@@ -237,52 +253,55 @@
value: {
account: originalSuggestions[0] as AccountInfo,
count: 1,
- },
+ } as unknown as string,
},
]);
});
});
- test('_computeChipClass', () => {
+ test('computeChipClass', () => {
const account = makeAccount() as AccountInfoInput;
- assert.equal(element._computeChipClass(account), '');
+ assert.equal(element.computeChipClass(account), '');
account._pendingAdd = true;
- assert.equal(element._computeChipClass(account), 'pendingAdd');
+ assert.equal(element.computeChipClass(account), 'pendingAdd');
account._group = true;
- assert.equal(element._computeChipClass(account), 'group pendingAdd');
+ assert.equal(element.computeChipClass(account), 'group pendingAdd');
account._pendingAdd = false;
- assert.equal(element._computeChipClass(account), 'group');
+ assert.equal(element.computeChipClass(account), 'group');
});
- test('_computeRemovable', () => {
+ test('computeRemovable', async () => {
const newAccount = makeAccount() as AccountInfoInput;
newAccount._pendingAdd = true;
element.readonly = false;
element.removableValues = [];
- assert.isFalse(element._computeRemovable(existingAccount1, false));
- assert.isTrue(element._computeRemovable(newAccount, false));
+ element.updateComplete;
+ assert.isFalse(element.computeRemovable(existingAccount1));
+ assert.isTrue(element.computeRemovable(newAccount));
element.removableValues = [existingAccount1];
- assert.isTrue(element._computeRemovable(existingAccount1, false));
- assert.isTrue(element._computeRemovable(newAccount, false));
- assert.isFalse(element._computeRemovable(existingAccount2, false));
+ element.updateComplete;
+ assert.isTrue(element.computeRemovable(existingAccount1));
+ assert.isTrue(element.computeRemovable(newAccount));
+ assert.isFalse(element.computeRemovable(existingAccount2));
element.readonly = true;
- assert.isFalse(element._computeRemovable(existingAccount1, true));
- assert.isFalse(element._computeRemovable(newAccount, true));
+ element.updateComplete;
+ assert.isFalse(element.computeRemovable(existingAccount1));
+ assert.isFalse(element.computeRemovable(newAccount));
});
- test('submitEntryText', () => {
+ test('submitEntryText', async () => {
element.allowAnyInput = true;
- flush();
+ await element.updateComplete;
- const getTextStub = sinon.stub(element.$.entry, 'getText');
+ const getTextStub = sinon.stub(element.entry!, 'getText');
getTextStub.onFirstCall().returns('');
getTextStub.onSecondCall().returns('test');
getTextStub.onThirdCall().returns('test@test');
// When entry is empty, return true.
- const clearStub = sinon.stub(element.$.entry, 'clear');
+ const clearStub = sinon.stub(element.entry!, 'clear');
assert.isTrue(element.submitEntryText());
assert.isFalse(clearStub.called);
@@ -363,12 +382,12 @@
assert.equal(element.accounts.length, 1);
});
- test('max-count', () => {
+ test('max-count', async () => {
element.maxCount = 1;
const acct = makeAccount();
handleAdd({account: acct, count: 1});
- flush();
- assert.isTrue(element.$.entry.hasAttribute('hidden'));
+ await element.updateComplete;
+ assert.isTrue(element.entry!.hasAttribute('hidden'));
});
test('enter text calls suggestions provider', async () => {
@@ -391,12 +410,12 @@
'makeSuggestionItem'
);
- const input = element.$.entry.$.input;
+ const input = element.entry!.$.input;
input.text = 'newTest';
MockInteractions.focus(input.$.input);
input.noDebounce = true;
- await flush();
+ await element.updateComplete;
assert.isTrue(getSuggestionsStub.calledOnce);
assert.equal(getSuggestionsStub.lastCall.args[0], 'newTest');
assert.equal(makeSuggestionItemSpy.getCalls().length, 2);
@@ -427,38 +446,38 @@
suite('keyboard interactions', () => {
test('backspace at text input start removes last account', async () => {
- const input = element.$.entry.$.input;
+ const input = element.entry!.$.input;
sinon.stub(input, '_updateSuggestions');
- sinon.stub(element, '_computeRemovable').returns(true);
- await flush();
+ sinon.stub(element, 'computeRemovable').returns(true);
+ await await element.updateComplete;
// Next line is a workaround for Firefox not moving cursor
// on input field update
- assert.equal(element._getNativeInput(input.$.input).selectionStart, 0);
+ assert.equal(element.getOwnNativeInput(input.$.input).selectionStart, 0);
input.text = 'test';
MockInteractions.focus(input.$.input);
- flush();
+ await element.updateComplete;
assert.equal(element.accounts.length, 2);
MockInteractions.pressAndReleaseKeyOn(
- element._getNativeInput(input.$.input),
+ element.getOwnNativeInput(input.$.input),
8
); // Backspace
assert.equal(element.accounts.length, 2);
input.text = '';
MockInteractions.pressAndReleaseKeyOn(
- element._getNativeInput(input.$.input),
+ element.getOwnNativeInput(input.$.input),
8
); // Backspace
- flush();
+ await element.updateComplete;
assert.equal(element.accounts.length, 1);
});
test('arrow key navigation', async () => {
- const input = element.$.entry.$.input;
+ const input = element.entry!.$.input;
input.text = '';
element.accounts = [makeAccount(), makeAccount()];
- flush();
+ await element.updateComplete;
MockInteractions.focus(input.$.input);
- await flush();
+ await await element.updateComplete;
const chips = element.accountChips;
const chipsOneSpy = sinon.spy(chips[1], 'focus');
MockInteractions.pressAndReleaseKeyOn(input.$.input, 37); // Left
@@ -472,9 +491,9 @@
assert.isTrue(chipsOneSpy.calledTwice);
});
- test('delete', () => {
+ test('delete', async () => {
element.accounts = [makeAccount(), makeAccount()];
- flush();
+ await element.updateComplete;
const focusSpy = sinon.spy(element.accountChips[1], 'focus');
const removeSpy = sinon.spy(element, 'removeAccount');
MockInteractions.pressAndReleaseKeyOn(element.accountChips[0], 8); // Backspace
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
index a685f32..36baf87 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
@@ -54,7 +54,7 @@
name?: string;
label?: string;
value?: T;
- text?: T;
+ text?: string;
}
export interface AutocompleteCommitEventDetail {
diff --git a/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts b/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts
index a74adf6..78cff25 100644
--- a/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts
+++ b/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts
@@ -25,10 +25,10 @@
isReviewerGroupSuggestion,
NumericChangeId,
ServerInfo,
- SuggestedReviewerInfo,
Suggestion,
} from '../../types/common';
import {assertNever} from '../../utils/common-util';
+import {AutocompleteSuggestion} from '../../elements/shared/gr-autocomplete/gr-autocomplete';
// TODO(TS): enum name doesn't follow typescript style guid rules
// Rename it
@@ -44,15 +44,10 @@
type ApiCallCallback = (input: string) => Promise<Suggestion[] | void>;
-export interface SuggestionItem {
- name: string;
- value: SuggestedReviewerInfo;
-}
-
export interface ReviewerSuggestionsProvider {
init(): void;
getSuggestions(input: string): Promise<Suggestion[]>;
- makeSuggestionItem(suggestion: Suggestion): SuggestionItem;
+ makeSuggestionItem(suggestion: Suggestion): AutocompleteSuggestion;
}
export class GrReviewerSuggestionsProvider
@@ -120,12 +115,15 @@
return this._apiCall(input).then(reviewers => reviewers || []);
}
- makeSuggestionItem(suggestion: Suggestion): SuggestionItem {
+ // this can be retyped to AutocompleteSuggestion<SuggestedReviewerInfo> but
+ // this would need to change generics of gr-autocomplete.
+ makeSuggestionItem(suggestion: Suggestion): AutocompleteSuggestion {
if (isReviewerAccountSuggestion(suggestion)) {
// Reviewer is an account suggestion from getChangeSuggestedReviewers.
return {
name: getAccountDisplayName(this.config, suggestion.account),
- value: suggestion,
+ // TODO(TS) this is temporary hack to avoid cascade of ts issues
+ value: suggestion as unknown as string,
};
}
@@ -133,7 +131,8 @@
// Reviewer is a group suggestion from getChangeSuggestedReviewers.
return {
name: getGroupDisplayName(suggestion.group),
- value: suggestion,
+ // TODO(TS) this is temporary hack to avoid cascade of ts issues
+ value: suggestion as unknown as string,
};
}
@@ -141,7 +140,8 @@
// Reviewer is an account suggestion from getSuggestedAccounts.
return {
name: getAccountDisplayName(this.config, suggestion),
- value: {account: suggestion, count: 1},
+ // TODO(TS) this is temporary hack to avoid cascade of ts issues
+ value: {account: suggestion, count: 1} as unknown as string,
};
}
assertNever(suggestion, 'Received an incorrect suggestion');
