Introduce gr-user-suggestion-fix
This change introduces suggested fix header with 2 buttons - copy to
clipboard and preview fix. Preview fix will not be action between
comments actions, but it is part of gr-user-suggestion-fix element.
It is much clearer what actions does when it is in header.
UI is not final and will be refined later.
Screenshot: https://imgur.com/a/jwDo14C
Release-Notes: skip
Google-Bug-Id: b/259630205
Change-Id: Ie01b56b2fab34217452c84e7db21a75e9bb1e6e9
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index c844d42..c36226a 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -275,6 +275,9 @@
this.save();
});
}
+ this.addEventListener('open-user-suggest-preview', e => {
+ this.handleShowFix(e.detail.code);
+ });
this.messagePlaceholder = 'Mention others with @';
subscribe(
this,
@@ -524,7 +527,6 @@
${this.renderCommentMessage()}
<gr-endpoint-slot name="above-actions"></gr-endpoint-slot>
${this.renderHumanActions()} ${this.renderRobotActions()}
- ${this.renderSuggestEditActions()}
</div>
</div>
</gr-endpoint-decorator>
@@ -776,32 +778,13 @@
return html`
<div class="rightActions">
${this.autoSaving ? html`. ` : ''}
- ${this.renderDiscardButton()} ${this.renderPreviewSuggestEditButton()}
- ${this.renderEditButton()} ${this.renderCancelButton()}
- ${this.renderSaveButton()} ${this.renderCopyLinkIcon()}
+ ${this.renderDiscardButton()} ${this.renderEditButton()}
+ ${this.renderCancelButton()} ${this.renderSaveButton()}
+ ${this.renderCopyLinkIcon()}
</div>
`;
}
- private renderPreviewSuggestEditButton() {
- if (!this.flagsService.isEnabled(KnownExperimentId.SUGGEST_EDIT)) {
- return nothing;
- }
- assertIsDefined(this.comment, 'comment');
- if (!hasUserSuggestion(this.comment)) return nothing;
- return html`
- <gr-button
- link
- secondary
- class="action show-fix"
- ?disabled=${this.saving}
- @click=${this.handleShowFix}
- >
- Preview Fix
- </gr-button>
- `;
- }
-
private renderSuggestEditButton() {
if (!this.flagsService.isEnabled(KnownExperimentId.SUGGEST_EDIT)) {
return nothing;
@@ -892,22 +875,6 @@
`;
}
- private renderSuggestEditActions() {
- if (!this.flagsService.isEnabled(KnownExperimentId.SUGGEST_EDIT)) {
- return nothing;
- }
- if (
- !this.account ||
- isRobot(this.comment) ||
- isDraftOrUnsaved(this.comment)
- ) {
- return nothing;
- }
- return html`
- <div class="robotActions">${this.renderPreviewSuggestEditButton()}</div>
- `;
- }
-
private renderShowFixButton() {
if (!(this.comment as RobotCommentInfo)?.fix_suggestions) return;
return html`
@@ -1037,12 +1004,14 @@
}
// private, but visible for testing
- async createFixPreview(): Promise<OpenFixPreviewEventDetail> {
+ async createFixPreview(
+ replacement?: string
+ ): Promise<OpenFixPreviewEventDetail> {
assertIsDefined(this.comment?.patch_set, 'comment.patch_set');
assertIsDefined(this.comment?.path, 'comment.path');
- if (hasUserSuggestion(this.comment)) {
- const replacement = getUserSuggestion(this.comment);
+ if (hasUserSuggestion(this.comment) || replacement) {
+ replacement = replacement ?? getUserSuggestion(this.comment);
assert(!!replacement, 'malformed user suggestion');
const line = await this.getCommentedCode();
@@ -1150,9 +1119,9 @@
fire(this, 'reply-to-comment', eventDetail);
}
- private async handleShowFix() {
+ private async handleShowFix(replacement?: string) {
// Handled top-level in the diff and change view components.
- fire(this, 'open-fix-preview', await this.createFixPreview());
+ fire(this, 'open-fix-preview', await this.createFixPreview(replacement));
}
async createSuggestEdit(e: MouseEvent) {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 3390369..6625844 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -29,19 +29,12 @@
import {
createComment,
createDraft,
- createFixSuggestionInfo,
createRobotComment,
createUnsaved,
} from '../../../test/test-data-generators';
-import {
- ReplyToCommentEvent,
- OpenFixPreviewEventDetail,
-} from '../../../types/events';
+import {ReplyToCommentEvent} from '../../../types/events';
import {GrConfirmDeleteCommentDialog} from '../gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog';
-import {
- DraftInfo,
- USER_SUGGESTION_START_PATTERN,
-} from '../../../utils/comment-util';
+import {DraftInfo} from '../../../utils/comment-util';
import {assertIsDefined} from '../../../utils/common-util';
import {Modifier} from '../../../utils/dom-util';
import {SinonStub} from 'sinon';
@@ -747,23 +740,6 @@
actions = query(element, '.robotActions gr-button.fix');
assert.isNotOk(actions);
});
-
- test('handleShowFix fires open-fix-preview event', async () => {
- const listener = listenOnce<CustomEvent<OpenFixPreviewEventDetail>>(
- element,
- 'open-fix-preview'
- );
- element.comment = {
- ...createRobotComment(),
- fix_suggestions: [{...createFixSuggestionInfo()}],
- };
- await element.updateComplete;
-
- queryAndAssert<GrButton>(element, '.show-fix').click();
-
- const e = await listener;
- assert.deepEqual(e.detail, await element.createFixPreview());
- });
});
suite('auto saving', () => {
@@ -869,33 +845,5 @@
</gr-button> `
);
});
-
- test('renders preview suggest fix', async () => {
- element.comment = {
- ...createComment(),
- author: {
- name: 'Mr. Peanutbutter',
- email: 'tenn1sballchaser@aol.com' as EmailAddress,
- },
- line: 5,
- path: 'test',
- message: `${USER_SUGGESTION_START_PATTERN}afterSuggestion${'\n```'}`,
- };
- await element.updateComplete;
-
- assert.dom.equal(
- queryAndAssert(element, 'gr-button.show-fix'),
- /* HTML */ `<gr-button
- aria-disabled="false"
- class="action show-fix"
- link=""
- role="button"
- secondary
- tabindex="0"
- >
- Preview Fix
- </gr-button> `
- );
- });
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.ts b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.ts
index 350aa7f..8c280c0 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.ts
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.ts
@@ -66,7 +66,10 @@
color: var(--primary-text-color);
}
gr-icon {
- color: var(--deemphasized-text-color);
+ color: var(
+ --gr-copy-clipboard-icon-color,
+ var(--deemphasized-text-color)
+ );
}
gr-button {
display: block;
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index 45eca40..023d8b5 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -18,6 +18,10 @@
import {CommentLinks, EmailAddress} from '../../../api/rest-api';
import {linkifyUrlsAndApplyRewrite} from '../../../utils/link-util';
import '../gr-account-chip/gr-account-chip';
+import '../gr-user-suggestion-fix/gr-user-suggestion-fix';
+import {KnownExperimentId} from '../../../services/flags/flags';
+import {getAppContext} from '../../../services/app-context';
+import {USER_SUGGESTION_INFO_STRING} from '../../../utils/comment-util';
/**
* This element optionally renders markdown and also applies some regex
@@ -34,6 +38,8 @@
@state()
private repoCommentLinks: CommentLinks = {};
+ private readonly flagsService = getAppContext().flagsService;
+
private readonly getConfigModel = resolve(this, configModelToken);
// Private const but used in tests.
@@ -134,6 +140,10 @@
}
private renderAsMarkdown() {
+ // need to find out here, since customRender is not arrow function
+ const suggestEditsEnable = this.flagsService.isEnabled(
+ KnownExperimentId.SUGGEST_EDIT
+ );
// <marked-element> internals will be in charge of calling our custom
// renderer so we wrap 'this.rewriteText' so that 'this' is preserved via
// closure.
@@ -167,7 +177,18 @@
``;
renderer['codespan'] = (text: string) =>
`<code>${unescapeHTML(text)}</code>`;
- renderer['code'] = (text: string) => `<pre><code>${text}</code></pre>`;
+ renderer['code'] = (text: string, infostring: string) => {
+ if (suggestEditsEnable && infostring === USER_SUGGESTION_INFO_STRING) {
+ // default santizer in markedjs is very restrictive, we need to use
+ // existing html element to mark element. We cannot use css class for it.
+ // Therefore we pick mark - as not frequently used html element to represent
+ // unconverted gr-user-suggestion-fix.
+ // TODO(milutin): Find a way to override sanitizer to directly use gr-user-suggestion-fix
+ return `<mark>${text}</mark>`;
+ } else {
+ return `<pre><code>${text}</code></pre>`;
+ }
+ };
renderer['text'] = boundRewriteText;
}
@@ -211,6 +232,9 @@
override updated() {
// Look for @mentions and replace them with an account-label chip.
this.convertEmailsToAccountChips();
+ if (this.flagsService.isEnabled(KnownExperimentId.SUGGEST_EDIT)) {
+ this.convertCodeToSuggestions();
+ }
}
private convertEmailsToAccountChips() {
@@ -235,6 +259,17 @@
}
}
}
+
+ private convertCodeToSuggestions() {
+ for (const userSuggestionMark of this.renderRoot.querySelectorAll('mark')) {
+ const userSuggestion = document.createElement('gr-user-suggestion-fix');
+ userSuggestion.textContent = userSuggestionMark.textContent ?? '';
+ userSuggestionMark.parentNode?.replaceChild(
+ userSuggestion,
+ userSuggestionMark
+ );
+ }
+ }
}
declare global {
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
index fcebeea..0e5117a 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
@@ -587,5 +587,25 @@
`
);
});
+
+ suite('user suggest fix', () => {
+ setup(async () => {
+ const flagsService = getAppContext().flagsService;
+ sinon.stub(flagsService, 'isEnabled').returns(true);
+ });
+
+ test('renders', async () => {
+ element.content = '```suggestion\nHello World```';
+ await element.updateComplete;
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `<marked-element>
+ <div class="markdown-html" slot="markdown-html">
+ <gr-user-suggestion-fix>Hello World</gr-user-suggestion-fix>
+ </div>
+ </marked-element>`
+ );
+ });
+ });
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
new file mode 100644
index 0000000..c557acc
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
@@ -0,0 +1,97 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {css, html, LitElement, nothing} from 'lit';
+import {customElement} from 'lit/decorators.js';
+import {getAppContext} from '../../../services/app-context';
+import {KnownExperimentId} from '../../../services/flags/flags';
+import {fire} from '../../../utils/event-util';
+
+declare global {
+ interface HTMLElementEventMap {
+ 'open-user-suggest-preview': OpenUserSuggestionPreviewEvent;
+ }
+}
+
+export type OpenUserSuggestionPreviewEvent =
+ CustomEvent<OpenUserSuggestionPreviewEventDetail>;
+export interface OpenUserSuggestionPreviewEventDetail {
+ code: string;
+}
+
+@customElement('gr-user-suggestion-fix')
+export class GrUserSuggetionFix extends LitElement {
+ private readonly flagsService = getAppContext().flagsService;
+
+ static override styles = [
+ css`
+ .header {
+ background-color: var(--user-suggestion-header-background);
+ color: var(--user-suggestion-header-color);
+ border: 1px solid var(--border-color);
+ border-bottom: 0;
+ padding: var(--spacing-xs) var(--spacing-s);
+ display: flex;
+ align-items: center;
+ }
+ .header .title {
+ flex: 1;
+ }
+ gr-copy-clipboard {
+ --gr-copy-clipboard-icon-color: var(--user-suggestion-header-color);
+ }
+ code {
+ max-width: var(--gr-formatted-text-prose-max-width, none);
+ background-color: var(--background-color-secondary);
+ border: 1px solid var(--border-color);
+ border-top: 0;
+ display: block;
+ font-family: var(--monospace-font-family);
+ font-size: var(--font-size-code);
+ line-height: var(--line-height-mono);
+ margin-bottom: var(--spacing-m);
+ padding: var(--spacing-xxs) var(--spacing-s);
+ overflow-x: auto;
+ /* Pre will preserve whitespace and line breaks but not wrap */
+ white-space: pre;
+ }
+ `,
+ ];
+
+ override render() {
+ if (!this.flagsService.isEnabled(KnownExperimentId.SUGGEST_EDIT)) {
+ return nothing;
+ }
+ if (!this.textContent) return nothing;
+ const code = this.textContent;
+ return html`<div class="header">
+ <div class="title">Suggested fix</div>
+ <div>
+ <gr-copy-clipboard hideInput="" text=${code}></gr-copy-clipboard>
+ </div>
+ <div>
+ <gr-button
+ secondary
+ class="action show-fix"
+ @click=${this.handleShowFix}
+ >
+ Preview Fix
+ </gr-button>
+ </div>
+ </div>
+ <code>${code}</code>`;
+ }
+
+ handleShowFix() {
+ if (!this.textContent) return;
+ fire(this, 'open-user-suggest-preview', {code: this.textContent});
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-user-suggestion-fix': GrUserSuggetionFix;
+ }
+}
diff --git a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts
new file mode 100644
index 0000000..80422a0
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix_test.ts
@@ -0,0 +1,46 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../../test/common-test-setup';
+import './gr-user-suggestion-fix';
+import {fixture, html, assert} from '@open-wc/testing';
+import {GrUserSuggetionFix} from './gr-user-suggestion-fix';
+import {getAppContext} from '../../../services/app-context';
+
+suite('gr-user-suggestion-fix tests', () => {
+ let element: GrUserSuggetionFix;
+
+ setup(async () => {
+ const flagsService = getAppContext().flagsService;
+ sinon.stub(flagsService, 'isEnabled').returns(true);
+ element = await fixture<GrUserSuggetionFix>(html`
+ <gr-user-suggestion-fix>Hello World</gr-user-suggestion-fix>
+ `);
+ await element.updateComplete;
+ });
+
+ test('render', async () => {
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `<div class="header">
+ <div class="title">Suggested fix</div>
+ <div>
+ <gr-copy-clipboard
+ hideinput=""
+ text="Hello World"
+ ></gr-copy-clipboard>
+ </div>
+ <div>
+ <gr-button class="action show-fix" secondary=""
+ >Preview Fix</gr-button
+ >
+ </div>
+ </div>
+ <code>Hello World</code>`
+ );
+ });
+});
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index 107ee16..0503e4c 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -278,6 +278,11 @@
--robot-comment-background-color: var(--blue-50);
--unresolved-comment-background-color: #fef7e0;
+
+ /* Suggest edits */
+ --user-suggestion-header-background: var(--gray-700);
+ --user-suggestion-header-color: white;
+
/* vote background colors */
--vote-color-approved: var(--green-300);
--vote-color-disliked: var(--red-50);
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts
index a183c86..dc3d4e9 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.ts
+++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -138,6 +138,10 @@
--robot-comment-background-color: #1e3a5f;
--unresolved-comment-background-color: #614a19;
+ /* Suggest edits */
+ --user-suggestion-header-background: var(--gray-700);
+ --user-suggestion-header-color: white;
+
/* vote background colors */
--vote-color-approved: var(--green-300);
--vote-color-disliked: var(--red-tonal);
diff --git a/polygerrit-ui/app/types/events.ts b/polygerrit-ui/app/types/events.ts
index e2612b0..477b884 100644
--- a/polygerrit-ui/app/types/events.ts
+++ b/polygerrit-ui/app/types/events.ts
@@ -163,6 +163,7 @@
userWantsToEdit: boolean;
unresolved: boolean;
}
+
export type ReplyToCommentEvent = CustomEvent<ReplyToCommentEventDetail>;
export interface PageErrorEventDetail {
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index a92f0f8..34a90de 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -523,7 +523,8 @@
};
}
-export const USER_SUGGESTION_START_PATTERN = '```suggestion\n';
+export const USER_SUGGESTION_INFO_STRING = 'suggestion';
+export const USER_SUGGESTION_START_PATTERN = `\`\`\`${USER_SUGGESTION_INFO_STRING}\n`;
// This can either mean a user or a checks provided fix.
// "Provided" means that the fix is sent along with the request
