Merge changes I855eefa5,I551eafd9
* changes:
Remove experiment PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
Remove setting explicit focus on comment textarea
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 0eb0227..6fba4e4 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
@@ -21,7 +21,6 @@
Timestamp,
} from '../../../types/common';
import {createChange} from '../../../test/test-data-generators';
-import {GrTextarea} from '../../shared/gr-textarea/gr-textarea';
import {GrButton} from '../../shared/gr-button/gr-button';
suite('gr-reply-dialog-it tests', () => {
@@ -120,14 +119,6 @@
getPluginLoader().loadPlugins([]);
await getPluginLoader().awaitPluginsLoaded();
await waitEventLoop();
- const textarea = queryAndAssert<GrTextarea>(
- element,
- 'gr-textarea'
- ).getNativeTextarea();
- textarea.value = 'LGTM';
- textarea.dispatchEvent(
- new CustomEvent('input', {bubbles: true, composed: true})
- );
await waitEventLoop();
const labelScoreRows = queryAndAssert(
element.getLabelScores(),
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 13de1cd..b540c89 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
@@ -3,12 +3,10 @@
* Copyright 2015 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import '@polymer/iron-autogrow-textarea/iron-autogrow-textarea';
import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
import '../../plugins/gr-endpoint-param/gr-endpoint-param';
import '../../plugins/gr-endpoint-slot/gr-endpoint-slot';
import '../../shared/gr-account-chip/gr-account-chip';
-import '../../shared/gr-textarea/gr-textarea';
import '../../shared/gr-button/gr-button';
import '../../shared/gr-icon/gr-icon';
import '../../shared/gr-formatted-text/gr-formatted-text';
@@ -51,7 +49,6 @@
AccountInfo,
AttentionSetInput,
ChangeInfo,
- CommentInput,
GroupInfo,
isAccount,
isDetailedLabelInfo,
@@ -86,7 +83,6 @@
isUnresolved,
UnsavedInfo,
} from '../../../utils/comment-util';
-import {GrTextarea} from '../../shared/gr-textarea/gr-textarea';
import {GrAccountChip} from '../../shared/gr-account-chip/gr-account-chip';
import {GrOverlay, GrOverlayStops} from '../../shared/gr-overlay/gr-overlay';
import {
@@ -117,11 +113,11 @@
LabelNameToValuesMap,
PatchSetNumber,
} from '../../../api/rest-api';
-import {css, html, PropertyValues, LitElement, nothing} from 'lit';
+import {css, html, PropertyValues, LitElement} from 'lit';
import {sharedStyles} from '../../../styles/shared-styles';
import {when} from 'lit/directives/when.js';
import {classMap} from 'lit/directives/class-map.js';
-import {BindValueChangeEvent, ValueChangedEvent} from '../../../types/events';
+import {ValueChangedEvent} from '../../../types/events';
import {customElement, property, state, query} from 'lit/decorators.js';
import {subscribe} from '../../lit/subscription-controller';
import {configModelToken} from '../../../models/config/config-model';
@@ -255,8 +251,6 @@
@query('#labelScores') labelScores?: GrLabelScores;
- @query('#textarea') textarea?: GrTextarea;
-
@query('#reviewerConfirmationOverlay')
reviewerConfirmationOverlay?: GrOverlay;
@@ -496,28 +490,13 @@
min-height: unset;
}
textareaContainer,
- #textarea,
gr-endpoint-decorator[name='reply-text'] {
display: flex;
width: 100%;
}
- #textarea {
- 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;
- }
#checkingStatusLabel,
#notLatestLabel {
margin-left: var(--spacing-l);
@@ -948,14 +927,6 @@
}
private renderPatchsetLevelComment() {
- if (
- !this.flagsService.isEnabled(
- KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
- )
- ) {
- return nothing;
- }
-
if (!this.patchsetLevelComment)
this.patchsetLevelComment = this.createDraft();
return html`
@@ -976,30 +947,6 @@
`;
}
- private renderPatchsetLevelTextarea() {
- if (
- this.flagsService.isEnabled(
- KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
- )
- )
- return nothing;
- return html` <gr-textarea
- id="textarea"
- class="message newReplyDialog"
- .autocomplete=${'on'}
- .placeholder=${this.messagePlaceholder}
- monospace
- ?disabled=${this.disabled}
- .rows=${4}
- .text=${this.patchsetLevelDraftMessage}
- @bind-value-changed=${(e: BindValueChangeEvent) => {
- this.patchsetLevelDraftMessage = e.detail.value ?? '';
- this.handleHeightChanged();
- }}
- >
- </gr-textarea>`;
- }
-
private renderReplyText() {
if (!this.change) return;
return html`
@@ -1013,7 +960,6 @@
>
<gr-endpoint-decorator name="reply-text">
${this.renderPatchsetLevelComment()}
- ${this.renderPatchsetLevelTextarea()}
<gr-endpoint-param name="change" .value=${this.change}>
</gr-endpoint-param>
</gr-endpoint-decorator>
@@ -1501,32 +1447,11 @@
reviewInput.remove_from_attention_set
);
- if (
- this.flagsService.isEnabled(
- KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
- )
- ) {
- const patchsetLevelComment = queryAndAssert<GrComment>(
- this,
- '#patchsetLevelComment'
- );
- await patchsetLevelComment.save();
- }
-
- if (
- this.patchsetLevelDraftMessage &&
- !this.flagsService.isEnabled(
- KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
- )
- ) {
- const comment: CommentInput = {
- message: this.patchsetLevelDraftMessage,
- unresolved: !this.patchsetLevelDraftIsResolved,
- };
- reviewInput.comments = {
- [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [comment],
- };
- }
+ const patchsetLevelComment = queryAndAssert<GrComment>(
+ this,
+ '#patchsetLevelComment'
+ );
+ await patchsetLevelComment.save();
assertIsDefined(this.change, 'change');
reviewInput.reviewers = this.computeReviewers();
@@ -1570,10 +1495,7 @@
if (!section || section === FocusTarget.ANY) {
section = this.chooseFocusTarget();
}
- if (section === FocusTarget.BODY) {
- const textarea = queryAndAssert<GrTextarea>(this, 'gr-textarea');
- setTimeout(() => textarea.getNativeTextarea().focus());
- } else if (section === FocusTarget.REVIEWERS) {
+ if (section === FocusTarget.REVIEWERS) {
const reviewerEntry = this.reviewersList?.focusStart;
setTimeout(() => reviewerEntry?.focus());
} else if (section === FocusTarget.CCS) {
@@ -1937,19 +1859,11 @@
bubbles: false,
})
);
- if (
- this.flagsService.isEnabled(
- KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
- )
- ) {
- const patchsetLevelComment = queryAndAssert<GrComment>(
- this,
- '#patchsetLevelComment'
- );
- await patchsetLevelComment.save();
- } else {
- queryAndAssert<GrTextarea>(this, 'gr-textarea').closeDropdown();
- }
+ const patchsetLevelComment = queryAndAssert<GrComment>(
+ this,
+ '#patchsetLevelComment'
+ );
+ await patchsetLevelComment.save();
this.rebuildReviewerArrays();
}
@@ -2139,14 +2053,9 @@
isDetailedLabelInfo(label) && getApprovalInfo(label, this.account!)
);
const revotingOrNewVote = this.labelsChanged || existingVote;
- let hasDrafts = this.includeComments && this.draftCommentThreads.length > 0;
- if (
- this.flagsService.isEnabled(
- KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
- )
- ) {
- hasDrafts = hasDrafts || this.patchsetLevelDraftMessage.length > 0;
- }
+ const hasDrafts =
+ (this.includeComments && this.draftCommentThreads.length > 0) ||
+ this.patchsetLevelDraftMessage.length > 0;
return (
!hasDrafts &&
!this.patchsetLevelDraftMessage.length &&
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 cfa4145..2001d6d 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
@@ -16,11 +16,7 @@
stubFlags,
stubRestApi,
} from '../../../test/test-utils';
-import {
- ChangeStatus,
- ReviewerState,
- SpecialFilePath,
-} from '../../../constants/constants';
+import {ChangeStatus, ReviewerState} from '../../../constants/constants';
import {JSON_PREFIX} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
import {StandardLabels} from '../../../utils/label-util';
import {
@@ -237,12 +233,12 @@
<section class="newReplyDialog textareaContainer">
<div class="patchsetLevelContainer resolved">
<gr-endpoint-decorator name="reply-text">
- <gr-textarea
- class="message monospace newReplyDialog"
- id="textarea"
- monospace=""
+ <gr-comment
+ hide-header=""
+ id="patchsetLevelComment"
+ permanent-editing-mode=""
>
- </gr-textarea>
+ </gr-comment>
<gr-endpoint-param name="change"> </gr-endpoint-param>
</gr-endpoint-decorator>
</div>
@@ -346,14 +342,6 @@
'Code-Review': 0,
Verified: 0,
},
- comments: {
- [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [
- {
- message: 'I wholeheartedly disapprove',
- unresolved: false,
- },
- ],
- },
reviewers: [],
add_to_attention_set: [
{reason: '<GERRIT_ACCOUNT_1> replied on the change', user: 999},
@@ -1090,14 +1078,6 @@
'Code-Review': -1,
Verified: -1,
},
- comments: {
- [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [
- {
- message: 'I wholeheartedly disapprove',
- unresolved: false,
- },
- ],
- },
reviewers: [],
add_to_attention_set: [
{user: 999, reason: '<GERRIT_ACCOUNT_1> replied on the change'},
@@ -1134,14 +1114,6 @@
'Code-Review': 0,
Verified: 0,
},
- comments: {
- [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [
- {
- message: 'I wholeheartedly disapprove',
- unresolved: false,
- },
- ],
- },
reviewers: [],
add_to_attention_set: [
{reason: '<GERRIT_ACCOUNT_1> replied on the change', user: 999},
@@ -1477,20 +1449,29 @@
// 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');
+ assert.equal(element?.shadowRoot?.activeElement?.tagName, 'GR-COMMENT');
+ assert.equal(
+ element?.shadowRoot?.activeElement?.id,
+ 'patchsetLevelComment'
+ );
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');
+ assert.equal(element?.shadowRoot?.activeElement?.tagName, 'GR-COMMENT');
+ assert.equal(
+ element?.shadowRoot?.activeElement?.id,
+ 'patchsetLevelComment'
+ );
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');
+ assert.equal(element?.shadowRoot?.activeElement?.tagName, 'GR-COMMENT');
+ assert.equal(
+ element?.shadowRoot?.activeElement?.id,
+ 'patchsetLevelComment'
+ );
element.focusOn(element.FocusTarget.REVIEWERS);
clock.tick(1);
@@ -2291,9 +2272,6 @@
suite('patchset level comment using GrComment', () => {
setup(async () => {
- stubFlags('isEnabled')
- .withArgs(KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT)
- .returns(true);
element.account = createAccountWithId(1);
element.requestUpdate();
await element.updateComplete;
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index e142b96..5d9800d 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -21,7 +21,6 @@
SUGGEST_EDIT = 'UiFeature__suggest_edit',
CHECKS_FIXES = 'UiFeature__checks_fixes',
MENTION_USERS = 'UiFeature__mention_users',
- PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT = 'UiFeature__patchset_level_comment_uses_GrComment',
RENDER_MARKDOWN = 'UiFeature__render_markdown',
AUTO_APP_THEME = 'UiFeature__auto_app_theme',
COPY_LINK_DIALOG = 'UiFeature__copy_link_dialog',
