Convert GrReplyDialog to Lit Release-Notes: skip Change-Id: I2e923e357749e87d8c254148477edc3aa1303dad
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 13cead6..5bd5d08 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
@@ -90,8 +90,8 @@ <gr-reply-dialog></gr-reply-dialog> `); setupElement(element); - // Allow the elements created by dom-repeat to be stamped. - flush(); + + await element.updateComplete; }); teardown(() => { @@ -101,13 +101,13 @@ 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.ccsList!.entry!.setText('test'); MockInteractions.tap(queryAndAssert(element, 'gr-button.send')); - assert.isFalse(element.$.ccs.submitEntryText()); + assert.isFalse(element.ccsList!.submitEntryText()); assert.isFalse(sendStub.called); flush(); - element.$.ccs.entry!.setText('test@test.test'); + element.ccsList!.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 c7b1982..f76725a 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
@@ -27,7 +27,6 @@ import '../gr-label-scores/gr-label-scores'; import '../gr-thread-list/gr-thread-list'; import '../../../styles/shared-styles'; -import {htmlTemplate} from './gr-reply-dialog_html'; import { GrReviewerSuggestionsProvider, SUGGESTIONS_PROVIDERS_USERS_TYPES, @@ -47,11 +46,11 @@ } from '../../../utils/account-util'; import {IronA11yAnnouncer} from '@polymer/iron-a11y-announcer/iron-a11y-announcer'; import {TargetElement} from '../../../api/plugin'; -import {customElement, observe, property} from '@polymer/decorators'; import {FixIronA11yAnnouncer} from '../../../types/types'; import { AccountAddition, AccountInfoInput, + AccountInput, AccountInputDetail, GrAccountList, GroupInfoInput, @@ -70,9 +69,7 @@ isReviewerGroupSuggestion, ParsedJSON, PatchSetNum, - ProjectInfo, ReviewerInput, - Reviewers, ReviewInput, ReviewResult, ServerInfo, @@ -82,7 +79,6 @@ import {GrButton} from '../../shared/gr-button/gr-button'; import {GrLabelScores} from '../gr-label-scores/gr-label-scores'; import {GrLabelScoreRow} from '../gr-label-score-row/gr-label-score-row'; -import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces'; import { areSetsEqual, assertIsDefined, @@ -113,10 +109,15 @@ import {getReplyByReason} from '../../../utils/attention-set-util'; import {addShortcut, Key, Modifier} from '../../../utils/dom-util'; import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api'; -import {resolve, DIPolymerElement} from '../../../models/dependency'; +import {resolve} from '../../../models/dependency'; import {changeModelToken} from '../../../models/change/change-model'; -import {LabelNameToValuesMap} from '../../../api/rest-api'; -import {ValueChangedEvent} from '../../../types/events'; +import {ConfigInfo, LabelNameToValuesMap} from '../../../api/rest-api'; +import {css, html, PropertyValues, LitElement} from 'lit'; +import {sharedStyles} from '../../../styles/shared-styles'; +import {when} from 'lit/directives/when'; +import {classMap} from 'lit/directives/class-map'; +import {BindValueChangeEvent, ValueChangedEvent} from '../../../types/events'; +import {customElement, property, state, query} from 'lit/decorators'; const STORAGE_DEBOUNCE_INTERVAL_MS = 400; @@ -152,24 +153,8 @@ const EMPTY_REPLY_MESSAGE = 'Cannot send an empty reply.'; -export interface GrReplyDialog { - $: { - reviewers: GrAccountList; - ccs: GrAccountList; - cancelButton: GrButton; - sendButton: GrButton; - labelScores: GrLabelScores; - textarea: GrTextarea; - reviewerConfirmationOverlay: GrOverlay; - }; -} - @customElement('gr-reply-dialog') -export class GrReplyDialog extends DIPolymerElement { - static get template() { - return htmlTemplate; - } - +export class GrReplyDialog extends LitElement { /** * Fired when a reply is successfully sent. * @@ -229,131 +214,132 @@ @property({type: Boolean}) canBeStarted = false; - @property({type: Boolean, reflectToAttribute: true}) + @property({type: Boolean, reflect: true}) disabled = false; - @property({ - type: Boolean, - computed: '_computeHasDrafts(draft, draftCommentThreads.*)', - }) - hasDrafts = false; - - @property({type: String, observer: '_draftChanged'}) - draft = ''; - - @property({type: Object}) - filterReviewerSuggestion: (input: Suggestion) => boolean; - - @property({type: Object}) - filterCCSuggestion: (input: Suggestion) => boolean; + @property({type: Array}) + draftCommentThreads: CommentThread[] | undefined; @property({type: Object}) permittedLabels?: LabelNameToValuesMap; @property({type: Object}) - projectConfig?: ProjectInfo; + projectConfig?: ConfigInfo; @property({type: Object}) serverConfig?: ServerInfo; - @property({type: String}) + @query('#reviewers') reviewersList?: GrAccountList; + + @query('#ccs') ccsList?: GrAccountList; + + @query('#cancelButton') cancelButton?: GrButton; + + @query('#sendButton') sendButton?: GrButton; + + @query('#labelScores') labelScores?: GrLabelScores; + + @query('#textarea') textarea?: GrTextarea; + + @query('#reviewerConfirmationOverlay') + reviewerConfirmationOverlay?: GrOverlay; + + @state() + hasDrafts = false; + + @state() + draft = ''; + + @state() + filterReviewerSuggestion: (input: Suggestion) => boolean; + + @state() + filterCCSuggestion: (input: Suggestion) => boolean; + + @state() knownLatestState?: LatestPatchState; - @property({type: Boolean}) + @state() underReview = true; - @property({type: Object}) - _account?: AccountInfo; + @state() + account?: AccountInfo; - @property({type: Array}) - _ccs: (AccountInfo | GroupInfo)[] = []; + @state() + ccs: (AccountInfoInput | GroupInfoInput)[] = []; - @property({type: Number}) - _attentionCcsCount = 0; + @state() + attentionCcsCount = 0; - @property({type: Object, observer: '_reviewerPendingConfirmationUpdated'}) - _ccPendingConfirmation: SuggestedReviewerGroupInfo | null = null; + @state() + ccPendingConfirmation: SuggestedReviewerGroupInfo | null = null; - @property({ - type: String, - computed: '_computeMessagePlaceholder(canBeStarted)', - }) - _messagePlaceholder?: string; + @state() + messagePlaceholder?: string; - @property({type: Object}) - _owner?: AccountInfo; + @state() + owner?: AccountInfo; - @property({type: Object, computed: '_computeUploader(change)'}) - _uploader?: AccountInfo; + @state() + uploader?: AccountInfo; - @property({type: Object}) - _pendingConfirmationDetails: SuggestedReviewerGroupInfo | null = null; + @state() + pendingConfirmationDetails: SuggestedReviewerGroupInfo | null = null; - @property({type: Boolean}) - _includeComments = true; + @state() + includeComments = true; - @property({type: Array}) - _reviewers: (AccountInfo | GroupInfo)[] = []; + @state() reviewers: AccountInput[] = []; - @property({type: Object, observer: '_reviewerPendingConfirmationUpdated'}) - _reviewerPendingConfirmation: SuggestedReviewerGroupInfo | null = null; + @state() + reviewerPendingConfirmation: SuggestedReviewerGroupInfo | null = null; - @property({type: Boolean, observer: '_handleHeightChanged'}) - _previewFormatting = false; + @state() + previewFormatting = false; - @property({type: String, computed: '_computeSendButtonLabel(canBeStarted)'}) - _sendButtonLabel?: string; + @state() + sendButtonLabel?: string; - @property({type: Boolean}) - _savingComments = false; + @state() + savingComments = false; - @property({type: Boolean}) - _reviewersMutated = false; + @state() + reviewersMutated = false; /** * Signifies that the user has changed their vote on a label or (if they have * not yet voted on a label) if a selected vote is different from the default * vote. */ - @property({type: Boolean}) - _labelsChanged = false; + @state() + labelsChanged = false; - @property({type: String}) - readonly _saveTooltip: string = ButtonTooltips.SAVE; + @state() + readonly saveTooltip: string = ButtonTooltips.SAVE; - @property({type: String}) - _pluginMessage = ''; + @state() + pluginMessage = ''; - @property({type: Boolean}) - _commentEditing = false; + @state() + commentEditing = false; - @property({type: Boolean}) - _attentionExpanded = false; + @state() + attentionExpanded = false; - @property({type: Object}) - _currentAttentionSet: Set<AccountId> = new Set(); + @state() + currentAttentionSet: Set<AccountId> = new Set(); - @property({type: Object}) - _newAttentionSet: Set<AccountId> = new Set(); + @state() + newAttentionSet: Set<AccountId> = new Set(); - @property({ - type: Boolean, - computed: - '_computeSendButtonDisabled(canBeStarted, ' + - 'draftCommentThreads, draft, _reviewersMutated, _labelsChanged, ' + - '_includeComments, disabled, _commentEditing, change, _account)', - observer: '_sendDisabledChanged', - }) - _sendDisabled?: boolean; + @state() + sendDisabled?: boolean; - @property({type: Array, observer: '_handleHeightChanged'}) - draftCommentThreads: CommentThread[] | undefined; + @state() + isResolvedPatchsetLevelComment = true; - @property({type: Boolean}) - _isResolvedPatchsetLevelComment = true; - - @property({type: Array, computed: '_computeAllReviewers(_reviewers.*)'}) - _allReviewers: (AccountInfo | GroupInfo)[] = []; + @state() + allReviewers: (AccountInfo | GroupInfo)[] = []; private readonly restApiService: RestApiService = getAppContext().restApiService; @@ -362,16 +348,312 @@ private readonly jsAPI = getAppContext().jsApiService; - private storeTask?: DelayedTask; + storeTask?: DelayedTask; /** Called in disconnectedCallback. */ private cleanups: (() => void)[] = []; + static override styles = [ + sharedStyles, + css` + :host { + background-color: var(--dialog-background-color); + display: block; + max-height: 90vh; + } + :host([disabled]) { + pointer-events: none; + } + :host([disabled]) .container { + opacity: 0.5; + } + section { + border-top: 1px solid var(--border-color); + flex-shrink: 0; + padding: var(--spacing-m) var(--spacing-xl); + width: 100%; + } + section.labelsContainer { + /* We want the :hover highlight to extend to the border of the dialog. */ + padding: var(--spacing-m) 0; + } + .stickyBottom { + background-color: var(--dialog-background-color); + box-shadow: 0px 0px 8px 0px rgba(60, 64, 67, 0.15); + margin-top: var(--spacing-s); + bottom: 0; + position: sticky; + /* @see Issue 8602 */ + z-index: 1; + } + .stickyBottom.newReplyDialog { + margin-top: unset; + } + .actions { + display: flex; + justify-content: space-between; + } + .actions .right gr-button { + margin-left: var(--spacing-l); + } + .peopleContainer, + .labelsContainer { + flex-shrink: 0; + } + .peopleContainer { + border-top: none; + display: table; + } + .peopleList { + display: flex; + } + .peopleListLabel { + color: var(--deemphasized-text-color); + margin-top: var(--spacing-xs); + min-width: 6em; + padding-right: var(--spacing-m); + } + gr-account-list { + display: flex; + flex-wrap: wrap; + flex: 1; + } + #reviewerConfirmationOverlay { + padding: var(--spacing-l); + text-align: center; + } + .reviewerConfirmationButtons { + margin-top: var(--spacing-l); + } + .groupName { + font-weight: var(--font-weight-bold); + } + .groupSize { + font-style: italic; + } + .textareaContainer { + min-height: 12em; + position: relative; + } + .newReplyDialog.textareaContainer { + min-height: unset; + } + textareaContainer, + #textarea, + gr-endpoint-decorator[name='reply-text'] { + display: flex; + width: 100%; + } + .newReplyDialog .textareaContainer, + #textarea, + gr-endpoint-decorator[name='reply-text'] { + display: block; + width: unset; + font-family: var(--monospace-font-family); + font-size: var(--font-size-code); + line-height: calc(var(--font-size-code) + var(--spacing-s)); + font-weight: var(--font-weight-normal); + } + .newReplyDialog#textarea { + padding: var(--spacing-m); + } + gr-endpoint-decorator[name='reply-text'] { + flex-direction: column; + } + #textarea { + flex: 1; + } + .previewContainer { + border-top: none; + } + .previewContainer gr-formatted-text { + background: var(--table-header-background-color); + padding: var(--spacing-l); + } + #checkingStatusLabel, + #notLatestLabel { + margin-left: var(--spacing-l); + } + #checkingStatusLabel { + color: var(--deemphasized-text-color); + font-style: italic; + } + #notLatestLabel, + #savingLabel { + color: var(--error-text-color); + } + #savingLabel { + display: none; + } + #savingLabel.saving { + display: inline; + } + #pluginMessage { + color: var(--deemphasized-text-color); + margin-left: var(--spacing-l); + margin-bottom: var(--spacing-m); + } + #pluginMessage:empty { + display: none; + } + .preview-formatting { + margin-left: var(--spacing-m); + } + .attention-icon { + width: 14px; + height: 14px; + vertical-align: top; + position: relative; + top: 3px; + --iron-icon-height: 24px; + --iron-icon-width: 24px; + } + .attention .edit-attention-button { + vertical-align: top; + --gr-button-padding: 0px 4px; + } + .attention .edit-attention-button iron-icon { + color: inherit; + } + .attention a, + .attention-detail a { + text-decoration: none; + } + .attentionSummary { + display: flex; + justify-content: space-between; + } + .attentionSummary { + /* The account label for selection is misbehaving currently: It consumes + 26px height instead of 20px, which is the default line-height and thus + the max that can be nicely fit into an inline layout flow. We + acknowledge that using a fixed 26px value here is a hack and not a + great solution. */ + line-height: 26px; + } + .attentionSummary gr-account-label, + .attention-detail gr-account-label { + --account-max-length: 120px; + display: inline-block; + padding: var(--spacing-xs) var(--spacing-m); + user-select: none; + --label-border-radius: 8px; + } + .attentionSummary gr-account-label { + margin: 0 var(--spacing-xs); + line-height: var(--line-height-normal); + vertical-align: top; + } + .attention-detail .peopleListValues { + line-height: calc(var(--line-height-normal) + 10px); + } + .attention-detail gr-account-label { + line-height: var(--line-height-normal); + } + .attentionSummary gr-account-label:focus, + .attention-detail gr-account-label:focus { + outline: none; + } + .attentionSummary gr-account-label:hover, + .attention-detail gr-account-label:hover { + box-shadow: var(--elevation-level-1); + cursor: pointer; + } + .attention-detail .attentionDetailsTitle { + display: flex; + justify-content: space-between; + } + .attention-detail .selectUsers { + color: var(--deemphasized-text-color); + margin-bottom: var(--spacing-m); + } + .attentionTip { + padding: var(--spacing-m); + border: 1px solid var(--border-color); + border-radius: var(--border-radius); + margin-top: var(--spacing-m); + background-color: var(--assignee-highlight-color); + } + .attentionTip div iron-icon { + margin-right: var(--spacing-s); + } + .patchsetLevelContainer { + width: 80ch; + border-radius: var(--border-radius); + box-shadow: var(--elevation-level-2); + } + .patchsetLevelContainer.resolved { + background-color: var(--comment-background-color); + } + .patchsetLevelContainer.unresolved { + background-color: var(--unresolved-comment-background-color); + } + .labelContainer { + padding-left: var(--spacing-m); + padding-bottom: var(--spacing-m); + } + `, + ]; + + override willUpdate(changedProperties: PropertyValues) { + if ( + changedProperties.has('draft') || + changedProperties.has('draftCommentThreads') + ) { + this.computeHasDrafts(); + } + if (changedProperties.has('draft')) { + this.draftChanged(changedProperties.get('draft') as string); + } + if (changedProperties.has('ccPendingConfirmation')) { + this.pendingConfirmationUpdated(this.ccPendingConfirmation); + } + if (changedProperties.has('reviewerPendingConfirmation')) { + this.pendingConfirmationUpdated(this.reviewerPendingConfirmation); + } + if (changedProperties.has('change')) { + this.computeUploader(); + this.changeUpdated(); + } + if (changedProperties.has('canBeStarted')) { + this.computeMessagePlaceholder(); + this.computeSendButtonLabel(); + } + if (changedProperties.has('reviewFormatting')) { + this.handleHeightChanged(); + } + if (changedProperties.has('draftCommentThreads')) { + this.handleHeightChanged(); + } + if (changedProperties.has('reviewers')) { + this.computeAllReviewers(); + } + if (changedProperties.has('sendDisabled')) { + this.sendDisabledChanged(); + } + if (changedProperties.has('attentionExpanded')) { + this.onAttentionExpandedChange(); + } + if ( + changedProperties.has('account') || + changedProperties.has('reviewers') || + changedProperties.has('ccs') || + changedProperties.has('change') || + changedProperties.has('draftCommentThreads') || + changedProperties.has('includeComments') || + changedProperties.has('labelsChanged') || + changedProperties.has('hasDrafts') + ) { + this.computeNewAttention(); + } + } + constructor() { super(); this.filterReviewerSuggestion = - this._filterReviewerSuggestionGenerator(false); - this.filterCCSuggestion = this._filterReviewerSuggestionGenerator(true); + this.filterReviewerSuggestionGenerator(false); + this.filterCCSuggestion = this.filterReviewerSuggestionGenerator(true); + this.jsAPI.addElement(TargetElement.REPLY_DIALOG, this); } override connectedCallback() { @@ -379,23 +661,23 @@ ( IronA11yAnnouncer as unknown as FixIronA11yAnnouncer ).requestAvailability(); - this._getAccount().then(account => { - if (account) this._account = account; + this.restApiService.getAccount().then(account => { + if (account) this.account = account; }); this.cleanups.push( addShortcut(this, {key: Key.ENTER, modifiers: [Modifier.CTRL_KEY]}, _ => - this._submit() + this.submit() ) ); this.cleanups.push( addShortcut(this, {key: Key.ENTER, modifiers: [Modifier.META_KEY]}, _ => - this._submit() + this.submit() ) ); this.cleanups.push(addShortcut(this, {key: Key.ESC}, _ => this.cancel())); this.addEventListener('comment-editing-changed', e => { - this._commentEditing = (e as CustomEvent).detail; + this.commentEditing = (e as CustomEvent).detail; }); // Plugins on reply-reviewers endpoint can take advantage of these @@ -404,22 +686,17 @@ this.addEventListener('add-reviewer', e => { // Only support account type, see more from: // elements/shared/gr-account-list/gr-account-list.js#addAccountItem - this.$.reviewers.addAccountItem({ + this.reviewersList?.addAccountItem({ account: (e as CustomEvent).detail.reviewer, count: 1, }); }); this.addEventListener('remove-reviewer', e => { - this.$.reviewers.removeAccount((e as CustomEvent).detail.reviewer); + this.reviewersList?.removeAccount((e as CustomEvent).detail.reviewer); }); } - override ready() { - super.ready(); - this.jsAPI.addElement(TargetElement.REPLY_DIALOG, this); - } - override disconnectedCallback() { this.storeTask?.cancel(); for (const cleanup of this.cleanups) cleanup(); @@ -427,6 +704,505 @@ super.disconnectedCallback(); } + override render() { + if (!this.change) return; + this.sendDisabled = this.computeSendButtonDisabled(); + return html` + <div tabindex="-1"> + <section class="peopleContainer"> + <gr-endpoint-decorator name="reply-reviewers"> + <gr-endpoint-param + .name=${'change'} + .value=${this.change} + ></gr-endpoint-param> + <gr-endpoint-param .name=${'reviewers'} .value=${this.allReviewers}> + </gr-endpoint-param> + ${this.renderReviewerList()} + <gr-endpoint-slot name="below"></gr-endpoint-slot> + </gr-endpoint-decorator> + ${this.renderCCList()} ${this.renderReviewConfirmation()} + </section> + <section class="labelsContainer">${this.renderLabels()}</section> + <section class="newReplyDialog textareaContainer"> + ${this.renderReplyText()} + </section> + ${when( + this.previewFormatting, + () => html` + <section class="previewContainer"> + <gr-formatted-text + .content=${this.draft} + .config=${this.projectConfig?.commentlinks} + ></gr-formatted-text> + </section> + ` + )} + ${this.renderDraftsSection()} + <div class="stickyBottom newReplyDialog"> + <gr-endpoint-decorator name="reply-bottom"> + <gr-endpoint-param + .name=${'change'} + .value=${this.change} + ></gr-endpoint-param> + ${this.renderAttentionSummarySection()} + ${this.renderAttentionDetailsSection()} + <gr-endpoint-slot name="above-actions"></gr-endpoint-slot> + ${this.renderActionsSection()} + </gr-endpoint-decorator> + </div> + </div> + `; + } + + private renderReviewerList() { + return html` + <div class="peopleList"> + <div class="peopleListLabel">Reviewers</div> + <gr-account-list + id="reviewers" + .accounts=${this.getAccountListCopy(this.reviewers)} + @account-added=${this.accountAdded} + @accounts-changed=${this.handleReviewersChanged} + .removableValues=${this.change?.removable_reviewers} + .filter=${this.filterReviewerSuggestion} + .pendingConfirmation=${this.reviewerPendingConfirmation} + @pending-confirmation-changed=${this + .handleReviewersConfirmationChanged} + .placeholder=${'Add reviewer...'} + @account-text-changed=${this.handleAccountTextEntry} + .suggestionsProvider=${this.getReviewerSuggestionsProvider( + this.change + )} + > + </gr-account-list> + <gr-endpoint-slot name="right"></gr-endpoint-slot> + </div> + `; + } + + private renderCCList() { + return html` + <div class="peopleList"> + <div class="peopleListLabel">CC</div> + <gr-account-list + id="ccs" + .accounts=${this.getAccountListCopy(this.ccs)} + @account-added=${this.accountAdded} + @accounts-changed=${this.handleCcsChanged} + .removableValues=${this.change?.removable_reviewers} + .filter=${this.filterCCSuggestion} + .pendingConfirmation=${this.ccPendingConfirmation} + @pending-confirmation-changed=${this.handleCcsConfirmationChanged} + allow-any-input + .placeholder=${'Add CC...'} + @account-text-changed=${this.handleAccountTextEntry} + .suggestionsProvider=${this.getCcSuggestionsProvider(this.change)} + > + </gr-account-list> + </div> + `; + } + + private renderReviewConfirmation() { + return html` + <gr-overlay + id="reviewerConfirmationOverlay" + @iron-overlay-canceled=${this.cancelPendingReviewer} + > + <div class="reviewerConfirmation"> + Group + <span class="groupName"> + ${this.pendingConfirmationDetails?.group.name} + </span> + has + <span class="groupSize"> + ${this.pendingConfirmationDetails?.count} + </span> + members. + <br /> + Are you sure you want to add them all? + </div> + <div class="reviewerConfirmationButtons"> + <gr-button @click=${this.confirmPendingReviewer}>Yes</gr-button> + <gr-button @click=${this.cancelPendingReviewer}>No</gr-button> + </div> + </gr-overlay> + `; + } + + private renderLabels() { + if (!this.change || !this.account || !this.permittedLabels) return; + return html` + <gr-endpoint-decorator name="reply-label-scores"> + <gr-label-scores + id="labelScores" + .account=${this.account} + .change=${this.change} + @labels-changed=${this._handleLabelsChanged} + .permittedLabels=${this.permittedLabels} + ></gr-label-scores> + <gr-endpoint-param + .name=${'change'} + .value=${this.change} + ></gr-endpoint-param> + </gr-endpoint-decorator> + <div id="pluginMessage">${this.pluginMessage}</div> + `; + } + + private renderReplyText() { + if (!this.change) return; + return html` + <div + class=${classMap({ + patchsetLevelContainer: true, + [this.getUnresolvedPatchsetLevelClass( + this.isResolvedPatchsetLevelComment + )]: true, + })} + > + <gr-endpoint-decorator name="reply-text"> + <gr-textarea + id="textarea" + class="message newReplyDialog" + .autocomplete=${'on'} + .placeholder=${this.messagePlaceholder} + monospace + ?disabled=${this.disabled} + .rows=${4} + .text=${this.draft} + @bind-value-changed=${(e: BindValueChangeEvent) => { + // Setting the value of this.draft in ReplyDialog triggers the + // bind-value-changed observer which ends up calling draftChanged + // with the same oldValue as the current oldValue messing up the + // debounced storage clearence as the first call with the correct + // oldValue is ignored in favor of this second call. + // TODO(dhruvsri): find a better solution to this + if (this.draft === e.detail.value) return; + this.draft = e.detail.value; + this.handleHeightChanged; + }} + > + </gr-textarea> + <gr-endpoint-param .name=${'change'} .value=${this.change}> + </gr-endpoint-param> + </gr-endpoint-decorator> + <div class="labelContainer"> + <label> + <input + id="resolvedPatchsetLevelCommentCheckbox" + type="checkbox" + ?checked=${this.isResolvedPatchsetLevelComment} + @change=${this.handleResolvedPatchsetLevelCommentCheckboxChanged} + /> + Resolved + </label> + <label class="preview-formatting"> + <input + type="checkbox" + ?checked=${this.previewFormatting} + @change=${this.handlePreviewFormattingChanged} + /> + Preview formatting + </label> + </div> + </div> + `; + } + + private renderDraftsSection() { + if (this.computeHideDraftList(this.draftCommentThreads)) return; + return html` + <section class="draftsContainer"> + <div class="includeComments"> + <input + type="checkbox" + id="includeComments" + @change=${this.handleIncludeCommentsChanged} + ?checked=${this.includeComments} + /> + <label for="includeComments" + >Publish ${this.computeDraftsTitle(this.draftCommentThreads)}</label + > + </div> + ${when( + this.includeComments, + () => html` + <gr-thread-list + id="commentList" + .threads=${this.draftCommentThreads!} + hide-dropdown + > + </gr-thread-list> + ` + )} + <span + id="savingLabel" + class=${this.computeSavingLabelClass(this.savingComments)} + > + Saving comments... + </span> + </section> + `; + } + + private renderAttentionSummarySection() { + if (this.attentionExpanded) return; + return html` + <section class="attention"> + <div class="attentionSummary"> + <div> + ${when( + this.computeShowNoAttentionUpdate(), + () => html` <span>${this.computeDoNotUpdateMessage()}</span> ` + )} + ${when( + !this.computeShowNoAttentionUpdate(), + () => html` + <span>Bring to attention of</span> + ${this.computeNewAttentionAccounts().map( + account => html` + <gr-account-label + .account=${account} + .forceAttention=${this.computeHasNewAttention(account)} + .selected=${this.computeHasNewAttention(account)} + .hideHovercard=${true} + .selectionChipStyle=${true} + @click=${this.handleAttentionClick} + ></gr-account-label> + ` + )} + ` + )} + <gr-tooltip-content + has-tooltip + title=${this.computeAttentionButtonTitle()} + > + <gr-button + class="edit-attention-button" + @click=${this.handleAttentionModify} + ?disabled=${this.sendDisabled} + link + position-below + data-label="Edit" + data-action-type="change" + data-action-key="edit" + role="button" + tabindex="0" + > + <iron-icon icon="gr-icons:edit"></iron-icon> + Modify + </gr-button> + </gr-tooltip-content> + </div> + <div> + <a + href="https://gerrit-review.googlesource.com/Documentation/user-attention-set.html" + target="_blank" + > + <iron-icon + icon="gr-icons:help-outline" + title="read documentation" + ></iron-icon> + </a> + </div> + </div> + </section> + `; + } + + private renderAttentionDetailsSection() { + if (!this.attentionExpanded) return; + return html` + <section class="attention-detail"> + <div class="attentionDetailsTitle"> + <div> + <span>Modify attention to</span> + </div> + <div></div> + <div> + <a + href="https://gerrit-review.googlesource.com/Documentation/user-attention-set.html" + target="_blank" + > + <iron-icon + icon="gr-icons:help-outline" + title="read documentation" + ></iron-icon> + </a> + </div> + </div> + <div class="selectUsers"> + <span + >Select chips to set who will be in the attention set after sending + this reply</span + > + </div> + <div class="peopleList"> + <div class="peopleListLabel">Owner</div> + <div class="peopleListValues"> + <gr-account-label + .account=${this.owner} + ?forceAttention=${this.computeHasNewAttention(this.owner)} + .selected=${this.computeHasNewAttention(this.owner)} + .hideHovercard=${true} + .selectionChipStyle=${true} + @click=${this.handleAttentionClick} + > + </gr-account-label> + </div> + </div> + ${when( + this.uploader, + () => html` + <div class="peopleList"> + <div class="peopleListLabel">Uploader</div> + <div class="peopleListValues"> + <gr-account-label + .account=${this.uploader} + ?forceAttention=${this.computeHasNewAttention(this.uploader)} + .selected=${this.computeHasNewAttention(this.uploader)} + .hideHovercard=${true} + .selectionChipStyle=${true} + @click=${this.handleAttentionClick} + > + </gr-account-label> + </div> + </div> + ` + )} + <div class="peopleList"> + <div class="peopleListLabel">Reviewers</div> + <div class="peopleListValues"> + ${this.removeServiceUsers(this.reviewers).map( + account => html` + <gr-account-label + .account=${account} + ?forceAttention=${this.computeHasNewAttention(account)} + .selected=${this.computeHasNewAttention(account)} + .hideHovercard=${true} + .selectionChipStyle=${true} + @click=${this.handleAttentionClick} + > + </gr-account-label> + ` + )} + </div> + </div> + + ${when( + this.attentionCcsCount, + () => html` + <div class="peopleList"> + <div class="peopleListLabel">CC</div> + <div class="peopleListValues"> + ${this.removeServiceUsers(this.ccs).map( + account => html` + <gr-account-label + .account=${account} + ?forceAttention=${this.computeHasNewAttention(account)} + .selected=${this.computeHasNewAttention(account)} + .hideHovercard=${true} + .selectionChipStyle=${true} + @click=${this.handleAttentionClick} + > + </gr-account-label> + ` + )} + </div> + </div> + ` + )} + ${when( + this.computeShowAttentionTip( + this.account, + this.owner, + this.currentAttentionSet, + this.newAttentionSet + ), + () => html` + <div class="attentionTip"> + <iron-icon + class="pointer" + icon="gr-icons:lightbulb-outline" + ></iron-icon> + Be mindful of requiring attention from too many users. + </div> + ` + )} + </section> + `; + } + + private renderActionsSection() { + return html` + <section class="actions"> + <div class="left"> + ${when( + this.knownLatestState === LatestPatchState.CHECKING, + () => html` + <span id="checkingStatusLabel"> + Checking whether patch ${this.patchNum} is latest... + </span> + ` + )} + ${when( + this.knownLatestState === LatestPatchState.NOT_LATEST, + () => html` + <span id="notLatestLabel"> + ${this.computePatchSetWarning()} + <gr-button link @click=${this._reload}>Reload</gr-button> + </span> + ` + )} + </div> + <div class="right"> + <gr-button + link + id="cancelButton" + class="action cancel" + @click=${this.cancelTapHandler} + >Cancel</gr-button + > + ${when( + this.canBeStarted, + () => html` + <!-- Use 'Send' here as the change may only about reviewers / ccs + and when this button is visible, the next button will always + be 'Start review' --> + <gr-tooltip-content has-tooltip title=${this.saveTooltip}> + <gr-button + link + ?disabled=${this.knownLatestState === + LatestPatchState.NOT_LATEST} + class="action save" + @click=${this.saveClickHandler} + >Send As WIP</gr-button + > + </gr-tooltip-content> + ` + )} + <gr-tooltip-content + has-tooltip + title=${this.computeSendButtonTooltip( + this.canBeStarted, + this.commentEditing + )} + > + <gr-button + id="sendButton" + primary + ?disabled=${this.sendDisabled} + class="action send" + @click=${this.sendTapHandler} + >${this.sendButtonLabel} + </gr-button> + </gr-tooltip-content> + </div> + </section> + `; + } + /** * Note that this method is not actually *opening* the dialog. Opening and * showing the dialog is dealt with by the overlay. This method is used by the @@ -444,48 +1220,57 @@ : LatestPatchState.NOT_LATEST; }); - this._focusOn(focusTarget); + this.focusOn(focusTarget); if (quote?.length) { // If a reply quote has been provided, use it. this.draft = quote; } else { // Otherwise, check for an unsaved draft in localstorage. - this.draft = this._loadStoredDraft(); + this.draft = this.loadStoredDraft(); } if (this.restApiService.hasPendingDiffDrafts()) { - this._savingComments = true; + this.savingComments = true; this.restApiService.awaitPendingDiffDrafts().then(() => { fireEvent(this, 'comment-refresh'); - this._savingComments = false; + this.savingComments = false; }); } } - _computeHasDrafts( - draft: string, - draftCommentThreads: PolymerDeepPropertyChange< - CommentThread[] | undefined, - CommentThread[] | undefined - > - ) { - if (draftCommentThreads.base === undefined) return false; - return draft.length > 0 || draftCommentThreads.base.length > 0; + computeHasDrafts() { + if (this.draftCommentThreads === undefined) return false; + return this.draft.length > 0 || this.draftCommentThreads.length > 0; } override focus() { - this._focusOn(FocusTarget.ANY); + this.focusOn(FocusTarget.ANY); } getFocusStops() { - const end = this._sendDisabled ? this.$.cancelButton : this.$.sendButton; - if (!this.$.reviewers.focusStart) return undefined; + const end = this.sendDisabled ? this.cancelButton : this.sendButton; + if (!this.reviewersList?.focusStart || !end) return undefined; return { - start: this.$.reviewers.focusStart, + start: this.reviewersList.focusStart, end, }; } - setLabelValue(label: string, value: string) { + private handleResolvedPatchsetLevelCommentCheckboxChanged(e: Event) { + if (!(e.target instanceof HTMLInputElement)) return; + this.isResolvedPatchsetLevelComment = e.target.checked; + } + + private handlePreviewFormattingChanged(e: Event) { + if (!(e.target instanceof HTMLInputElement)) return; + this.previewFormatting = e.target.checked; + } + + private handleIncludeCommentsChanged(e: Event) { + if (!(e.target instanceof HTMLInputElement)) return; + this.includeComments = e.target.checked; + } + + setLabelValue(label: string, value: string): void { const selectorEl = this.getLabelScores().shadowRoot?.querySelector<GrLabelScoreRow>( `gr-label-score-row[name="${label}"]` @@ -509,14 +1294,14 @@ ? ReviewerType.CC : ReviewerType.REVIEWER; const isReviewer = ReviewerType.REVIEWER === reviewerType; - const array = isReviewer ? this._ccs : this._reviewers; + const array = isReviewer ? this.ccs : this.reviewers; const index = array.findIndex( reviewer => accountOrGroupKey(reviewer) === key ); if (index >= 0) { // Remove any accounts that already exist as a CC for reviewer // or vice versa. - this.splice(isReviewer ? '_ccs' : '_reviewers', index, 1); + array.splice(index, 1); const moveFrom = isReviewer ? 'CC' : 'reviewer'; const moveTo = isReviewer ? 'reviewer' : 'CC'; const id = account.name || key; @@ -541,33 +1326,27 @@ reviewers.push(reviewer); }); }; - addToReviewInput(this.$.reviewers.additions(), ReviewerState.REVIEWER); - addToReviewInput(this.$.ccs.additions(), ReviewerState.CC); + addToReviewInput(this.reviewersList!.additions(), ReviewerState.REVIEWER); + addToReviewInput(this.ccsList!.additions(), ReviewerState.CC); addToReviewInput( - this.$.reviewers.removals().filter( + this.reviewersList!.removals().filter( r => isReviewerOrCC(change, r) && // ignore removal from reviewer request if being added to CC - !this.$.ccs - .additions() - .some( - account => - mapReviewer(account).reviewer === mapReviewer(r).reviewer - ) + !this.ccsList!.additions().some( + account => mapReviewer(account).reviewer === mapReviewer(r).reviewer + ) ), ReviewerState.REMOVED ); addToReviewInput( - this.$.ccs.removals().filter( + this.ccsList!.removals().filter( r => isReviewerOrCC(change, r) && // ignore removal from CC request if being added as reviewer - !this.$.reviewers - .additions() - .some( - account => - mapReviewer(account).reviewer === mapReviewer(r).reviewer - ) + !this.reviewersList!.additions().some( + account => mapReviewer(account).reviewer === mapReviewer(r).reviewer + ) ), ReviewerState.REMOVED ); @@ -589,23 +1368,23 @@ reviewInput.ready = true; } - const reason = getReplyByReason(this._account, this.serverConfig); + const reason = getReplyByReason(this.account, this.serverConfig); reviewInput.ignore_automatic_attention_set_rules = true; reviewInput.add_to_attention_set = []; - for (const user of this._newAttentionSet) { - if (!this._currentAttentionSet.has(user)) { + for (const user of this.newAttentionSet) { + if (!this.currentAttentionSet.has(user)) { reviewInput.add_to_attention_set.push({user, reason}); } } reviewInput.remove_from_attention_set = []; - for (const user of this._currentAttentionSet) { - if (!this._newAttentionSet.has(user)) { + for (const user of this.currentAttentionSet) { + if (!this.newAttentionSet.has(user)) { reviewInput.remove_from_attention_set.push({user, reason}); } } this.reportAttentionSetChanges( - this._attentionExpanded, + this.attentionExpanded, reviewInput.add_to_attention_set, reviewInput.remove_from_attention_set ); @@ -613,7 +1392,7 @@ if (this.draft) { const comment: CommentInput = { message: this.draft, - unresolved: !this._isResolvedPatchsetLevelComment, + unresolved: !this.isResolvedPatchsetLevelComment, }; reviewInput.comments = { [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [comment], @@ -624,8 +1403,8 @@ reviewInput.reviewers = this.computeReviewers(this.change); this.disabled = true; - const errFn = (r?: Response | null) => this._handle400Error(r); - return this._saveReview(reviewInput, errFn) + const errFn = (r?: Response | null) => this.handle400Error(r); + return this.saveReview(reviewInput, errFn) .then(response => { if (!response) { // Null or undefined response indicates that an error handler @@ -638,7 +1417,7 @@ } this.draft = ''; - this._includeComments = true; + this.includeComments = true; this.dispatchEvent( new CustomEvent('send', { composed: true, @@ -658,31 +1437,31 @@ }); } - _focusOn(section?: FocusTarget) { + focusOn(section?: FocusTarget) { // Safeguard- always want to focus on something. if (!section || section === FocusTarget.ANY) { - section = this._chooseFocusTarget(); + section = this.chooseFocusTarget(); } if (section === FocusTarget.BODY) { const textarea = queryAndAssert<GrTextarea>(this, 'gr-textarea'); setTimeout(() => textarea.getNativeTextarea().focus()); } else if (section === FocusTarget.REVIEWERS) { - const reviewerEntry = this.$.reviewers.focusStart; + const reviewerEntry = this.reviewersList?.focusStart; setTimeout(() => reviewerEntry?.focus()); } else if (section === FocusTarget.CCS) { - const ccEntry = this.$.ccs.focusStart; + const ccEntry = this.ccsList?.focusStart; setTimeout(() => ccEntry?.focus()); } } - _chooseFocusTarget() { + chooseFocusTarget() { // If we are the owner and the reviewers field is empty, focus on that. if ( - this._account && + this.account && this.change && this.change.owner && - this._account._account_id === this.change.owner._account_id && - (!this._reviewers || this._reviewers.length === 0) + this.account._account_id === this.change.owner._account_id && + (!this.reviewers || this.reviewers?.length === 0) ) { return FocusTarget.REVIEWERS; } @@ -691,15 +1470,15 @@ return FocusTarget.BODY; } - _isOwner(account?: AccountInfo, change?: ChangeInfo) { + isOwner(account?: AccountInfo, change?: ChangeInfo) { if (!account || !change || !change.owner) return false; return account._account_id === change.owner._account_id; } - _handle400Error(r?: Response | null) { + handle400Error(r?: Response | null) { if (!r) throw new Error('Response is empty.'); let response: Response = r; - // A call to _saveReview could fail with a server error if erroneous + // A call to saveReview could fail with a server error if erroneous // reviewers were requested. This is signalled with a 400 Bad Request // status. The default gr-rest-api error handling would result in a large // JSON response body being displayed to the user in the gr-error-manager @@ -734,45 +1513,42 @@ }); } - _computeHideDraftList(draftCommentThreads?: CommentThread[]) { + computeHideDraftList(draftCommentThreads?: CommentThread[]) { return !draftCommentThreads || draftCommentThreads.length === 0; } - _computeDraftsTitle(draftCommentThreads?: CommentThread[]) { + computeDraftsTitle(draftCommentThreads?: CommentThread[]) { const total = draftCommentThreads ? draftCommentThreads.length : 0; return pluralize(total, 'Draft'); } - _computeMessagePlaceholder(canBeStarted: boolean) { - return canBeStarted + computeMessagePlaceholder() { + this.messagePlaceholder = this.canBeStarted ? 'Add a note for your reviewers...' : 'Say something nice...'; } - @observe('change.reviewers.*', 'change.owner') - _changeUpdated( - changeRecord: PolymerDeepPropertyChange<Reviewers, Reviewers>, - owner: AccountInfo - ) { - if (changeRecord === undefined || owner === undefined) return; - this._rebuildReviewerArrays(changeRecord.base, owner); + changeUpdated() { + if (this.change === undefined) return; + this.rebuildReviewerArrays(); } - _rebuildReviewerArrays(changeReviewers: Reviewers, owner: AccountInfo) { - this._owner = owner; + rebuildReviewerArrays() { + if (!this.change?.owner || !this.change?.reviewers) return; + this.owner = this.change.owner; const reviewers = []; const ccs = []; - if (changeReviewers) { - for (const key of Object.keys(changeReviewers)) { + if (this.change.reviewers) { + for (const key of Object.keys(this.change.reviewers)) { if (key !== 'REVIEWER' && key !== 'CC') { this.reporting.error(new Error(`Unexpected reviewer state: ${key}`)); continue; } - if (!changeReviewers[key]) continue; - for (const entry of changeReviewers[key]!) { - if (entry._account_id === owner._account_id) { + if (!this.change.reviewers[key]) continue; + for (const entry of this.change.reviewers[key]!) { + if (entry._account_id === this.owner._account_id) { continue; } switch (key) { @@ -787,172 +1563,141 @@ } } - this._ccs = ccs; - this._reviewers = reviewers; + this.ccs = ccs; + this.reviewers = reviewers; } - _handleAttentionModify() { - this._attentionExpanded = true; + handleAttentionModify() { + this.attentionExpanded = true; } - @observe('_attentionExpanded') - _onAttentionExpandedChange() { + onAttentionExpandedChange() { // If the attention-detail section is expanded without dispatching this // event, then the dialog may expand beyond the screen's bottom border. fireEvent(this, 'iron-resize'); } - _showAttentionSummary(attentionExpanded?: boolean) { - return !attentionExpanded; - } - - _showAttentionDetails(attentionExpanded?: boolean) { - return attentionExpanded; - } - - _computeAttentionButtonTitle(sendDisabled?: boolean) { + computeAttentionButtonTitle(sendDisabled?: boolean) { return sendDisabled ? 'Modify the attention set by adding a comment or use the account ' + 'hovercard in the change page.' : 'Edit attention set changes'; } - _handleAttentionClick(e: Event) { + handleAttentionClick(e: Event) { const id = (e.target as GrAccountChip)?.account?._account_id; if (!id) return; - const selfId = (this._account && this._account._account_id) || -1; + const selfId = (this.account && this.account._account_id) || -1; const ownerId = (this.change && this.change.owner && this.change.owner._account_id) || -1; const self = id === selfId ? '_SELF' : ''; - const role = id === ownerId ? '_OWNER' : '_REVIEWER'; + const role = id === ownerId ? 'OWNER' : '_REVIEWER'; - if (this._newAttentionSet.has(id)) { - this._newAttentionSet.delete(id); + if (this.newAttentionSet.has(id)) { + this.newAttentionSet.delete(id); this.reporting.reportInteraction(Interaction.ATTENTION_SET_CHIP, { action: `REMOVE${self}${role}`, }); } else { - this._newAttentionSet.add(id); + this.newAttentionSet.add(id); this.reporting.reportInteraction(Interaction.ATTENTION_SET_CHIP, { action: `ADD${self}${role}`, }); } // Ensure that Polymer picks up the change. - this._newAttentionSet = new Set(this._newAttentionSet); + this.newAttentionSet = new Set(this.newAttentionSet); } - _computeHasNewAttention( - account?: AccountInfo, - newAttention?: Set<AccountId> - ) { - return ( - newAttention && + computeHasNewAttention(account?: AccountInfo) { + return !!( account && account._account_id && - newAttention.has(account._account_id) + this.newAttentionSet?.has(account._account_id) ); } - @observe( - '_account', - '_reviewers.*', - '_ccs.*', - 'change', - 'draftCommentThreads', - '_includeComments', - '_labelsChanged', - 'hasDrafts' - ) - _computeNewAttention( - currentUser?: AccountInfo, - reviewers?: PolymerDeepPropertyChange< - AccountInfoInput[], - AccountInfoInput[] - >, - ccs?: PolymerDeepPropertyChange<AccountInfoInput[], AccountInfoInput[]>, - change?: ChangeInfo, - draftCommentThreads?: CommentThread[], - includeComments?: boolean, - _labelsChanged?: boolean, - hasDrafts?: boolean - ) { + computeNewAttention() { if ( - currentUser === undefined || - currentUser._account_id === undefined || - reviewers === undefined || - ccs === undefined || - change === undefined || - draftCommentThreads === undefined || - includeComments === undefined + this.account?._account_id === undefined || + this.change === undefined || + this.includeComments === undefined || + this.draftCommentThreads === undefined ) { return; } // The draft comments are only relevant for the attention set as long as the // user actually plans to publish their drafts. - draftCommentThreads = includeComments ? draftCommentThreads : []; - const hasVote = !!_labelsChanged; - const isOwner = this._isOwner(currentUser, change); - const isUploader = this._uploader?._account_id === currentUser._account_id; - this._attentionCcsCount = removeServiceUsers(ccs.base).length; - this._currentAttentionSet = new Set( - Object.keys(change.attention_set || {}).map(id => Number(id) as AccountId) + const draftCommentThreads = this.includeComments + ? this.draftCommentThreads + : []; + const hasVote = !!this.labelsChanged; + const isOwner = this.isOwner(this.account, this.change); + const isUploader = this.uploader?._account_id === this.account._account_id; + this.attentionCcsCount = removeServiceUsers(this.ccs).length; + this.currentAttentionSet = new Set( + Object.keys(this.change.attention_set || {}).map( + id => Number(id) as AccountId + ) ); - const newAttention = new Set(this._currentAttentionSet); - if (change.status === ChangeStatus.NEW) { + const newAttention = new Set(this.currentAttentionSet); + if (this.change.status === ChangeStatus.NEW) { // Add everyone that the user is replying to in a comment thread. - this._computeCommentAccounts(draftCommentThreads).forEach(id => + this.computeCommentAccounts(draftCommentThreads).forEach(id => newAttention.add(id) ); // Remove the current user. - newAttention.delete(currentUser._account_id); + newAttention.delete(this.account._account_id); // Add all new reviewers, but not the current reviewer, if they are also // sending a draft or a label vote. const notIsReviewerAndHasDraftOrLabel = (r: AccountInfo) => - !(r._account_id === currentUser._account_id && (hasDrafts || hasVote)); - reviewers.base - .filter(r => r._account_id) + !( + r._account_id === this.account!._account_id && + (this.hasDrafts || hasVote) + ); + this.reviewers + .filter(r => isAccount(r)) .filter(r => r._pendingAdd || (this.canBeStarted && isOwner)) .filter(notIsReviewerAndHasDraftOrLabel) - .forEach(r => newAttention.add(r._account_id!)); + .forEach(r => newAttention.add((r as AccountInfo)._account_id!)); // Add owner and uploader, if someone else replies. - if (hasDrafts || hasVote) { - if (this._uploader?._account_id && !isUploader) { - newAttention.add(this._uploader._account_id); + if (this.hasDrafts || hasVote) { + if (this.uploader?._account_id && !isUploader) { + newAttention.add(this.uploader._account_id); } - if (change.owner?._account_id && !isOwner) { - newAttention.add(change.owner._account_id); + if (this.change.owner?._account_id && !isOwner) { + newAttention.add(this.change.owner._account_id); } } } else { // The only reason for adding someone to the attention set for merged or // abandoned changes is that someone makes a comment thread unresolved. const hasUnresolvedDraft = draftCommentThreads.some(isUnresolved); - if (change.owner && hasUnresolvedDraft) { - // A change owner must have an _account_id. - newAttention.add(change.owner._account_id!); + if (this.change.owner && hasUnresolvedDraft) { + // A change owner must have an account_id. + newAttention.add(this.change.owner._account_id!); } // Remove the current user. - newAttention.delete(currentUser._account_id); + newAttention.delete(this.account._account_id); } // Finally make sure that everyone in the attention set is still active as // owner, reviewer or cc. - const allAccountIds = this._allAccounts() + const allAccountIds = this.allAccounts() .map(a => a._account_id) .filter(id => !!id); - this._newAttentionSet = new Set( + this.newAttentionSet = new Set( [...newAttention].filter(id => allAccountIds.includes(id)) ); - this._attentionExpanded = this._computeShowAttentionTip( - currentUser, - change.owner, - this._currentAttentionSet, - this._newAttentionSet + this.attentionExpanded = this.computeShowAttentionTip( + this.account, + this.change.owner, + this.currentAttentionSet, + this.newAttentionSet ); } - _computeShowAttentionTip( + computeShowAttentionTip( currentUser?: AccountInfo, owner?: AccountInfo, currentAttentionSet?: Set<AccountId>, @@ -967,7 +1712,7 @@ return isOwner && addedIds.length > 2; } - _computeCommentAccounts(threads: CommentThread[]) { + computeCommentAccounts(threads: CommentThread[]) { const crLabel = this.change?.labels?.[StandardLabels.CODE_REVIEW]; const maxCrVoteAccountIds = getMaxAccounts(crLabel).map(a => a._account_id); const accountIds = new Set<AccountId>(); @@ -975,7 +1720,7 @@ const unresolved = isUnresolved(thread); thread.comments.forEach(comment => { if (comment.author) { - // A comment author must have an _account_id. + // A comment author must have an account_id. const authorId = comment.author._account_id!; const hasGivenMaxReviewVote = maxCrVoteAccountIds.includes(authorId); if (unresolved || !hasGivenMaxReviewVote) accountIds.add(authorId); @@ -985,110 +1730,93 @@ return accountIds; } - _computeShowNoAttentionUpdate( - config?: ServerInfo, - currentAttentionSet?: Set<AccountId>, - newAttentionSet?: Set<AccountId>, - sendDisabled?: boolean - ) { - return ( - sendDisabled || - this._computeNewAttentionAccounts( - config, - currentAttentionSet, - newAttentionSet - ).length === 0 - ); + computeShowNoAttentionUpdate() { + return this.sendDisabled || this.computeNewAttentionAccounts().length === 0; } - _computeDoNotUpdateMessage( - currentAttentionSet?: Set<AccountId>, - newAttentionSet?: Set<AccountId>, - sendDisabled?: boolean - ) { - if (!currentAttentionSet || !newAttentionSet) return ''; - if (sendDisabled || areSetsEqual(currentAttentionSet, newAttentionSet)) { + computeDoNotUpdateMessage() { + if (!this.currentAttentionSet || !this.newAttentionSet) return ''; + if ( + this.sendDisabled || + areSetsEqual(this.currentAttentionSet, this.newAttentionSet) + ) { return 'No changes to the attention set.'; } - if (containsAll(currentAttentionSet, newAttentionSet)) { + if (containsAll(this.currentAttentionSet, this.newAttentionSet)) { return 'No additions to the attention set.'; } this.reporting.error( new Error( - '_computeDoNotUpdateMessage()' + + 'computeDoNotUpdateMessage()' + 'should not be called when users were added to the attention set.' ) ); return ''; } - _computeNewAttentionAccounts( - _?: ServerInfo, - currentAttentionSet?: Set<AccountId>, - newAttentionSet?: Set<AccountId> - ) { - if (currentAttentionSet === undefined || newAttentionSet === undefined) { + computeNewAttentionAccounts(): AccountInfo[] { + if ( + this.currentAttentionSet === undefined || + this.newAttentionSet === undefined + ) { return []; } - return [...newAttentionSet] - .filter(id => !currentAttentionSet.has(id)) - .map(id => this._findAccountById(id)) - .filter(account => !!account); + return [...this.newAttentionSet] + .filter(id => !this.currentAttentionSet.has(id)) + .map(id => this.findAccountById(id)) + .filter(account => !!account) as AccountInfo[]; } - _findAccountById(accountId: AccountId) { - return this._allAccounts().find(r => r._account_id === accountId); + findAccountById(accountId: AccountId) { + return this.allAccounts().find(r => r._account_id === accountId); } - _allAccounts() { + allAccounts() { let allAccounts: (AccountInfoInput | GroupInfoInput)[] = []; if (this.change && this.change.owner) allAccounts.push(this.change.owner); - if (this._uploader) allAccounts.push(this._uploader); - if (this._reviewers) allAccounts = [...allAccounts, ...this._reviewers]; - if (this._ccs) allAccounts = [...allAccounts, ...this._ccs]; + if (this.uploader) allAccounts.push(this.uploader); + if (this.reviewers) allAccounts = [...allAccounts, ...this.reviewers]; + if (this.ccs) allAccounts = [...allAccounts, ...this.ccs]; return removeServiceUsers(allAccounts.filter(isAccount)); } - /** - * The newAttentionSet param is only used to force re-computation. - */ - _removeServiceUsers(accounts: AccountInfo[], _: Set<AccountId>) { + removeServiceUsers(accounts: AccountInfo[]) { return removeServiceUsers(accounts); } - _computeUploader(change: ChangeInfo) { + computeUploader() { if ( - !change || - !change.current_revision || - !change.revisions || - !change.revisions[change.current_revision] + !this.change?.current_revision || + !this.change?.revisions?.[this.change.current_revision] ) { - return undefined; + this.uploader = undefined; + return; } - const rev = change.revisions[change.current_revision]; + const rev = this.change.revisions[this.change.current_revision]; if ( !rev.uploader || - change.owner._account_id === rev.uploader._account_id + this.change?.owner._account_id === rev.uploader._account_id ) { - return undefined; + this.uploader = undefined; + return; } - return rev.uploader; + this.uploader = rev.uploader; } /** * Generates a function to filter out reviewer/CC entries. When isCCs is - * truthy, the function filters out entries that already exist in this._ccs. - * When falsy, the function filters entries that exist in this._reviewers. + * truthy, the function filters out entries that already exist in this.ccs. + * When falsy, the function filters entries that exist in this.reviewers. */ - _filterReviewerSuggestionGenerator( + filterReviewerSuggestionGenerator( isCCs: boolean ): (input: Suggestion) => boolean { return suggestion => { let entry: AccountInfo | GroupInfo; if (isReviewerAccountSuggestion(suggestion)) { entry = suggestion.account; - if (entry._account_id === this._owner?._account_id) { + if (entry._account_id === this.owner?._account_id) { return false; } } else if (isReviewerGroupSuggestion(suggestion)) { @@ -1104,24 +1832,20 @@ const finder = (entry: AccountInfo | GroupInfo) => accountOrGroupKey(entry) === key; if (isCCs) { - return this._ccs.find(finder) === undefined; + return this.ccs.find(finder) === undefined; } - return this._reviewers.find(finder) === undefined; + return this.reviewers.find(finder) === undefined; }; } - _getAccount() { - return this.restApiService.getAccount(); - } - - _cancelTapHandler(e: Event) { + cancelTapHandler(e: Event) { e.preventDefault(); this.cancel(); } cancel() { assertIsDefined(this.change, 'change'); - if (!this._owner) throw new Error('missing required _owner property'); + if (!this.owner) throw new Error('missing required owner property'); this.dispatchEvent( new CustomEvent('cancel', { composed: true, @@ -1129,36 +1853,36 @@ }) ); queryAndAssert<GrTextarea>(this, 'gr-textarea').closeDropdown(); - this.$.reviewers.clearPendingRemovals(); - this._rebuildReviewerArrays(this.change.reviewers, this._owner); + this.reviewersList?.clearPendingRemovals(); + this.rebuildReviewerArrays(); } - _saveClickHandler(e: Event) { + saveClickHandler(e: Event) { e.preventDefault(); - if (!this.$.ccs.submitEntryText()) { + if (!this.ccsList?.submitEntryText()) { // Do not proceed with the save if there is an invalid email entry in // the text field of the CC entry. return; } - this.send(this._includeComments, false); + this.send(this.includeComments, false); } - _sendTapHandler(e: Event) { + sendTapHandler(e: Event) { e.preventDefault(); - this._submit(); + this.submit(); } - _submit() { - if (!this.$.ccs.submitEntryText()) { + submit() { + if (!this.ccsList?.submitEntryText()) { // Do not proceed with the send if there is an invalid email entry in // the text field of the CC entry. return; } - if (this._sendDisabled) { + if (this.sendDisabled) { fireAlert(this, EMPTY_REPLY_MESSAGE); return; } - return this.send(this._includeComments, this.canBeStarted).catch(err => { + return this.send(this.includeComments, this.canBeStarted).catch(err => { this.dispatchEvent( new CustomEvent('show-error', { bubbles: true, @@ -1169,7 +1893,7 @@ }); } - _saveReview(review: ReviewInput, errFn?: ErrorCallback) { + saveReview(review: ReviewInput, errFn?: ErrorCallback) { assertIsDefined(this.change, 'change'); assertIsDefined(this.patchNum, 'patchNum'); return this.restApiService.saveChangeReview( @@ -1180,43 +1904,43 @@ ); } - _reviewerPendingConfirmationUpdated(reviewer: RawAccountInput | null) { + pendingConfirmationUpdated(reviewer: RawAccountInput | null) { if (reviewer === null) { - this.$.reviewerConfirmationOverlay.close(); + this.reviewerConfirmationOverlay?.close(); } else { - this._pendingConfirmationDetails = - this._ccPendingConfirmation || this._reviewerPendingConfirmation; - this.$.reviewerConfirmationOverlay.open(); + this.pendingConfirmationDetails = + this.ccPendingConfirmation || this.reviewerPendingConfirmation; + this.reviewerConfirmationOverlay?.open(); } } - _confirmPendingReviewer() { - if (this._ccPendingConfirmation) { - this.$.ccs.confirmGroup(this._ccPendingConfirmation.group); - this._focusOn(FocusTarget.CCS); + confirmPendingReviewer() { + if (this.ccPendingConfirmation) { + this.ccsList?.confirmGroup(this.ccPendingConfirmation.group); + this.focusOn(FocusTarget.CCS); return; } - if (this._reviewerPendingConfirmation) { - this.$.reviewers.confirmGroup(this._reviewerPendingConfirmation.group); - this._focusOn(FocusTarget.REVIEWERS); + if (this.reviewerPendingConfirmation) { + this.reviewersList?.confirmGroup(this.reviewerPendingConfirmation.group); + this.focusOn(FocusTarget.REVIEWERS); return; } this.reporting.error( - new Error('_confirmPendingReviewer called without pending confirm') + new Error('confirmPendingReviewer called without pending confirm') ); } - _cancelPendingReviewer() { - this._ccPendingConfirmation = null; - this._reviewerPendingConfirmation = null; + cancelPendingReviewer() { + this.ccPendingConfirmation = null; + this.reviewerPendingConfirmation = null; - const target = this._ccPendingConfirmation + const target = this.ccPendingConfirmation ? FocusTarget.CCS : FocusTarget.REVIEWERS; - this._focusOn(target); + this.focusOn(target); } - _getStorageLocation(): StorageLocation { + getStorageLocation(): StorageLocation { assertIsDefined(this.change, 'change'); return { changeNum: this.change._number, @@ -1225,77 +1949,73 @@ }; } - _loadStoredDraft() { - const draft = this.storage.getDraftComment(this._getStorageLocation()); + loadStoredDraft() { + const draft = this.storage.getDraftComment(this.getStorageLocation()); return draft?.message ?? ''; } - _handleAccountTextEntry() { + handleAccountTextEntry() { // When either of the account entries has input added to the autocomplete, // it should trigger the save button to enable/ // // Note: if the text is removed, the save button will not get disabled. - this._reviewersMutated = true; + this.reviewersMutated = true; } - _draftChanged(newDraft: string, oldDraft?: string) { + draftChanged(oldDraft: string) { this.storeTask = debounce( this.storeTask, () => { - if (!newDraft.length && oldDraft) { + if (!this.draft.length && oldDraft) { // If the draft has been modified to be empty, then erase the storage // entry. - this.storage.eraseDraftComment(this._getStorageLocation()); - } else if (newDraft.length) { - this.storage.setDraftComment(this._getStorageLocation(), this.draft); + this.storage.eraseDraftComment(this.getStorageLocation()); + } else if (this.draft.length) { + this.storage.setDraftComment(this.getStorageLocation(), this.draft); } }, STORAGE_DEBOUNCE_INTERVAL_MS ); } - _handleHeightChanged() { + handleHeightChanged() { fireEvent(this, 'autogrow'); } - getLabelScores() { - return this.$.labelScores || queryAndAssert(this, 'gr-label-scores'); + getLabelScores(): GrLabelScores { + return this.labelScores || queryAndAssert(this, 'gr-label-scores'); } _handleLabelsChanged() { - this._labelsChanged = + this.labelsChanged = Object.keys(this.getLabelScores().getLabelValues(false)).length !== 0; } // To decouple account-list and reply dialog - _getAccountListCopy(list: (AccountInfo | GroupInfo)[]) { + getAccountListCopy(list: (AccountInfo | GroupInfo)[]) { return list.slice(); } - _handleReviewersChanged(e: ValueChangedEvent<(AccountInfo | GroupInfo)[]>) { - this._reviewers = e.detail.value.slice(); - this._reviewersMutated = true; + 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; + handleCcsChanged(e: ValueChangedEvent<(AccountInfo | GroupInfo)[]>) { + this.ccs = e.detail.value.slice(); + this.reviewersMutated = true; } - _handleReviewersConfirmationChanged( + handleReviewersConfirmationChanged( e: ValueChangedEvent<SuggestedReviewerGroupInfo | null> ) { - this._reviewerPendingConfirmation = e.detail.value; + this.reviewerPendingConfirmation = e.detail.value; } - _handleCcsConfirmationChanged( + handleCcsConfirmationChanged( e: ValueChangedEvent<SuggestedReviewerGroupInfo | null> ) { - this._ccPendingConfirmation = e.detail.value; - } - - _isState(knownLatestState?: LatestPatchState, value?: LatestPatchState) { - return knownLatestState === value; + this.ccPendingConfirmation = e.detail.value; } _reload() { @@ -1303,82 +2023,77 @@ this.cancel(); } - _computeSendButtonLabel(canBeStarted: boolean) { - return canBeStarted + computeSendButtonLabel() { + this.sendButtonLabel = this.canBeStarted ? ButtonLabels.SEND + ' and ' + ButtonLabels.START_REVIEW : ButtonLabels.SEND; } - _computeSendButtonTooltip(canBeStarted?: boolean, commentEditing?: boolean) { + computeSendButtonTooltip(canBeStarted?: boolean, commentEditing?: boolean) { if (commentEditing) { return ButtonTooltips.DISABLED_COMMENT_EDITING; } return canBeStarted ? ButtonTooltips.START_REVIEW : ButtonTooltips.SEND; } - _computeSavingLabelClass(savingComments: boolean) { + computeSavingLabelClass(savingComments: boolean) { return savingComments ? 'saving' : ''; } - _computeSendButtonDisabled( - canBeStarted?: boolean, - draftCommentThreads?: CommentThread[], - text?: string, - reviewersMutated?: boolean, - labelsChanged?: boolean, - includeComments?: boolean, - disabled?: boolean, - commentEditing?: boolean, - change?: ChangeInfo, - account?: AccountInfo - ) { + computeSendButtonDisabled() { if ( - canBeStarted === undefined || - draftCommentThreads === undefined || - text === undefined || - reviewersMutated === undefined || - labelsChanged === undefined || - includeComments === undefined || - disabled === undefined || - commentEditing === undefined || - change?.labels === undefined || - account === undefined + this.canBeStarted === undefined || + this.draftCommentThreads === undefined || + this.draft === undefined || + this.reviewersMutated === undefined || + this.labelsChanged === undefined || + this.includeComments === undefined || + this.disabled === undefined || + this.commentEditing === undefined || + this.change?.labels === undefined || + this.account === undefined ) { return undefined; } - if (commentEditing || disabled) { + if (this.commentEditing || this.disabled) { return true; } - if (canBeStarted === true) { + if (this.canBeStarted === true) { return false; } - const existingVote = Object.values(change.labels).some( - label => isDetailedLabelInfo(label) && getApprovalInfo(label, account) + const existingVote = Object.values(this.change.labels).some( + label => + isDetailedLabelInfo(label) && getApprovalInfo(label, this.account!) ); - const revotingOrNewVote = labelsChanged || existingVote; - const hasDrafts = includeComments && draftCommentThreads.length; + const revotingOrNewVote = this.labelsChanged || existingVote; + const hasDrafts = + this.includeComments && this.draftCommentThreads.length > 0; return ( - !hasDrafts && !text.length && !reviewersMutated && !revotingOrNewVote + !hasDrafts && + !this.draft.length && + !this.reviewersMutated && + !revotingOrNewVote ); } - _computePatchSetWarning(patchNum?: PatchSetNum, labelsChanged?: boolean) { - let str = `Patch ${patchNum} is not latest.`; - if (labelsChanged) { + computePatchSetWarning() { + let str = `Patch ${this.patchNum} is not latest.`; + if (this.labelsChanged) { str += ' Voting may have no effect.'; } return str; } setPluginMessage(message: string) { - this._pluginMessage = message; + this.pluginMessage = message; } - _sendDisabledChanged() { + sendDisabledChanged() { this.dispatchEvent(new CustomEvent('send-disabled-changed')); } - _getReviewerSuggestionsProvider(change: ChangeInfo) { + getReviewerSuggestionsProvider(change?: ChangeInfo) { + if (!change) return; const provider = GrReviewerSuggestionsProvider.create( this.restApiService, change._number, @@ -1388,7 +2103,8 @@ return provider; } - _getCcSuggestionsProvider(change: ChangeInfo) { + getCcSuggestionsProvider(change?: ChangeInfo) { + if (!change) return; const provider = GrReviewerSuggestionsProvider.create( this.restApiService, change._number, @@ -1406,24 +2122,24 @@ const actions = modified ? ['MODIFIED'] : ['NOT_MODIFIED']; const ownerId = (this.change && this.change.owner && this.change.owner._account_id) || -1; - const selfId = (this._account && this._account._account_id) || -1; + const selfId = (this.account && this.account._account_id) || -1; for (const added of addedSet || []) { const addedId = added.user; const self = addedId === selfId ? '_SELF' : ''; - const role = addedId === ownerId ? '_OWNER' : '_REVIEWER'; + const role = addedId === ownerId ? 'OWNER' : '_REVIEWER'; actions.push('ADD' + self + role); } for (const removed of removedSet || []) { const removedId = removed.user; const self = removedId === selfId ? '_SELF' : ''; - const role = removedId === ownerId ? '_OWNER' : '_REVIEWER'; + const role = removedId === ownerId ? 'OWNER' : '_REVIEWER'; actions.push('REMOVE' + self + role); } this.reporting.reportInteraction('attention-set-actions', {actions}); } - _computeAllReviewers() { - return [...this._reviewers]; + computeAllReviewers() { + this.allReviewers = [...this.reviewers]; } }
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 deleted file mode 100644 index c4b3578..0000000 --- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts +++ /dev/null
@@ -1,652 +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"> - :host { - background-color: var(--dialog-background-color); - display: block; - max-height: 90vh; - } - :host([disabled]) { - pointer-events: none; - } - :host([disabled]) .container { - opacity: 0.5; - } - section { - border-top: 1px solid var(--border-color); - flex-shrink: 0; - padding: var(--spacing-m) var(--spacing-xl); - width: 100%; - } - section.labelsContainer { - /* We want the :hover highlight to extend to the border of the dialog. */ - padding: var(--spacing-m) 0; - } - .stickyBottom { - background-color: var(--dialog-background-color); - box-shadow: 0px 0px 8px 0px rgba(60, 64, 67, 0.15); - margin-top: var(--spacing-s); - bottom: 0; - position: sticky; - /* @see Issue 8602 */ - z-index: 1; - } - .stickyBottom.newReplyDialog { - margin-top: unset; - } - .actions { - display: flex; - justify-content: space-between; - } - .actions .right gr-button { - margin-left: var(--spacing-l); - } - .peopleContainer, - .labelsContainer { - flex-shrink: 0; - } - .peopleContainer { - border-top: none; - display: table; - } - .peopleList { - display: flex; - } - .peopleListLabel { - color: var(--deemphasized-text-color); - margin-top: var(--spacing-xs); - min-width: 6em; - padding-right: var(--spacing-m); - } - gr-account-list { - display: flex; - flex-wrap: wrap; - flex: 1; - } - #reviewerConfirmationOverlay { - padding: var(--spacing-l); - text-align: center; - } - .reviewerConfirmationButtons { - margin-top: var(--spacing-l); - } - .groupName { - font-weight: var(--font-weight-bold); - } - .groupSize { - font-style: italic; - } - .textareaContainer { - min-height: 12em; - position: relative; - } - .newReplyDialog.textareaContainer { - min-height: unset; - } - textareaContainer, - #textarea, - gr-endpoint-decorator[name='reply-text'] { - display: flex; - width: 100%; - } - .newReplyDialog .textareaContainer, - #textarea, - gr-endpoint-decorator[name='reply-text'] { - display: block; - width: unset; - font-family: var(--monospace-font-family); - font-size: var(--font-size-code); - line-height: calc(var(--font-size-code) + var(--spacing-s)); - font-weight: var(--font-weight-normal); - } - .newReplyDialog#textarea { - padding: var(--spacing-m); - } - gr-endpoint-decorator[name='reply-text'] { - flex-direction: column; - } - #textarea { - flex: 1; - } - .previewContainer { - border-top: none; - } - .previewContainer gr-formatted-text { - background: var(--table-header-background-color); - padding: var(--spacing-l); - } - #checkingStatusLabel, - #notLatestLabel { - margin-left: var(--spacing-l); - } - #checkingStatusLabel { - color: var(--deemphasized-text-color); - font-style: italic; - } - #notLatestLabel, - #savingLabel { - color: var(--error-text-color); - } - #savingLabel { - display: none; - } - #savingLabel.saving { - display: inline; - } - #pluginMessage { - color: var(--deemphasized-text-color); - margin-left: var(--spacing-l); - margin-bottom: var(--spacing-m); - } - #pluginMessage:empty { - display: none; - } - .preview-formatting { - margin-left: var(--spacing-m); - } - .attention-icon { - width: 14px; - height: 14px; - vertical-align: top; - position: relative; - top: 3px; - --iron-icon-height: 24px; - --iron-icon-width: 24px; - } - .attention .edit-attention-button { - vertical-align: top; - --gr-button-padding: 0px 4px; - } - .attention .edit-attention-button iron-icon { - color: inherit; - } - .attention a, - .attention-detail a { - text-decoration: none; - } - .attentionSummary { - display: flex; - justify-content: space-between; - } - .attentionSummary { - /* The account label for selection is misbehaving currently: It consumes - 26px height instead of 20px, which is the default line-height and thus - the max that can be nicely fit into an inline layout flow. We - acknowledge that using a fixed 26px value here is a hack and not a - great solution. */ - line-height: 26px; - } - .attentionSummary gr-account-label, - .attention-detail gr-account-label { - --account-max-length: 120px; - display: inline-block; - padding: var(--spacing-xs) var(--spacing-m); - user-select: none; - --label-border-radius: 8px; - } - .attentionSummary gr-account-label { - margin: 0 var(--spacing-xs); - line-height: var(--line-height-normal); - vertical-align: top; - } - .attention-detail .peopleListValues { - line-height: calc(var(--line-height-normal) + 10px); - } - .attention-detail gr-account-label { - line-height: var(--line-height-normal); - } - .attentionSummary gr-account-label:focus, - .attention-detail gr-account-label:focus { - outline: none; - } - .attentionSummary gr-account-label:hover, - .attention-detail gr-account-label:hover { - box-shadow: var(--elevation-level-1); - cursor: pointer; - } - .attention-detail .attentionDetailsTitle { - display: flex; - justify-content: space-between; - } - .attention-detail .selectUsers { - color: var(--deemphasized-text-color); - margin-bottom: var(--spacing-m); - } - .attentionTip { - padding: var(--spacing-m); - border: 1px solid var(--border-color); - border-radius: var(--border-radius); - margin-top: var(--spacing-m); - background-color: var(--assignee-highlight-color); - } - .attentionTip div iron-icon { - margin-right: var(--spacing-s); - } - .patchsetLevelContainer { - width: 80ch; - border-radius: var(--border-radius); - box-shadow: var(--elevation-level-2); - } - .patchsetLevelContainer.resolved{ - background-color: var(--comment-background-color); - } - .patchsetLevelContainer.unresolved{ - background-color: var(--unresolved-comment-background-color); - } - .labelContainer { - padding-left: var(--spacing-m); - padding-bottom: var(--spacing-m); - } - - </style> - <div tabindex="-1"> - <section class="peopleContainer"> - <gr-endpoint-decorator name="reply-reviewers"> - <gr-endpoint-param name="change" value="[[change]]"></gr-endpoint-param> - <gr-endpoint-param name="reviewers" value="[[_allReviewers]]"> - </gr-endpoint-param> - <div class="peopleList"> - <div class="peopleListLabel">Reviewers</div> - <gr-account-list - id="reviewers" - accounts="[[_getAccountListCopy(_reviewers)]]" - on-account-added="accountAdded" - on-accounts-changed="_handleReviewersChanged" - removable-values="[[change.removable_reviewers]]" - filter="[[filterReviewerSuggestion]]" - pending-confirmation="[[_reviewerPendingConfirmation]]" - on-pending-confirmation-changed="_handleReviewersConfirmationChanged" - placeholder="Add reviewer..." - on-account-text-changed="_handleAccountTextEntry" - suggestions-provider="[[_getReviewerSuggestionsProvider(change)]]" - > - </gr-account-list> - <gr-endpoint-slot name="right"></gr-endpoint-slot> - </div> - <gr-endpoint-slot name="below"></gr-endpoint-slot> - </gr-endpoint-decorator> - <div class="peopleList"> - <div class="peopleListLabel">CC</div> - <gr-account-list - id="ccs" - accounts="[[_getAccountListCopy(_ccs)]]" - on-account-added="accountAdded" - on-accounts-changed="_handleCcsChanged" - filter="[[filterCCSuggestion]]" - pending-confirmation="[[_ccPendingConfirmation]]" - pending-confirmation-changed="_handleCcsConfirmationChanged" - allow-any-input="" - placeholder="Add CC..." - on-account-text-changed="_handleAccountTextEntry" - suggestions-provider="[[_getCcSuggestionsProvider(change)]]" - > - </gr-account-list> - </div> - <gr-overlay - id="reviewerConfirmationOverlay" - on-iron-overlay-canceled="_cancelPendingReviewer" - > - <div class="reviewerConfirmation"> - Group - <span class="groupName"> - [[_pendingConfirmationDetails.group.name]] - </span> - has - <span class="groupSize"> [[_pendingConfirmationDetails.count]] </span> - members. - <br /> - Are you sure you want to add them all? - </div> - <div class="reviewerConfirmationButtons"> - <gr-button on-click="_confirmPendingReviewer">Yes</gr-button> - <gr-button on-click="_cancelPendingReviewer">No</gr-button> - </div> - </gr-overlay> - </section> - - <section class="labelsContainer"> - <gr-endpoint-decorator name="reply-label-scores"> - <gr-label-scores - id="labelScores" - account="[[_account]]" - change="[[change]]" - on-labels-changed="_handleLabelsChanged" - permitted-labels="[[permittedLabels]]" - ></gr-label-scores> - <gr-endpoint-param name="change" value="[[change]]"></gr-endpoint-param> - </gr-endpoint-decorator> - <div id="pluginMessage">[[_pluginMessage]]</div> - </section> - <section class="newReplyDialog textareaContainer"> - <div class$="patchsetLevelContainer [[getUnresolvedPatchsetLevelClass(_isResolvedPatchsetLevelComment)]]"> - <gr-endpoint-decorator name="reply-text"> - <gr-textarea - id="textarea" - class="message newReplyDialog" - autocomplete="on" - placeholder="[[_messagePlaceholder]]" - monospace="true" - disabled="{{disabled}}" - rows="4" - text="{{draft}}" - on-bind-value-changed="_handleHeightChanged" - > - </gr-textarea> - <gr-endpoint-param name="change" value="[[change]]"> - </gr-endpoint-param> - </gr-endpoint-decorator> - <div class="labelContainer"> - <label> - <input - id="resolvedPatchsetLevelCommentCheckbox" - type="checkbox" - checked="{{_isResolvedPatchsetLevelComment::change}}" - /> - Resolved - </label> - <label class="preview-formatting"> - <input type="checkbox" checked="{{_previewFormatting::change}}" /> - Preview formatting - </label> - </div> - </div> - </section> - <template is="dom-if" if="[[_previewFormatting]]"> - <section class="previewContainer"> - <gr-formatted-text - content="[[draft]]" - config="[[projectConfig.commentlinks]]" - ></gr-formatted-text> - </template> - </section> - - <section - class="draftsContainer" - hidden$="[[_computeHideDraftList(draftCommentThreads)]]" - > - <div class="includeComments"> - <input - type="checkbox" - id="includeComments" - checked="{{_includeComments::change}}" - /> - <label for="includeComments" - >Publish [[_computeDraftsTitle(draftCommentThreads)]]</label - > - </div> - <gr-thread-list - id="commentList" - hidden$="[[!_includeComments]]" - threads="[[draftCommentThreads]]" - hide-dropdown="" - > - </gr-thread-list> - <span - id="savingLabel" - class$="[[_computeSavingLabelClass(_savingComments)]]" - > - Saving comments... - </span> - </section> - <div class="stickyBottom newReplyDialog"> - <gr-endpoint-decorator name="reply-bottom"> - <gr-endpoint-param name="change" value="[[change]]"></gr-endpoint-param> - <section - hidden$="[[!_showAttentionSummary(_attentionExpanded)]]" - class="attention" - > - <div class="attentionSummary"> - <div> - <template - is="dom-if" - if="[[_computeShowNoAttentionUpdate(serverConfig, _currentAttentionSet, _newAttentionSet, _sendDisabled)]]" - > - <span - >[[_computeDoNotUpdateMessage(_currentAttentionSet, - _newAttentionSet, _sendDisabled)]]</span - > - </template> - <template - is="dom-if" - if="[[!_computeShowNoAttentionUpdate(serverConfig, _currentAttentionSet, _newAttentionSet, _sendDisabled)]]" - > - <span>Bring to attention of</span> - <template - is="dom-repeat" - items="[[_computeNewAttentionAccounts(serverConfig, _currentAttentionSet, _newAttentionSet)]]" - as="account" - > - <gr-account-label - account="[[account]]" - force-attention="[[_computeHasNewAttention(account, _newAttentionSet)]]" - selected="[[_computeHasNewAttention(account, _newAttentionSet)]]" - hideHovercard - selectionChipStyle - on-click="_handleAttentionClick" - ></gr-account-label> - </template> - </template> - <gr-tooltip-content - has-tooltip - title="[[_computeAttentionButtonTitle(_sendDisabled)]]" - > - <gr-button - class="edit-attention-button" - on-click="_handleAttentionModify" - disabled="[[_sendDisabled]]" - link="" - position-below="" - data-label="Edit" - data-action-type="change" - data-action-key="edit" - role="button" - tabindex="0" - > - <iron-icon icon="gr-icons:edit"></iron-icon> - Modify - </gr-button> - </gr-tooltip-content> - </div> - <div> - <a - href="https://gerrit-review.googlesource.com/Documentation/user-attention-set.html" - target="_blank" - > - <iron-icon - icon="gr-icons:help-outline" - title="read documentation" - ></iron-icon> - </a> - </div> - </div> - </section> - <section - hidden$="[[!_showAttentionDetails(_attentionExpanded)]]" - class="attention-detail" - > - <div class="attentionDetailsTitle"> - <div> - <span>Modify attention to</span> - </div> - <div></div> - <div> - <a - href="https://gerrit-review.googlesource.com/Documentation/user-attention-set.html" - target="_blank" - > - <iron-icon - icon="gr-icons:help-outline" - title="read documentation" - ></iron-icon> - </a> - </div> - </div> - <div class="selectUsers"> - <span - >Select chips to set who will be in the attention set after sending - this reply</span - > - </div> - <div class="peopleList"> - <div class="peopleListLabel">Owner</div> - <div class="peopleListValues"> - <gr-account-label - account="[[_owner]]" - force-attention="[[_computeHasNewAttention(_owner, _newAttentionSet)]]" - selected="[[_computeHasNewAttention(_owner, _newAttentionSet)]]" - hideHovercard - selectionChipStyle - on-click="_handleAttentionClick" - > - </gr-account-label> - </div> - </div> - <template is="dom-if" if="[[_uploader]]"> - <div class="peopleList"> - <div class="peopleListLabel">Uploader</div> - <div class="peopleListValues"> - <gr-account-label - account="[[_uploader]]" - force-attention="[[_computeHasNewAttention(_uploader, _newAttentionSet)]]" - selected="[[_computeHasNewAttention(_uploader, _newAttentionSet)]]" - hideHovercard - selectionChipStyle - on-click="_handleAttentionClick" - > - </gr-account-label> - </div> - </div> - </template> - <div class="peopleList"> - <div class="peopleListLabel">Reviewers</div> - <div class="peopleListValues"> - <template - is="dom-repeat" - items="[[_removeServiceUsers(_reviewers, _newAttentionSet)]]" - as="account" - > - <gr-account-label - account="[[account]]" - force-attention="[[_computeHasNewAttention(account, _newAttentionSet)]]" - selected="[[_computeHasNewAttention(account, _newAttentionSet)]]" - hideHovercard - selectionChipStyle - on-click="_handleAttentionClick" - > - </gr-account-label> - </template> - </div> - </div> - <template is="dom-if" if="[[_attentionCcsCount]]"> - <div class="peopleList"> - <div class="peopleListLabel">CC</div> - <div class="peopleListValues"> - <template - is="dom-repeat" - items="[[_removeServiceUsers(_ccs, _newAttentionSet)]]" - as="account" - > - <gr-account-label - account="[[account]]" - force-attention="[[_computeHasNewAttention(account, _newAttentionSet)]]" - selected="[[_computeHasNewAttention(account, _newAttentionSet)]]" - hideHovercard - selectionChipStyle - on-click="_handleAttentionClick" - > - </gr-account-label> - </template> - </div> - </div> - </template> - <template - is="dom-if" - if="[[_computeShowAttentionTip(_account, _owner, _currentAttentionSet, _newAttentionSet)]]" - > - <div class="attentionTip"> - <iron-icon - class="pointer" - icon="gr-icons:lightbulb-outline" - ></iron-icon> - Be mindful of requiring attention from too many users. - </div> - </template> - </section> - <gr-endpoint-slot name="above-actions"></gr-endpoint-slot> - <section class="actions"> - <div class="left"> - <span - id="checkingStatusLabel" - hidden$="[[!_isState(knownLatestState, 'checking')]]" - > - Checking whether patch [[patchNum]] is latest... - </span> - <span - id="notLatestLabel" - hidden$="[[!_isState(knownLatestState, 'not-latest')]]" - > - [[_computePatchSetWarning(patchNum, _labelsChanged)]] - <gr-button link="" on-click="_reload">Reload</gr-button> - </span> - </div> - <div class="right"> - <gr-button - link="" - id="cancelButton" - class="action cancel" - on-click="_cancelTapHandler" - >Cancel</gr-button - > - <template is="dom-if" if="[[canBeStarted]]"> - <!-- Use 'Send' here as the change may only about reviewers / ccs - and when this button is visible, the next button will always - be 'Start review' --> - <gr-tooltip-content - has-tooltip="" - title$="[[_saveTooltip]]" - > - <gr-button - link="" - disabled="[[_isState(knownLatestState, 'not-latest')]]" - class="action save" - on-click="_saveClickHandler" - >Send As WIP</gr-button - > - </gr-tooltip-content> - </template> - <gr-tooltip-content - has-tooltip="" - title$="[[_computeSendButtonTooltip(canBeStarted, _commentEditing)]]" - > - <gr-button - id="sendButton" - primary="" - disabled="[[_sendDisabled]]" - class="action send" - on-click="_sendTapHandler" - >[[_sendButtonLabel]] - </gr-button> - </gr-tooltip-content> - </div> - </section> - </gr-endpoint-decorator> - </div> - </div> -`;
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 5ea5cb4..78afaca 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
@@ -61,17 +61,14 @@ UrlEncodedCommentId, } from '../../../types/common'; import {CommentThread} from '../../../utils/comment-util'; -import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces'; -import { - AccountInfoInput, - GrAccountList, -} from '../../shared/gr-account-list/gr-account-list'; +import {GrAccountList} from '../../shared/gr-account-list/gr-account-list'; import {GrLabelScoreRow} from '../gr-label-score-row/gr-label-score-row'; import {GrLabelScores} from '../gr-label-scores/gr-label-scores'; import {GrThreadList} from '../gr-thread-list/gr-thread-list'; import {GrAutocomplete} from '../../shared/gr-autocomplete/gr-autocomplete'; - -const basicFixture = fixtureFromElement('gr-reply-dialog'); +import {fixture, html} from '@open-wc/testing-helpers'; +import {accountKey} from '../../../utils/account-util'; +import {GrButton} from '../../shared/gr-button/gr-button'; function cloneableResponse(status: number, text: string) { return { @@ -102,12 +99,6 @@ let setDraftCommentStub: sinon.SinonStub; let eraseDraftCommentStub: sinon.SinonStub; - const emptyAccountInfoInputChanges = - [] as unknown as PolymerDeepPropertyChange< - AccountInfoInput[], - AccountInfoInput[] - >; - let lastId = 1; const makeAccount = function () { return {_account_id: lastId++ as AccountId}; @@ -123,7 +114,10 @@ stubRestApi('getChange').returns(Promise.resolve({...createChange()})); stubRestApi('getChangeSuggestedReviewers').returns(Promise.resolve([])); - element = basicFixture.instantiate(); + element = await fixture<GrReplyDialog>(html` + <gr-reply-dialog></gr-reply-dialog> + `); + element.change = { ...createChange(), _number: changeNum, @@ -162,17 +156,13 @@ setDraftCommentStub = stubStorage('setDraftComment'); eraseDraftCommentStub = stubStorage('eraseDraftComment'); - // sinon.stub(patchSetUtilMockProxy, 'fetchChangeUpdates') - // .returns(Promise.resolve({isLatest: true})); - - // Allow the elements created by dom-repeat to be stamped. - await flush(); + await element.updateComplete; }); function stubSaveReview( jsonResponseProducer: (input: ReviewInput) => ReviewResult | void ) { - return sinon.stub(element, '_saveReview').callsFake( + return sinon.stub(element, 'saveReview').callsFake( review => new Promise((resolve, reject) => { try { @@ -206,15 +196,18 @@ test('default to publishing draft comments with reply', async () => { // Async tick is needed because iron-selector content is distributed and // distributed content requires an observer to be set up. - await flush(); + await element.updateComplete; element.draft = 'I wholeheartedly disapprove'; + element.draftCommentThreads = [createCommentThread([createComment()])]; + + element.includeComments = true; const saveReviewPromise = interceptSaveReview(); // This is needed on non-Blink engines most likely due to the ways in // which the dom-repeat elements are stamped. - await flush(); + await element.updateComplete; tap(queryAndAssert(element, '.send')); - await flush(); + await element.updateComplete; const review = await saveReviewPromise; assert.deepEqual(review, { @@ -242,13 +235,13 @@ }); test('modified attention set', async () => { - await flush(); - element._account = {_account_id: 123 as AccountId}; - element._newAttentionSet = new Set([314 as AccountId]); + await element.updateComplete; + element.account = {_account_id: 123 as AccountId}; + element.newAttentionSet = new Set([314 as AccountId]); const saveReviewPromise = interceptSaveReview(); const modifyButton = queryAndAssert(element, '.edit-attention-button'); tap(modifyButton); - await flush(); + await element.updateComplete; tap(queryAndAssert(element, '.send')); const review = await saveReviewPromise; @@ -269,13 +262,13 @@ }); test('modified attention set by anonymous', async () => { - await flush(); - element._account = {}; - element._newAttentionSet = new Set([314 as AccountId]); + await element.updateComplete; + element.account = {}; + element.newAttentionSet = new Set([314 as AccountId]); const saveReviewPromise = interceptSaveReview(); const modifyButton = queryAndAssert(element, '.edit-attention-button'); tap(modifyButton); - await flush(); + await element.updateComplete; tap(queryAndAssert(element, '.send')); const review = await saveReviewPromise; @@ -293,11 +286,12 @@ remove_from_attention_set: [], ignore_automatic_attention_set_rules: true, }); - element._newAttentionSet = new Set(); - await flush(); + element.newAttentionSet = new Set(); + await element.updateComplete; }); - function checkComputeAttention( + async function checkComputeAttention( + element: GrReplyDialog, status: ChangeStatus, userId?: AccountId, reviewerIds?: AccountId[], @@ -309,12 +303,11 @@ hasDraft = true, includeComments = true ) { - const user = {_account_id: userId}; - const reviewers = { - base: reviewerIds?.map(id => { + element.account = {_account_id: userId}; + element.reviewers = + reviewerIds?.map(id => { return {_account_id: id}; - }), - } as PolymerDeepPropertyChange<AccountInfoInput[], AccountInfoInput[]>; + }) ?? []; let draftThreads: CommentThread[] = []; if (hasDraft) { draftThreads = [ @@ -333,6 +326,9 @@ ...createChange(), owner: {_account_id: ownerId}, status, + reviewers: { + [ReviewerState.REVIEWER]: element.reviewers, + }, }; attSetIds?.forEach(id => { if (!change.attention_set) change.attention_set = {}; @@ -348,25 +344,20 @@ }; } element.change = change; - element._reviewers = reviewers.base!; + element.ccs = []; + element.draftCommentThreads = draftThreads; + element.includeComments = includeComments; + element.hasDrafts = draftThreads.length > 0; - flush(); - const hasDrafts = draftThreads.length > 0; - element._computeNewAttention( - user, - reviewers, - emptyAccountInfoInputChanges, - change, - draftThreads, - includeComments, - undefined, - hasDrafts - ); - assert.sameMembers([...element._newAttentionSet], expectedIds!); + await element.updateComplete; + + element.computeNewAttention(); + assert.sameMembers([...element.newAttentionSet], expectedIds!); } - test('computeNewAttention NEW', () => { - checkComputeAttention( + test('computeNewAttention NEW', async () => { + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [], @@ -375,7 +366,8 @@ [], [999 as AccountId] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [], @@ -384,7 +376,8 @@ [], [999 as AccountId] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [22 as AccountId], @@ -393,7 +386,8 @@ [], [999 as AccountId] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [22 as AccountId], @@ -402,7 +396,8 @@ [], [22 as AccountId, 999 as AccountId] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [22 as AccountId], @@ -411,7 +406,8 @@ [22 as AccountId], [22 as AccountId, 999 as AccountId] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [22 as AccountId, 33 as AccountId], @@ -421,7 +417,8 @@ [22 as AccountId, 33 as AccountId, 999 as AccountId] ); // If the owner replies, then do not add them. - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [], @@ -430,7 +427,8 @@ [], [] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [], @@ -439,7 +437,8 @@ [], [] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [22 as AccountId], @@ -449,7 +448,8 @@ [] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [22 as AccountId], @@ -458,7 +458,8 @@ [22 as AccountId], [22 as AccountId] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [22 as AccountId, 33 as AccountId], @@ -467,7 +468,8 @@ [22 as AccountId], [22 as AccountId, 33 as AccountId] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [22 as AccountId, 33 as AccountId], @@ -476,7 +478,8 @@ [22 as AccountId], [22 as AccountId] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [22 as AccountId, 33 as AccountId], @@ -485,7 +488,8 @@ [22 as AccountId, 33 as AccountId], [22 as AccountId, 33 as AccountId] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [22 as AccountId, 33 as AccountId], @@ -495,7 +499,8 @@ [22 as AccountId, 33 as AccountId] ); // with uploader - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [], @@ -505,7 +510,8 @@ [2 as AccountId], 2 as AccountId ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [], @@ -515,7 +521,8 @@ [2 as AccountId], 2 as AccountId ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.NEW, 1 as AccountId, [], @@ -527,8 +534,9 @@ ); }); - test('computeNewAttention MERGED', () => { - checkComputeAttention( + test('computeNewAttention MERGED', async () => { + await checkComputeAttention( + element, ChangeStatus.MERGED, undefined, [], @@ -539,7 +547,8 @@ undefined, false ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [], @@ -550,7 +559,8 @@ undefined, false ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [], @@ -561,7 +571,8 @@ undefined, true ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [], @@ -573,7 +584,8 @@ true, false ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [], @@ -584,7 +596,8 @@ undefined, false ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [22 as AccountId], @@ -595,7 +608,8 @@ undefined, false ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [22 as AccountId], @@ -606,7 +620,8 @@ undefined, false ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [22 as AccountId], @@ -615,7 +630,8 @@ [22 as AccountId], [] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [22 as AccountId, 33 as AccountId], @@ -624,7 +640,8 @@ [22 as AccountId], [33 as AccountId] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [], @@ -633,7 +650,8 @@ [], [] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [], @@ -644,7 +662,8 @@ undefined, true ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [], @@ -653,7 +672,8 @@ [], [] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [], @@ -664,7 +684,8 @@ undefined, true ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [22 as AccountId], @@ -673,7 +694,8 @@ [], [] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [22 as AccountId], @@ -682,7 +704,8 @@ [22 as AccountId], [] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [22 as AccountId, 33 as AccountId], @@ -691,7 +714,8 @@ [22 as AccountId], [33 as AccountId] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [22 as AccountId, 33 as AccountId], @@ -700,7 +724,8 @@ [22 as AccountId], [] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [22 as AccountId, 33 as AccountId], @@ -709,7 +734,8 @@ [22 as AccountId, 33 as AccountId], [] ); - checkComputeAttention( + await checkComputeAttention( + element, ChangeStatus.MERGED, 1 as AccountId, [22 as AccountId, 33 as AccountId], @@ -720,118 +746,86 @@ ); }); - test('computeNewAttention when adding reviewers', () => { - const user = {_account_id: 1 as AccountId}; - const reviewers = { - base: [ - {_account_id: 1 as AccountId, _pendingAdd: true}, - {_account_id: 2 as AccountId, _pendingAdd: true}, - ], - } as PolymerDeepPropertyChange<AccountInfoInput[], AccountInfoInput[]>; - const change = { + test('computeNewAttention when adding reviewers', async () => { + element.account = {_account_id: 1 as AccountId}; + element.change = { ...createChange(), owner: {_account_id: 5 as AccountId}, status: ChangeStatus.NEW, attention_set: {}, }; - element.change = change; - element._reviewers = reviewers.base; - flush(); + // let rebuildReviewers triggered by change update finish running + await element.updateComplete; - element._computeNewAttention( - user, - reviewers, - emptyAccountInfoInputChanges, - change, - [], - true - ); - assert.sameMembers([...element._newAttentionSet], [1, 2]); + element.reviewers = [ + {_account_id: 1 as AccountId, _pendingAdd: true}, + {_account_id: 2 as AccountId, _pendingAdd: true}, + ]; + element.ccs = []; + element.draftCommentThreads = []; + element.includeComments = true; + element.canBeStarted = true; + await element.updateComplete; + assert.sameMembers([...element.newAttentionSet], [1, 2]); // If the user votes on the change, then they should not be added to the // attention set, even if they have just added themselves as reviewer. // But voting should also add the owner (5). - const labelsChanged = true; - element._computeNewAttention( - user, - reviewers, - emptyAccountInfoInputChanges, - change, - [], - true, - labelsChanged - ); - assert.sameMembers([...element._newAttentionSet], [2, 5]); + element.labelsChanged = true; + await element.updateComplete; + assert.sameMembers([...element.newAttentionSet], [2, 5]); }); - test('computeNewAttention when sending wip change for review', () => { - const reviewers = { - base: [{...createAccountWithId(2)}, {...createAccountWithId(3)}], - } as PolymerDeepPropertyChange<AccountInfoInput[], AccountInfoInput[]>; - const change = { + test('computeNewAttention when sending wip change for review', async () => { + element.change = { ...createChange(), owner: {_account_id: 1 as AccountId}, status: ChangeStatus.NEW, attention_set: {}, }; - element.change = change; - element._reviewers = reviewers.base; - flush(); + // let rebuildReviewers triggered by change update finish running + await element.updateComplete; + + element.reviewers = [ + {...createAccountWithId(2)}, + {...createAccountWithId(3)}, + ]; + + element.ccs = []; + element.draftCommentThreads = []; + element.includeComments = false; + element.account = {_account_id: 1 as AccountId}; + + await element.updateComplete; // For an active change there is no reason to add anyone to the set. - let user = {_account_id: 1 as AccountId}; - element._computeNewAttention( - user, - reviewers, - emptyAccountInfoInputChanges, - change, - [], - false - ); - assert.sameMembers([...element._newAttentionSet], []); + element.computeNewAttention(); + assert.sameMembers([...element.newAttentionSet], []); // If the change is "work in progress" and the owner sends a reply, then // add all reviewers. element.canBeStarted = true; - flush(); - user = {_account_id: 1 as AccountId}; - element._computeNewAttention( - user, - reviewers, - emptyAccountInfoInputChanges, - change, - [], - false - ); - assert.sameMembers([...element._newAttentionSet], [2, 3]); + element.computeNewAttention(); + await element.updateComplete; + assert.sameMembers([...element.newAttentionSet], [2, 3]); // ... but not when someone else replies. - user = {_account_id: 4 as AccountId}; - element._computeNewAttention( - user, - reviewers, - emptyAccountInfoInputChanges, - change, - [], - false - ); - assert.sameMembers([...element._newAttentionSet], []); + element.account = {_account_id: 4 as AccountId}; + element.computeNewAttention(); + assert.sameMembers([...element.newAttentionSet], []); }); test('computeNewAttentionAccounts', () => { - element._reviewers = [ + element.reviewers = [ {_account_id: 123 as AccountId, display_name: 'Ernie'}, {_account_id: 321 as AccountId, display_name: 'Bert'}, ]; - element._ccs = [{_account_id: 7 as AccountId, display_name: 'Elmo'}]; - const compute = (currentAtt: AccountId[], newAtt: AccountId[]) => - element - ._computeNewAttentionAccounts( - undefined, - new Set(currentAtt), - new Set(newAtt) - ) - .map(a => a!._account_id); + element.ccs = [{_account_id: 7 as AccountId, display_name: 'Elmo'}]; + const compute = (currentAtt: AccountId[], newAtt: AccountId[]) => { + element.currentAttentionSet = new Set(currentAtt); + element.newAttentionSet = new Set(newAtt); + return element.computeNewAttentionAccounts().map(a => a?._account_id); + }; assert.sameMembers(compute([], []), []); assert.sameMembers(compute([], [999 as AccountId]), [999 as AccountId]); @@ -850,7 +844,7 @@ ); }); - test('_computeCommentAccounts', () => { + test('computeCommentAccounts', () => { element.change = { ...createChange(), labels: { @@ -906,7 +900,7 @@ ]), }, ]; - const actualAccounts = [...element._computeCommentAccounts(threads)]; + const actualAccounts = [...element.computeCommentAccounts(threads)]; // Account 3 is not included, because the comment is resolved *and* they // have given the highest possible vote on the Code-Review label. assert.sameMembers(actualAccounts, [1, 2, 4]); @@ -921,13 +915,15 @@ // Async tick is needed because iron-selector content is distributed and // distributed content requires an observer to be set up. - await flush(); + await element.updateComplete; element.draft = 'I wholeheartedly disapprove'; + element.draftCommentThreads = [createCommentThread([createComment()])]; + const saveReviewPromise = interceptSaveReview(); // This is needed on non-Blink engines most likely due to the ways in // which the dom-repeat elements are stamped. - await flush(); + await element.updateComplete; tap(queryAndAssert(element, '.send')); const review = await saveReviewPromise; @@ -954,6 +950,8 @@ test('label picker', async () => { element.draft = 'I wholeheartedly disapprove'; + element.draftCommentThreads = [createCommentThread([createComment()])]; + const saveReviewPromise = interceptSaveReview(); sinon.stub(element.getLabelScores(), 'getLabelValues').callsFake(() => { @@ -965,12 +963,12 @@ // This is needed on non-Blink engines most likely due to the ways in // which the dom-repeat elements are stamped. - await flush(); + await element.updateComplete; tap(queryAndAssert(element, '.send')); assert.isTrue(element.disabled); const review = await saveReviewPromise; - await flush(); + await element.updateComplete; assert.isFalse( element.disabled, 'Element should be enabled when done sending reply.' @@ -991,29 +989,35 @@ ], }, reviewers: [], - add_to_attention_set: [], + add_to_attention_set: [ + {user: 999, reason: '<GERRIT_ACCOUNT_1> replied on the change'}, + ], remove_from_attention_set: [], ignore_automatic_attention_set_rules: true, }); }); test('keep draft comments with reply', async () => { + element.draftCommentThreads = [createCommentThread([createComment()])]; + await element.updateComplete; + tap(queryAndAssert(element, '#includeComments')); - assert.equal(element._includeComments, false); + assert.equal(element.includeComments, false); // Async tick is needed because iron-selector content is distributed and // distributed content requires an observer to be set up. - await flush(); + await element.updateComplete; element.draft = 'I wholeheartedly disapprove'; + const saveReviewPromise = interceptSaveReview(); // This is needed on non-Blink engines most likely due to the ways in // which the dom-repeat elements are stamped. - await flush(); + await element.updateComplete; tap(queryAndAssert(element, '.send')); const review = await saveReviewPromise; - await flush(); + await element.updateComplete; assert.deepEqual(review, { drafts: 'KEEP', labels: { @@ -1036,7 +1040,6 @@ }); test('getlabelValue returns value', async () => { - await flush(); const el = queryAndAssert<GrLabelScoreRow>( queryAndAssert(element, 'gr-label-scores'), 'gr-label-score-row[name="Verified"]' @@ -1046,7 +1049,6 @@ }); test('getlabelValue when no score is selected', async () => { - await flush(); const el = queryAndAssert<GrLabelScoreRow>( queryAndAssert(element, 'gr-label-scores'), 'gr-label-score-row[name="Code-Review"]' @@ -1056,12 +1058,12 @@ }); test('setlabelValue', async () => { - element._account = {_account_id: 1 as AccountId}; - await flush(); + element.account = {_account_id: 1 as AccountId}; + await element.updateComplete; const label = 'Verified'; const value = '+1'; element.setLabelValue(label, value); - await flush(); + await element.updateComplete; const labels = queryAndAssert<GrLabelScores>( element, @@ -1122,9 +1124,9 @@ '.reviewerConfirmationButtons gr-button:last-child' ); - element._ccPendingConfirmation = null; - element._reviewerPendingConfirmation = null; - flush(); + element.ccPendingConfirmation = null; + element.reviewerPendingConfirmation = null; + await element.updateComplete; assert.isFalse( isVisible(queryAndAssert(element, 'reviewerConfirmationOverlay')) ); @@ -1136,29 +1138,29 @@ name: 'name' as GroupName, }; if (cc) { - element._ccPendingConfirmation = { + element.ccPendingConfirmation = { group, confirm: false, count: 1, }; } else { - element._reviewerPendingConfirmation = { + element.reviewerPendingConfirmation = { group, confirm: false, count: 1, }; } - flush(); + await element.updateComplete; if (cc) { assert.deepEqual( - element._ccPendingConfirmation, - element._pendingConfirmationDetails + element.ccPendingConfirmation, + element.pendingConfirmationDetails ); } else { assert.deepEqual( - element._reviewerPendingConfirmation, - element._pendingConfirmationDetails + element.reviewerPendingConfirmation, + element.pendingConfirmationDetails ); } @@ -1203,13 +1205,13 @@ // Reopen confirmation dialog. observer = overlayObserver('opened'); if (cc) { - element._ccPendingConfirmation = { + element.ccPendingConfirmation = { group, confirm: false, count: 1, }; } else { - element._reviewerPendingConfirmation = { + element.reviewerPendingConfirmation = { group, confirm: false, count: 1, @@ -1271,16 +1273,15 @@ testConfirmationDialog(false); }); - test('_getStorageLocation', () => { - const actual = element._getStorageLocation(); + test('getStorageLocation', () => { + const actual = element.getStorageLocation(); assert.equal(actual.changeNum, changeNum); assert.equal(actual.patchNum, '@change'); assert.equal(actual.path, '@change'); }); - test('_reviewersMutated when account-text-change is fired from ccs', () => { - flush(); - assert.isFalse(element._reviewersMutated); + test('reviewersMutated when account-text-change is fired from ccs', () => { + assert.isFalse(element.reviewersMutated); assert.isTrue(queryAndAssert<GrAccountList>(element, '#ccs').allowAnyInput); assert.isFalse( queryAndAssert<GrAccountList>(element, '#reviewers').allowAnyInput @@ -1288,7 +1289,7 @@ queryAndAssert(element, '#ccs').dispatchEvent( new CustomEvent('account-text-changed', {bubbles: true, composed: true}) ); - assert.isTrue(element._reviewersMutated); + assert.isTrue(element.reviewersMutated); }); test('gets draft from storage on open', () => { @@ -1329,17 +1330,19 @@ const clock = sinon.useFakeTimers(); const firstEdit = 'hello'; - const location = element._getStorageLocation(); + const location = element.getStorageLocation(); element.draft = firstEdit; clock.tick(1000); - await flush(); + await element.updateComplete; + await element.storeTask?.flush(); assert.isTrue(setDraftCommentStub.calledWith(location, firstEdit)); element.draft = ''; clock.tick(1000); - await flush(); + await element.updateComplete; + await element.storeTask?.flush(); assert.isTrue(eraseDraftCommentStub.calledWith(location)); }); @@ -1369,7 +1372,7 @@ }; addListenerForTest(document, 'server-error', listener); - await flush(); + await element.updateComplete; element.send(false, false); await promise; }); @@ -1395,7 +1398,7 @@ // Async tick is needed because iron-selector content is distributed and // distributed content requires an observer to be set up. - await flush(); + await element.updateComplete; element.send(false, false); await promise; }); @@ -1406,11 +1409,11 @@ const reviewer2 = makeGroup(); const cc1 = makeAccount(); const cc2 = makeGroup(); - let filter = element._filterReviewerSuggestionGenerator(false); + let filter = element.filterReviewerSuggestionGenerator(false); - element._owner = owner; - element._reviewers = [reviewer1, reviewer2]; - element._ccs = [cc1, cc2]; + element.owner = owner; + element.reviewers = [reviewer1, reviewer2]; + element.ccs = [cc1, cc2]; assert.isTrue(filter({account: makeAccount()} as Suggestion)); assert.isTrue(filter({group: makeGroup()} as Suggestion)); @@ -1422,35 +1425,40 @@ assert.isFalse(filter({account: reviewer1} as Suggestion)); assert.isFalse(filter({group: reviewer2} as Suggestion)); - filter = element._filterReviewerSuggestionGenerator(true); + filter = element.filterReviewerSuggestionGenerator(true); // Existing and pending CCs should be excluded when isCC = true;. assert.isFalse(filter({account: cc1} as Suggestion)); assert.isFalse(filter({group: cc2} as Suggestion)); }); - test('_focusOn', async () => { - const chooseFocusTargetSpy = sinon.spy(element, '_chooseFocusTarget'); - element._focusOn(); - await flush(); + test('focusOn', async () => { + const clock = sinon.useFakeTimers(); + const chooseFocusTargetSpy = sinon.spy(element, 'chooseFocusTarget'); + element.focusOn(); + // element.focus() is called after a setTimeout(). The focusOn() method + // does not trigger any changes in the element hence element.updateComplete + // resolves immediately and cannot be used here, hence tick the clock here + // explicitly instead + clock.tick(1); assert.equal(chooseFocusTargetSpy.callCount, 1); assert.equal(element?.shadowRoot?.activeElement?.tagName, 'GR-TEXTAREA'); assert.equal(element?.shadowRoot?.activeElement?.id, 'textarea'); - element._focusOn(element.FocusTarget.ANY); - await flush(); + element.focusOn(element.FocusTarget.ANY); + clock.tick(1); assert.equal(chooseFocusTargetSpy.callCount, 2); assert.equal(element?.shadowRoot?.activeElement?.tagName, 'GR-TEXTAREA'); assert.equal(element?.shadowRoot?.activeElement?.id, 'textarea'); - element._focusOn(element.FocusTarget.BODY); - await flush(); + element.focusOn(element.FocusTarget.BODY); + clock.tick(1); assert.equal(chooseFocusTargetSpy.callCount, 2); assert.equal(element?.shadowRoot?.activeElement?.tagName, 'GR-TEXTAREA'); assert.equal(element?.shadowRoot?.activeElement?.id, 'textarea'); - element._focusOn(element.FocusTarget.REVIEWERS); - await flush(); + element.focusOn(element.FocusTarget.REVIEWERS); + clock.tick(1); assert.equal(chooseFocusTargetSpy.callCount, 2); assert.equal( element?.shadowRoot?.activeElement?.tagName, @@ -1458,44 +1466,45 @@ ); assert.equal(element?.shadowRoot?.activeElement?.id, 'reviewers'); - element._focusOn(element.FocusTarget.CCS); - await flush(); + element.focusOn(element.FocusTarget.CCS); + clock.tick(1); assert.equal(chooseFocusTargetSpy.callCount, 2); assert.equal( element?.shadowRoot?.activeElement?.tagName, 'GR-ACCOUNT-LIST' ); assert.equal(element?.shadowRoot?.activeElement?.id, 'ccs'); + clock.restore(); }); - test('_chooseFocusTarget', () => { - element._account = undefined; - assert.strictEqual(element._chooseFocusTarget(), element.FocusTarget.BODY); + test('chooseFocusTarget', () => { + element.account = undefined; + assert.strictEqual(element.chooseFocusTarget(), element.FocusTarget.BODY); - element._account = {_account_id: 1 as AccountId}; - assert.strictEqual(element._chooseFocusTarget(), element.FocusTarget.BODY); + element.account = {_account_id: 1 as AccountId}; + assert.strictEqual(element.chooseFocusTarget(), element.FocusTarget.BODY); element.change!.owner = {_account_id: 2 as AccountId}; - assert.strictEqual(element._chooseFocusTarget(), element.FocusTarget.BODY); + assert.strictEqual(element.chooseFocusTarget(), element.FocusTarget.BODY); element.change!.owner._account_id = 1 as AccountId; assert.strictEqual( - element._chooseFocusTarget(), + element.chooseFocusTarget(), element.FocusTarget.REVIEWERS ); - element._reviewers = []; + element.reviewers = []; assert.strictEqual( - element._chooseFocusTarget(), + element.chooseFocusTarget(), element.FocusTarget.REVIEWERS ); - element._reviewers.push({}); - assert.strictEqual(element._chooseFocusTarget(), element.FocusTarget.BODY); + element.reviewers.push({}); + assert.strictEqual(element.chooseFocusTarget(), element.FocusTarget.BODY); }); test('only send labels that have changed', async () => { - await flush(); + await element.updateComplete; stubSaveReview((review: ReviewInput) => { assert.deepEqual(review?.labels, { 'Code-Review': 0, @@ -1520,9 +1529,7 @@ await promise; }); - test('moving from cc to reviewer', () => { - flush(); - + test('moving from cc to reviewer', async () => { const reviewer1 = makeAccount(); const reviewer2 = makeAccount(); const reviewer3 = makeAccount(); @@ -1530,41 +1537,36 @@ const cc2 = makeAccount(); const cc3 = makeAccount(); const cc4 = makeAccount(); - element._reviewers = [reviewer1, reviewer2, reviewer3]; - element._ccs = [cc1, cc2, cc3, cc4]; - element.push('_reviewers', cc1); - element.$.reviewers.dispatchEvent( + element.reviewers = [reviewer1, reviewer2, reviewer3]; + element.ccs = [cc1, cc2, cc3, cc4]; + element.reviewers.push(cc1); + element.reviewersList!.dispatchEvent( new CustomEvent('account-added', { detail: {account: cc1}, }) ); - flush(); + await element.updateComplete; - assert.deepEqual(element._reviewers, [ - reviewer1, - reviewer2, - reviewer3, - cc1, - ]); - assert.deepEqual(element._ccs, [cc2, cc3, cc4]); + assert.deepEqual(element.reviewers, [reviewer1, reviewer2, reviewer3, cc1]); + assert.deepEqual(element.ccs, [cc2, cc3, cc4]); - element.push('_reviewers', cc4); - element.$.reviewers.dispatchEvent( + element.reviewers.push(cc4); + element.reviewersList!.dispatchEvent( new CustomEvent('account-added', { detail: {account: cc4}, }) ); - flush(); + await element.updateComplete; - element.push('_reviewers', cc3); - element.$.reviewers.dispatchEvent( + element.reviewers.push(cc3); + element.reviewersList!.dispatchEvent( new CustomEvent('account-added', { detail: {account: cc3}, }) ); - flush(); + await element.updateComplete; - assert.deepEqual(element._reviewers, [ + assert.deepEqual(element.reviewers, [ reviewer1, reviewer2, reviewer3, @@ -1572,54 +1574,65 @@ cc4, cc3, ]); - assert.deepEqual(element._ccs, [cc2]); + assert.deepEqual(element.ccs, [cc2]); }); - test('update attention section when reviewers and ccs change', () => { - element._account = makeAccount(); - element._reviewers = [makeAccount(), makeAccount()]; - element._ccs = [makeAccount(), makeAccount()]; + test('update attention section when reviewers and ccs change', async () => { + element.account = makeAccount(); + element.reviewers = [makeAccount(), makeAccount()]; + element.ccs = [makeAccount(), makeAccount()]; element.draftCommentThreads = []; + const modifyButton = queryAndAssert(element, '.edit-attention-button'); tap(modifyButton); - flush(); - // "Modify" button disabled, because "Send" button is disabled. - assert.isFalse(element._attentionExpanded); + await element.updateComplete; + + assert.isFalse(element.attentionExpanded); + element.draft = 'a test comment'; + await element.updateComplete; + tap(modifyButton); - flush(); - assert.isTrue(element._attentionExpanded); + + await element.updateComplete; + + assert.isTrue(element.attentionExpanded); let accountLabels = Array.from( queryAll(element, '.attention-detail gr-account-label') ); assert.equal(accountLabels.length, 5); - element.push('_reviewers', makeAccount()); - element.push('_ccs', makeAccount()); - flush(); + element.reviewers = [...element.reviewers, makeAccount()]; + element.ccs = [...element.ccs, makeAccount()]; + await element.updateComplete; // The 'attention modified' section collapses and resets when reviewers or // ccs change. - assert.isFalse(element._attentionExpanded); + assert.isFalse(element.attentionExpanded); tap(queryAndAssert(element, '.edit-attention-button')); - flush(); + await element.updateComplete; - assert.isTrue(element._attentionExpanded); + assert.isTrue(element.attentionExpanded); accountLabels = Array.from( queryAll(element, '.attention-detail gr-account-label') ); assert.equal(accountLabels.length, 7); - element.pop('_reviewers'); - element.pop('_reviewers'); - element.pop('_ccs'); - element.pop('_ccs'); + element.reviewers.pop(); + element.reviewers.pop(); + element.ccs.pop(); + element.ccs.pop(); + element.reviewers = [...element.reviewers]; + element.ccs = [...element.ccs]; // trigger willUpdate observer + + await element.updateComplete; tap(queryAndAssert(element, '.edit-attention-button')); - flush(); + + await element.updateComplete; accountLabels = Array.from( queryAll(element, '.attention-detail gr-account-label') @@ -1627,9 +1640,7 @@ assert.equal(accountLabels.length, 3); }); - test('moving from reviewer to cc', () => { - flush(); - + test('moving from reviewer to cc', async () => { const reviewer1 = makeAccount(); const reviewer2 = makeAccount(); const reviewer3 = makeAccount(); @@ -1637,38 +1648,38 @@ const cc2 = makeAccount(); const cc3 = makeAccount(); const cc4 = makeAccount(); - element._reviewers = [reviewer1, reviewer2, reviewer3]; - element._ccs = [cc1, cc2, cc3, cc4]; - element.push('_ccs', reviewer1); - element.$.ccs.dispatchEvent( + element.reviewers = [reviewer1, reviewer2, reviewer3]; + element.ccs = [cc1, cc2, cc3, cc4]; + element.ccs.push(reviewer1); + element.ccsList!.dispatchEvent( new CustomEvent('account-added', { detail: {account: reviewer1}, }) ); - flush(); + await element.updateComplete; - assert.deepEqual(element._reviewers, [reviewer2, reviewer3]); - assert.deepEqual(element._ccs, [cc1, cc2, cc3, cc4, reviewer1]); + assert.deepEqual(element.reviewers, [reviewer2, reviewer3]); + assert.deepEqual(element.ccs, [cc1, cc2, cc3, cc4, reviewer1]); - element.push('_ccs', reviewer3); - element.$.ccs.dispatchEvent( + element.ccs.push(reviewer3); + element.ccsList!.dispatchEvent( new CustomEvent('account-added', { detail: {account: reviewer3}, }) ); - flush(); + await element.updateComplete; - element.push('_ccs', reviewer2); - element.$.ccs.dispatchEvent( + element.ccs.push(reviewer2); + element.ccsList!.dispatchEvent( new CustomEvent('account-added', { detail: {account: reviewer2}, }) ); - flush(); + await element.updateComplete; - assert.deepEqual(element._reviewers, []); - assert.deepEqual(element._ccs, [ + assert.deepEqual(element.reviewers, []); + assert.deepEqual(element.ccs, [ cc1, cc2, cc3, @@ -1680,7 +1691,6 @@ }); test('migrate reviewers between states', async () => { - flush(); const reviewers = queryAndAssert<GrAccountList>(element, '#reviewers'); const ccs = queryAndAssert<GrAccountList>(element, '#ccs'); const reviewer1 = makeAccount(); @@ -1688,14 +1698,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]; + await element.updateComplete; const mutations: ReviewerInput[] = []; @@ -1703,7 +1713,7 @@ mutations.push(...review.reviewers!); }); - assert.isFalse(element._reviewersMutated); + assert.isFalse(element.reviewersMutated); // Remove and add to other field. reviewers.dispatchEvent( @@ -1714,8 +1724,8 @@ }) ); - await flush(); - assert.isTrue(element._reviewersMutated); + await element.updateComplete; + assert.isTrue(element.reviewersMutated); ccs.entry!.dispatchEvent( new CustomEvent('add', { detail: {value: {account: reviewer1}}, @@ -1745,8 +1755,16 @@ }) ); - // Add to other field without removing from former field. - // (Currently not possible in UI, but this is a good consistency check). + assert.deepEqual( + element.reviewers.map(v => accountKey(v)), + [reviewer2, cc1].map(v => accountKey(v)) + ); + assert.deepEqual( + element.ccs.map(v => accountKey(v)), + [cc2, reviewer1].map(v => accountKey(v)) + ); + + // Add to Reviewer/CC which will automatically remove from CC/Reviewer. reviewers.entry!.dispatchEvent( new CustomEvent('add', { detail: {value: {account: cc2}}, @@ -1754,6 +1772,18 @@ bubbles: true, }) ); + + await element.updateComplete; + + assert.deepEqual( + element.reviewers.map(v => accountKey(v)), + [reviewer2, cc1, cc2].map(v => accountKey(v)) + ); + assert.deepEqual( + element.ccs.map(v => accountKey(v)), + [reviewer1].map(v => accountKey(v)) + ); + ccs.entry!.dispatchEvent( new CustomEvent('add', { detail: {value: {account: reviewer2}}, @@ -1762,6 +1792,17 @@ }) ); + await element.updateComplete; + + assert.deepEqual( + element.reviewers.map(v => accountKey(v)), + [cc1, cc2].map(v => accountKey(v)) + ); + assert.deepEqual( + element.ccs.map(v => accountKey(v)), + [reviewer1, reviewer2].map(v => accountKey(v)) + ); + const mapReviewer = function ( reviewer: AccountInfo, opt_state?: ReviewerState @@ -1777,8 +1818,9 @@ // Send and purge and verify moves, delete cc3. await element.send(false, false); - await flush(); - expect(mutations).to.have.lengthOf(5); + await element.updateComplete; + assert.equal(mutations.length, 5); + expect(mutations[0]).to.deep.equal( mapReviewer(cc1, ReviewerState.REVIEWER) ); @@ -1800,12 +1842,11 @@ }); test('Ignore removal requests if being added as reviewer/CC', async () => { - flush(); const reviewers = queryAndAssert<GrAccountList>(element, '#reviewers'); const ccs = queryAndAssert<GrAccountList>(element, '#ccs'); const reviewer1 = makeAccount(); - element._reviewers = [reviewer1]; - element._ccs = []; + element.reviewers = [reviewer1]; + element.ccs = []; element.change!.reviewers = { [ReviewerState.CC]: [], @@ -1843,11 +1884,11 @@ }); }); - test('emits cancel on esc key', () => { + test('emits cancel on esc key', async () => { const cancelHandler = sinon.spy(); element.addEventListener('cancel', cancelHandler); pressAndReleaseKeyOn(element, 27, null, 'Escape'); - flush(); + await element.updateComplete; assert.isTrue(cancelHandler.called); }); @@ -1866,26 +1907,30 @@ await promise; }); - test('_computeMessagePlaceholder', () => { + test('computeMessagePlaceholder', async () => { + element.canBeStarted = false; + await element.updateComplete; + assert.equal(element.messagePlaceholder, 'Say something nice...'); + + element.canBeStarted = true; + await element.updateComplete; assert.equal( - element._computeMessagePlaceholder(false), - 'Say something nice...' - ); - assert.equal( - element._computeMessagePlaceholder(true), + element.messagePlaceholder, 'Add a note for your reviewers...' ); }); - test('_computeSendButtonLabel', () => { - assert.equal(element._computeSendButtonLabel(false), 'Send'); - assert.equal( - element._computeSendButtonLabel(true), - 'Send and Start review' - ); + test('computeSendButtonLabel', async () => { + element.canBeStarted = false; + await element.updateComplete; + assert.equal(element.sendButtonLabel, 'Send'); + + element.canBeStarted = true; + await element.updateComplete; + assert.equal(element.sendButtonLabel, 'Send and Start review'); }); - test('_handle400Error reviewers and CCs', async () => { + test('handle400Error reviewers and CCs', async () => { const error1 = 'error 1'; const error2 = 'error 2'; const error3 = 'error 3'; @@ -1915,18 +1960,18 @@ }); }; addListenerForTest(document, 'server-error', listener); - element._handle400Error(cloneableResponse(400, text) as Response); + element.handle400Error(cloneableResponse(400, text) as Response); await promise; }); test('fires height change when the drafts comments load', async () => { // Flush DOM operations before binding to the autogrow event so we don't // catch the events fired from the initial layout. - await flush(); + await element.updateComplete; const autoGrowHandler = sinon.stub(); element.addEventListener('autogrow', autoGrowHandler); element.draftCommentThreads = []; - await flush(); + await element.updateComplete; assert.isTrue(autoGrowHandler.called); }); @@ -1937,18 +1982,18 @@ sendStub = sinon.stub(element, 'send').callsFake(() => Promise.resolve()); element.canBeStarted = true; // Flush to make both Start/Save buttons appear in DOM. - await flush(); + await element.updateComplete; }); test('start review sets ready', async () => { tap(queryAndAssert(element, '.send')); - await flush(); + await element.updateComplete; assert.isTrue(sendStub.calledWith(true, true)); }); test("save review doesn't set ready", async () => { tap(queryAndAssert(element, '.save')); - await flush(); + await element.updateComplete; assert.isTrue(sendStub.calledWith(true, false)); }); }); @@ -1975,7 +2020,7 @@ assert.isFalse(element.disabled); } - test('error occurs in _saveReview', () => { + test('error occurs in saveReview', () => { stubSaveReview(() => { throw expectedError; }); @@ -1996,209 +2041,163 @@ element.open(); assert.isFalse(refreshSpy.called); - assert.isTrue(element._savingComments); + assert.isTrue(element.savingComments); promise.resolve(); - await flush(); + await element.updateComplete; assert.isTrue(refreshSpy.called); - assert.isFalse(element._savingComments); + assert.isFalse(element.savingComments); }); test('no', () => { stubRestApi('hasPendingDiffDrafts').returns(0); element.open(); - assert.isFalse(element._savingComments); + assert.isFalse(element.savingComments); }); }); }); - test('_computeSendButtonDisabled_canBeStarted', () => { + test('computeSendButtonDisabled_canBeStarted', () => { // Mock canBeStarted - assert.isFalse( - element._computeSendButtonDisabled( - /* canBeStarted= */ true, - /* draftCommentThreads= */ [], - /* text= */ '', - /* reviewersMutated= */ false, - /* labelsChanged= */ false, - /* includeComments= */ false, - /* disabled= */ false, - /* commentEditing= */ false, - /* change= */ element.change, - /* account= */ makeAccount() - ) - ); + element.canBeStarted = true; + element.draftCommentThreads = []; + element.draft = ''; + element.reviewersMutated = false; + element.labelsChanged = false; + element.includeComments = false; + element.disabled = false; + element.commentEditing = false; + element.account = makeAccount(); + assert.isFalse(element.computeSendButtonDisabled()); }); - test('_computeSendButtonDisabled_allFalse', () => { + test('computeSendButtonDisabled_allFalse', () => { // Mock everything false - assert.isTrue( - element._computeSendButtonDisabled( - /* canBeStarted= */ false, - /* draftCommentThreads= */ [], - /* text= */ '', - /* reviewersMutated= */ false, - /* labelsChanged= */ false, - /* includeComments= */ false, - /* disabled= */ false, - /* commentEditing= */ false, - /* change= */ element.change, - /* account= */ makeAccount() - ) - ); + element.canBeStarted = false; + element.draftCommentThreads = []; + element.draft = ''; + element.reviewersMutated = false; + element.labelsChanged = false; + element.includeComments = false; + element.disabled = false; + element.commentEditing = false; + element.account = makeAccount(); + assert.isTrue(element.computeSendButtonDisabled()); }); - test('_computeSendButtonDisabled_draftCommentsSend', () => { - // Mock nonempty comment draft array, with sending comments. - assert.isFalse( - element._computeSendButtonDisabled( - /* canBeStarted= */ false, - /* draftCommentThreads= */ [ - {...createCommentThread([createComment()])}, - ], - /* text= */ '', - /* reviewersMutated= */ false, - /* labelsChanged= */ false, - /* includeComments= */ true, - /* disabled= */ false, - /* commentEditing= */ false, - /* change= */ element.change, - /* account= */ makeAccount() - ) - ); + test('computeSendButtonDisabled_draftCommentsSend', () => { + // Mock nonempty comment draft array; with sending comments. + element.canBeStarted = false; + element.draftCommentThreads = [{...createCommentThread([createComment()])}]; + element.draft = ''; + element.reviewersMutated = false; + element.labelsChanged = false; + element.includeComments = true; + element.disabled = false; + element.commentEditing = false; + element.account = makeAccount(); + assert.isFalse(element.computeSendButtonDisabled()); }); - test('_computeSendButtonDisabled_draftCommentsDoNotSend', () => { - // Mock nonempty comment draft array, without sending comments. - assert.isTrue( - element._computeSendButtonDisabled( - /* canBeStarted= */ false, - /* draftCommentThreads= */ [ - {...createCommentThread([createComment()])}, - ], - /* text= */ '', - /* reviewersMutated= */ false, - /* labelsChanged= */ false, - /* includeComments= */ false, - /* disabled= */ false, - /* commentEditing= */ false, - /* change= */ element.change, - /* account= */ makeAccount() - ) - ); + test('computeSendButtonDisabled_draftCommentsDoNotSend', () => { + // Mock nonempty comment draft array; without sending comments. + element.canBeStarted = false; + element.draftCommentThreads = [{...createCommentThread([createComment()])}]; + element.draft = ''; + element.reviewersMutated = false; + element.labelsChanged = false; + element.includeComments = false; + element.disabled = false; + element.commentEditing = false; + element.account = makeAccount(); + + assert.isTrue(element.computeSendButtonDisabled()); }); - test('_computeSendButtonDisabled_changeMessage', () => { + test('computeSendButtonDisabled_changeMessage', () => { // Mock nonempty change message. - assert.isFalse( - element._computeSendButtonDisabled( - /* canBeStarted= */ false, - /* draftCommentThreads= */ [ - {...createCommentThread([createComment()])}, - ], - /* text= */ 'test', - /* reviewersMutated= */ false, - /* labelsChanged= */ false, - /* includeComments= */ false, - /* disabled= */ false, - /* commentEditing= */ false, - /* change= */ element.change, - /* account= */ makeAccount() - ) - ); + element.canBeStarted = false; + element.draftCommentThreads = [{...createCommentThread([createComment()])}]; + element.draft = 'test'; + element.reviewersMutated = false; + element.labelsChanged = false; + element.includeComments = false; + element.disabled = false; + element.commentEditing = false; + element.account = makeAccount(); + + assert.isFalse(element.computeSendButtonDisabled()); }); - test('_computeSendButtonDisabled_reviewersChanged', () => { + test('computeSendButtonDisabledreviewersChanged', () => { // Mock reviewers mutated. - assert.isFalse( - element._computeSendButtonDisabled( - /* canBeStarted= */ false, - /* draftCommentThreads= */ [ - {...createCommentThread([createComment()])}, - ], - /* text= */ '', - /* reviewersMutated= */ true, - /* labelsChanged= */ false, - /* includeComments= */ false, - /* disabled= */ false, - /* commentEditing= */ false, - /* change= */ element.change, - /* account= */ makeAccount() - ) - ); + element.canBeStarted = false; + element.draftCommentThreads = [{...createCommentThread([createComment()])}]; + element.draft = ''; + element.reviewersMutated = true; + element.labelsChanged = false; + element.includeComments = false; + element.disabled = false; + element.commentEditing = false; + element.account = makeAccount(); + + assert.isFalse(element.computeSendButtonDisabled()); }); - test('_computeSendButtonDisabled_labelsChanged', () => { + test('computeSendButtonDisabled_labelsChanged', () => { // Mock labels changed. - assert.isFalse( - element._computeSendButtonDisabled( - /* canBeStarted= */ false, - /* draftCommentThreads= */ [ - {...createCommentThread([createComment()])}, - ], - /* text= */ '', - /* reviewersMutated= */ false, - /* labelsChanged= */ true, - /* includeComments= */ false, - /* disabled= */ false, - /* commentEditing= */ false, - /* change= */ element.change, - /* account= */ makeAccount() - ) - ); + element.canBeStarted = false; + element.draftCommentThreads = [{...createCommentThread([createComment()])}]; + element.draft = ''; + element.reviewersMutated = false; + element.labelsChanged = true; + element.includeComments = false; + element.disabled = false; + element.commentEditing = false; + element.account = makeAccount(); + + assert.isFalse(element.computeSendButtonDisabled()); }); - test('_computeSendButtonDisabled_dialogDisabled', () => { + test('computeSendButtonDisabled_dialogDisabled', () => { // Whole dialog is disabled. - assert.isTrue( - element._computeSendButtonDisabled( - /* canBeStarted= */ false, - /* draftCommentThreads= */ [ - {...createCommentThread([createComment()])}, - ], - /* text= */ '', - /* reviewersMutated= */ false, - /* labelsChanged= */ true, - /* includeComments= */ false, - /* disabled= */ true, - /* commentEditing= */ false, - /* change= */ element.change, - /* account= */ makeAccount() - ) - ); + element.canBeStarted = false; + element.draftCommentThreads = [{...createCommentThread([createComment()])}]; + element.draft = ''; + element.reviewersMutated = false; + element.labelsChanged = true; + element.includeComments = false; + element.disabled = true; + element.commentEditing = false; + element.account = makeAccount(); + + assert.isTrue(element.computeSendButtonDisabled()); }); - test('_computeSendButtonDisabled_existingVote', async () => { + test('computeSendButtonDisabled_existingVote', async () => { const account = createAccountWithId(); ( element.change!.labels![StandardLabels.CODE_REVIEW]! as DetailedLabelInfo ).all = [account]; - await flush(); + element.canBeStarted = false; + element.draftCommentThreads = [{...createCommentThread([createComment()])}]; + element.draft = ''; + element.reviewersMutated = false; + element.labelsChanged = false; + element.includeComments = false; + element.disabled = false; + element.commentEditing = false; + element.account = account; // User has already voted. - assert.isFalse( - element._computeSendButtonDisabled( - /* canBeStarted= */ false, - /* draftCommentThreads= */ [ - {...createCommentThread([createComment()])}, - ], - /* text= */ '', - /* reviewersMutated= */ false, - /* labelsChanged= */ false, - /* includeComments= */ false, - /* disabled= */ false, - /* commentEditing= */ false, - /* change= */ element.change, - /* account= */ account - ) - ); + assert.isFalse(element.computeSendButtonDisabled()); }); test('_submit blocked when no mutations exist', async () => { const sendStub = sinon.stub(element, 'send').returns(Promise.resolve()); element.draftCommentThreads = []; - await flush(); + await element.updateComplete; tap(queryAndAssert(element, 'gr-button.send')); assert.isFalse(sendStub.called); @@ -2215,7 +2214,7 @@ ]), }, ]; - await flush(); + await element.updateComplete; tap(queryAndAssert(element, 'gr-button.send')); assert.isTrue(sendStub.called); @@ -2225,11 +2224,11 @@ // Setting draftCommentThreads to an empty object causes _sendDisabled to be // computed to false. element.draftCommentThreads = []; - await flush(); + await element.updateComplete; assert.equal( element.getFocusStops()!.end, - queryAndAssert(element, '#cancelButton') + queryAndAssert<GrButton>(element, '#cancelButton') ); element.draftCommentThreads = [ { @@ -2243,16 +2242,17 @@ ]), }, ]; - await flush(); + await element.updateComplete; assert.equal( element.getFocusStops()!.end, - queryAndAssert(element, '#sendButton') + queryAndAssert<GrButton>(element, '#sendButton') ); }); - test('setPluginMessage', () => { + test('setPluginMessage', async () => { element.setPluginMessage('foo'); + await element.updateComplete; assert.equal(queryAndAssert(element, '#pluginMessage').textContent, 'foo'); }); });