Merge "Render change subject before branch | repo"
diff --git a/.gitignore b/.gitignore
index 623369b..53bc9f6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -35,6 +35,7 @@
/plugins/*
/polygerrit-ui/coverage/
/polygerrit-ui/app/plugins/*
+/polygerrit-ui/screenshots/Chrome/failed/
!/plugins/.eslintignore
!/plugins/.eslintrc.js
!/plugins/.prettierrc.js
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index e4bc8f0..7e06e4a 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1292,6 +1292,7 @@
"mute_common_path_prefixes": true,
"publish_comments_on_push": true,
"work_in_progress_by_default": true,
+ "allow_browser_notifications": true,
"default_base_for_merges": "FIRST_PARENT",
"my": [
{
@@ -1344,6 +1345,7 @@
"size_bar_in_change_table": true,
"disable_keyboard_shortcuts": true,
"disable_token_highlighting": true,
+ "allow_browser_notifications": false,
"diff_view": "SIDE_BY_SIDE",
"mute_common_path_prefixes": true,
"my": [
@@ -2686,6 +2688,8 @@
|`signed_off_by` |not set if `false`|
Whether to insert Signed-off-by footer in changes created with the
inline edit feature.
+|`allow_browser_notifications` |not set if `false`|
+Whether to prompt user to enable browser notification in browser.
|`my` ||
The menu items of the `MY` top menu as a list of
link:rest-api-config.html#top-menu-item-info[TopMenuItemInfo] entities.
@@ -2755,6 +2759,8 @@
|`signed_off_by` |optional|
Whether to insert Signed-off-by footer in changes created with the
inline edit feature.
+|`allow_browser_notifications` |not set if `false`|
+Whether to prompt user to enable browser notification in browser.
|`my` |optional|
The menu items of the `MY` top menu as a list of
link:rest-api-config.html#top-menu-item-info[TopMenuItemInfo] entities.
diff --git a/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java b/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
index 91d6294..1ee2cd8 100644
--- a/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
+++ b/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
@@ -150,6 +150,7 @@
public Boolean workInProgressByDefault;
public List<MenuItem> my;
public List<String> changeTable;
+ public Boolean allowBrowserNotifications;
public DateFormat getDateFormat() {
if (dateFormat == null) {
@@ -208,6 +209,7 @@
p.disableKeyboardShortcuts = false;
p.disableTokenHighlighting = false;
p.workInProgressByDefault = false;
+ p.allowBrowserNotifications = true;
return p;
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
index ff1aa89..f5b311b 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
@@ -84,6 +84,7 @@
i.legacycidInChangeTable ^= true;
i.muteCommonPathPrefixes ^= true;
i.signedOffBy ^= true;
+ i.allowBrowserNotifications ^= false;
i.diffView = DiffView.UNIFIED_DIFF;
i.my = new ArrayList<>();
i.my.add(new MenuItem("name", "url"));
@@ -95,6 +96,7 @@
assertThat(o.my).containsExactlyElementsIn(i.my);
assertThat(o.changeTable).containsExactlyElementsIn(i.changeTable);
assertThat(o.theme).isEqualTo(i.theme);
+ assertThat(o.allowBrowserNotifications).isEqualTo(i.allowBrowserNotifications);
assertThat(o.disableKeyboardShortcuts).isEqualTo(i.disableKeyboardShortcuts);
}
diff --git a/package.json b/package.json
index 1dfee7c..cdaf400 100644
--- a/package.json
+++ b/package.json
@@ -39,6 +39,8 @@
"start": "run-p -rl compile:watch start:server",
"start:server": "web-dev-server",
"test": "yarn --cwd=polygerrit-ui test",
+ "test:screenshot": "yarn --cwd=polygerrit-ui test:screenshot",
+ "test:screenshot-update": "yarn --cwd=polygerrit-ui test:screenshot-update",
"test:coverage": "yarn --cwd=polygerrit-ui test:coverage",
"test:watch": "yarn --cwd=polygerrit-ui test:watch",
"test:single": "yarn --cwd=polygerrit-ui test:single",
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
index 0ac9903..a58c7bb 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
@@ -31,6 +31,9 @@
import {resolve} from '../../../models/dependency';
import {subscribe} from '../../lit/subscription-controller';
import {createChangeUrl} from '../../../models/views/change';
+import {debounce, DelayedTask} from '../../../utils/async-util';
+
+const GET_CHANGES_DEBOUNCE_INTERVAL_MS = 10;
const LOOKUP_QUERY_PATTERNS: RegExp[] = [
/^\s*i?[0-9a-f]{7,40}\s*$/i, // CHANGE_ID
@@ -141,12 +144,17 @@
this.preferences = x;
if (this.changesPerPage !== x.changes_per_page) {
this.changesPerPage = x.changes_per_page;
- this.viewStateChanged();
+ this.debouncedGetChanges();
}
}
);
}
+ override disconnectedCallback() {
+ this.getChangesTask?.flush();
+ super.disconnectedCallback();
+ }
+
static override get styles() {
return [
sharedStyles,
@@ -275,12 +283,7 @@
}
reload() {
- if (this.loading) return;
- this.loading = true;
- this.getChanges().then(changes => {
- this.changes = changes || [];
- this.loading = false;
- });
+ if (!this.loading) this.debouncedGetChanges();
}
// private, but visible for testing
@@ -300,33 +303,44 @@
// in an async so that attachment to the DOM can take place first.
setTimeout(() => fireTitleChange(this, this.query));
- return this.getChanges().then(changes => {
- changes = changes || [];
- if (this.query && changes.length === 1) {
- for (const queryPattern of LOOKUP_QUERY_PATTERNS) {
- if (this.query.match(queryPattern)) {
- // "Back"/"Forward" buttons work correctly only with replaceUrl()
- this.getNavigation().replaceUrl(
- createChangeUrl({change: changes[0]})
- );
- return;
- }
- }
- }
- this.changes = changes;
- this.loading = false;
- });
+ this.debouncedGetChanges(true);
}
- // private but used in test
- getChanges() {
- return this.restApiService.getChanges(
- this.changesPerPage,
- this.query,
- this.offset
+ private getChangesTask?: DelayedTask;
+
+ private debouncedGetChanges(shouldSingleMatchRedirect = false) {
+ this.getChangesTask = debounce(
+ this.getChangesTask,
+ () => {
+ this.getChanges(shouldSingleMatchRedirect);
+ },
+ GET_CHANGES_DEBOUNCE_INTERVAL_MS
);
}
+ async getChanges(shouldSingleMatchRedirect = false) {
+ this.loading = true;
+ const changes =
+ (await this.restApiService.getChanges(
+ this.changesPerPage,
+ this.query,
+ this.offset
+ )) ?? [];
+ if (shouldSingleMatchRedirect && this.query && changes.length === 1) {
+ for (const queryPattern of LOOKUP_QUERY_PATTERNS) {
+ if (this.query.match(queryPattern)) {
+ // "Back"/"Forward" buttons work correctly only with replaceUrl()
+ this.getNavigation().replaceUrl(
+ createChangeUrl({change: changes[0]})
+ );
+ return;
+ }
+ }
+ }
+ this.changes = changes;
+ this.loading = false;
+ }
+
// private but used in test
limitFor(query: string, defaultLimit?: number) {
if (defaultLimit === undefined) return 0;
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts
index 860f8a3..b003b66 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts
@@ -24,16 +24,22 @@
import {fixture, html, waitUntil, assert} from '@open-wc/testing';
import {GerritView} from '../../../services/router/router-model';
import {testResolver} from '../../../test/common-test-setup';
-import {SinonStub} from 'sinon';
+import {SinonFakeTimers, SinonStub} from 'sinon';
+import {GrChangeList} from '../gr-change-list/gr-change-list';
+import {GrChangeListSection} from '../gr-change-list-section/gr-change-list-section';
+import {GrChangeListItem} from '../gr-change-list-item/gr-change-list-item';
const CHANGE_ID = 'IcA3dAB3edAB9f60B8dcdA6ef71A75980e4B7127';
const COMMIT_HASH = '12345678';
suite('gr-change-list-view tests', () => {
let element: GrChangeListView;
+ let changes: ChangeInfo[] | undefined = [];
+ let clock: SinonFakeTimers;
setup(async () => {
- stubRestApi('getChanges').returns(Promise.resolve([]));
+ clock = sinon.useFakeTimers();
+ stubRestApi('getChanges').callsFake(() => Promise.resolve(changes));
element = await fixture(html`<gr-change-list-view></gr-change-list-view>`);
element.viewState = {
view: GerritView.SEARCH,
@@ -68,32 +74,39 @@
});
suite('bulk actions', () => {
- let getChangesStub: sinon.SinonStub;
setup(async () => {
stubFlags('isEnabled').returns(true);
- getChangesStub = sinon.stub(element, 'getChanges');
- getChangesStub.returns(Promise.resolve([createChange()]));
+ changes = [createChange()];
element.loading = false;
element.reload();
- await waitUntil(() => element.loading === false);
- element.requestUpdate();
+ clock.tick(100);
await element.updateComplete;
+ await waitUntil(() => element.loading === false);
});
test('checkboxes remain checked after soft reload', async () => {
+ const changeListEl = queryAndAssert<GrChangeList>(
+ element,
+ 'gr-change-list'
+ );
+ await changeListEl.updateComplete;
+ const changeListSectionEl = queryAndAssert<GrChangeListSection>(
+ changeListEl,
+ 'gr-change-list-section'
+ );
+ await changeListSectionEl.updateComplete;
+ const changeListItemEl = queryAndAssert<GrChangeListItem>(
+ changeListSectionEl,
+ 'gr-change-list-item'
+ );
+ await changeListItemEl.updateComplete;
let checkbox = queryAndAssert<HTMLInputElement>(
- query(
- query(query(element, 'gr-change-list'), 'gr-change-list-section'),
- 'gr-change-list-item'
- ),
+ changeListItemEl,
'.selection > label > input'
);
checkbox.click();
await waitUntil(() => checkbox.checked);
- getChangesStub.restore();
- getChangesStub.returns(Promise.resolve([[createChange()]]));
-
element.reload();
await element.updateComplete;
checkbox = queryAndAssert<HTMLInputElement>(
@@ -288,9 +301,10 @@
test('Searching for a change ID redirects to change', async () => {
const change = {...createChange(), _number: 1 as NumericChangeId};
- sinon.stub(element, 'getChanges').returns(Promise.resolve([change]));
+ changes = [change];
element.viewState = {view: GerritView.SEARCH, query: CHANGE_ID};
+ clock.tick(100);
await element.updateComplete;
assert.isTrue(replaceUrlStub.called);
@@ -299,9 +313,10 @@
test('Searching for a change num redirects to change', async () => {
const change = {...createChange(), _number: 1 as NumericChangeId};
- sinon.stub(element, 'getChanges').returns(Promise.resolve([change]));
+ changes = [change];
element.viewState = {view: GerritView.SEARCH, query: '1'};
+ clock.tick(100);
await element.updateComplete;
assert.isTrue(replaceUrlStub.called);
@@ -310,9 +325,10 @@
test('Commit hash redirects to change', async () => {
const change = {...createChange(), _number: 1 as NumericChangeId};
- sinon.stub(element, 'getChanges').returns(Promise.resolve([change]));
+ changes = [change];
element.viewState = {view: GerritView.SEARCH, query: COMMIT_HASH};
+ clock.tick(100);
await element.updateComplete;
assert.isTrue(replaceUrlStub.called);
@@ -320,18 +336,20 @@
});
test('Searching for an invalid change ID searches', async () => {
- sinon.stub(element, 'getChanges').returns(Promise.resolve([]));
+ changes = [];
element.viewState = {view: GerritView.SEARCH, query: CHANGE_ID};
+ clock.tick(100);
await element.updateComplete;
assert.isFalse(replaceUrlStub.called);
});
test('Change ID with multiple search results searches', async () => {
- sinon.stub(element, 'getChanges').returns(Promise.resolve(undefined));
+ changes = undefined;
element.viewState = {view: GerritView.SEARCH, query: CHANGE_ID};
+ clock.tick(100);
await element.updateComplete;
assert.isFalse(replaceUrlStub.called);
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_screenshot_test.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_screenshot_test.ts
new file mode 100644
index 0000000..f80f48b
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_screenshot_test.ts
@@ -0,0 +1,58 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../../test/common-test-setup';
+import '../../shared/gr-date-formatter/gr-date-formatter';
+import {fixture, html} from '@open-wc/testing';
+import {visualDiff} from '@web/test-runner-visual-regression';
+import {FileInfo, PARENT, RevisionPatchSetNum} from '../../../api/rest-api';
+import {normalize} from '../../../models/change/files-model';
+import {PatchRange} from '../../../types/common';
+import {DiffPreferencesInfo} from '../../../api/diff';
+import {NormalizedFileInfo, GrFileList} from './gr-file-list';
+import './gr-file-list';
+
+suite('gr-file-list screenshot tests', () => {
+ let element: GrFileList;
+
+ function createFiles(
+ count: number,
+ fileInfo: FileInfo
+ ): NormalizedFileInfo[] {
+ return Array.from(Array(count).keys()).map(index =>
+ normalize(fileInfo, `/file${index}`)
+ );
+ }
+
+ setup(async () => {
+ const patchRange: PatchRange = {
+ basePatchNum: PARENT,
+ patchNum: 2 as RevisionPatchSetNum,
+ };
+ const diffPrefs: DiffPreferencesInfo = {
+ context: 10,
+ tab_size: 8,
+ font_size: 12,
+ line_length: 100,
+ ignore_whitespace: 'IGNORE_NONE',
+ };
+ element = await fixture(
+ html`<gr-file-list
+ .patchRange=${patchRange}
+ .diffPrefs=${diffPrefs}
+ ></gr-file-list>`
+ );
+ });
+
+ test('screenshot', async () => {
+ element.files = [
+ ...createFiles(3, {lines_inserted: 9}),
+ ...createFiles(2, {lines_deleted: 14}),
+ ];
+ await element.updateComplete;
+
+ await visualDiff(element, 'gr-file-list');
+ });
+});
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/elements/checks/gr-hovercard-run_test.ts b/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
index 95ae3dd..4ae2a46 100644
--- a/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
+++ b/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
@@ -15,6 +15,8 @@
let element: GrHovercardRun;
setup(async () => {
+ const fakeNow = new Date('Sep 26 2022 12:00:00');
+ sinon.useFakeTimers(fakeNow);
element = await fixture<GrHovercardRun>(html`
<gr-hovercard-run class="hovered"></gr-hovercard-run>
`);
@@ -145,11 +147,11 @@
<div class="sectionContent">
<div class="row">
<div class="title">Started</div>
- <div>1 year 5 m ago</div>
+ <div>1 year 6 m ago</div>
</div>
<div class="row">
<div class="title">Ended</div>
- <div>1 year 5 m ago</div>
+ <div>1 year 6 m ago</div>
</div>
<div class="row">
<div class="title">Completion</div>
diff --git a/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown.ts b/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown.ts
index 50f0602..c315603 100644
--- a/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown.ts
+++ b/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown.ts
@@ -80,9 +80,9 @@
/* Pre will preserve whitespace and line breaks but not wrap */
white-space: pre;
}
- /* Code within a sentence needs display:inline to shrink and not take a
- whole row */
- p code {
+ /* Non-multiline code elements need display:inline to shrink and not take
+ a whole row */
+ :not(pre) > code {
display: inline;
}
p {
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index e142b96..0bf1522 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -16,12 +16,12 @@
export enum KnownExperimentId {
NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui',
CHECKS_DEVELOPER = 'UiFeature__checks_developer',
+ PUSH_NOTIFICATIONS_DEVELOPER = 'UiFeature__push_notifications_developer',
DIFF_RENDERING_LIT = 'UiFeature__diff_rendering_lit',
PUSH_NOTIFICATIONS = 'UiFeature__push_notifications',
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',
diff --git a/polygerrit-ui/app/services/service-worker-installer.ts b/polygerrit-ui/app/services/service-worker-installer.ts
index 3af79a3..0eec23a 100644
--- a/polygerrit-ui/app/services/service-worker-installer.ts
+++ b/polygerrit-ui/app/services/service-worker-installer.ts
@@ -5,9 +5,13 @@
*/
import {FlagsService, KnownExperimentId} from './flags/flags';
-import {registerServiceWorker} from '../utils/worker-util';
+import {
+ areNotificationsEnabled,
+ registerServiceWorker,
+} from '../utils/worker-util';
import {UserModel} from '../models/user/user-model';
import {AccountDetailInfo} from '../api/rest-api';
+import {until} from '../utils/async-util';
/** Type of incoming messages for ServiceWorker. */
export enum ServiceWorkerMessageType {
@@ -30,8 +34,23 @@
async init() {
if (this.initialized) return;
- if (!this.flagsService.isEnabled(KnownExperimentId.PUSH_NOTIFICATIONS)) {
- return;
+ if (
+ !this.flagsService.isEnabled(
+ KnownExperimentId.PUSH_NOTIFICATIONS_DEVELOPER
+ )
+ ) {
+ if (!this.flagsService.isEnabled(KnownExperimentId.PUSH_NOTIFICATIONS)) {
+ return;
+ }
+ const timeout1s = new Promise(resolve => {
+ setTimeout(resolve, 1000);
+ });
+ // We wait for account to be defined, if its not defined in 1s, it's guest
+ await Promise.race([
+ timeout1s,
+ until(this.userModel.account$, account => !!account),
+ ]);
+ if (!areNotificationsEnabled(this.account)) return;
}
if (!('serviceWorker' in navigator)) {
console.error('Service worker API not available');
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index 6f6fe73..306747b 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -38,6 +38,7 @@
Provider,
} from '../models/dependency';
import * as sinon from 'sinon';
+import '../styles/themes/app-theme.ts';
declare global {
interface Window {
@@ -52,8 +53,7 @@
const log = _testOnly_defaultResinReportHandler;
log(isViolation, fmt, ...args);
if (isViolation) {
- // This will cause the test to fail if there is a data binding
- // violation.
+ // This will cause the test to fail if there is a data binding violation.
throw new Error('polymer-resin violation: ' + fmt + JSON.stringify(args));
}
});
@@ -132,7 +132,7 @@
// Very simple function to catch unexpected elements in documents body.
// It can't catch everything, but in most cases it is enough.
function checkChildAllowed(element: Element) {
- const allowedTags = ['SCRIPT', 'IRON-A11Y-ANNOUNCER'];
+ const allowedTags = ['SCRIPT', 'IRON-A11Y-ANNOUNCER', 'LINK'];
if (allowedTags.includes(element.tagName)) {
return;
}
diff --git a/polygerrit-ui/app/utils/worker-util.ts b/polygerrit-ui/app/utils/worker-util.ts
index c735a44..aeee537 100644
--- a/polygerrit-ui/app/utils/worker-util.ts
+++ b/polygerrit-ui/app/utils/worker-util.ts
@@ -4,6 +4,11 @@
* SPDX-License-Identifier: Apache-2.0
*/
+// This file adds some simple checks to match internal Google rules.
+// Internally at Google it has different a implementation.
+
+import {AccountDetailInfo} from '../api/rest-api';
+
/**
* We cannot import the worker script from cdn directly, because that is
* creating cross-origin issues. Instead we have to create a worker script on
@@ -25,6 +30,10 @@
return window.navigator.serviceWorker.register(workerUrl);
}
+export function areNotificationsEnabled(account?: AccountDetailInfo): boolean {
+ return !!account?._account_id;
+}
+
export function importScript(scope: WorkerGlobalScope, url: string): void {
scope.importScripts(url);
}
diff --git a/polygerrit-ui/package.json b/polygerrit-ui/package.json
index b8fef7d..1287d0c 100644
--- a/polygerrit-ui/package.json
+++ b/polygerrit-ui/package.json
@@ -11,6 +11,7 @@
"@open-wc/testing": "^3.1.6",
"@web/dev-server-esbuild": "^0.3.2",
"@web/test-runner": "^0.14.0",
+ "@web/test-runner-visual-regression": "^0.6.6",
"accessibility-developer-tools": "^2.12.0",
"karma": "^6.3.20",
"karma-chrome-launcher": "^3.1.1",
@@ -22,6 +23,8 @@
},
"scripts": {
"test": "web-test-runner",
+ "test:screenshot": "web-test-runner --run-screenshots",
+ "test:screenshot-update": "web-test-runner --update-screenshots --files",
"test:coverage": "web-test-runner --coverage",
"test:watch": "web-test-runner --watch",
"test:single": "web-test-runner --watch --files",
diff --git a/polygerrit-ui/screenshots/Chrome/baseline/gr-file-list.png b/polygerrit-ui/screenshots/Chrome/baseline/gr-file-list.png
new file mode 100644
index 0000000..4d0cbed
--- /dev/null
+++ b/polygerrit-ui/screenshots/Chrome/baseline/gr-file-list.png
Binary files differ
diff --git a/polygerrit-ui/web-test-runner.config.mjs b/polygerrit-ui/web-test-runner.config.mjs
index 44b5969..552e609 100644
--- a/polygerrit-ui/web-test-runner.config.mjs
+++ b/polygerrit-ui/web-test-runner.config.mjs
@@ -1,24 +1,61 @@
import { esbuildPlugin } from "@web/dev-server-esbuild";
import { defaultReporter, summaryReporter } from "@web/test-runner";
+import { visualRegressionPlugin } from "@web/test-runner-visual-regression/plugin";
/** @type {import('@web/test-runner').TestRunnerConfig} */
const config = {
- files: ["app/**/*_test.{ts,js}", "!**/node_modules/**/*"],
+ files: [
+ "app/**/*_test.{ts,js}",
+ "!**/node_modules/**/*",
+ ...(process.argv.includes("--run-screenshots")
+ ? []
+ : ["!app/**/*_screenshot_test.{ts,js}"]),
+ ],
port: 9876,
nodeResolve: true,
- testFramework: {
- config: {
- ui: "tdd",
- timeout: 5000,
- },
- },
+ testFramework: { config: { ui: "tdd", timeout: 5000 } },
plugins: [
esbuildPlugin({
ts: true,
target: "es2020",
tsconfig: "app/tsconfig.json",
}),
+ visualRegressionPlugin({
+ diffOptions: {
+ threshold: 0.8,
+ },
+ update: process.argv.includes("--update-screenshots"),
+ }),
],
+ // serve from gerrit root directory so that we can serve fonts from
+ // /lib/fonts/, see middleware.
+ rootDir: "..",
reporters: [defaultReporter(), summaryReporter()],
+ middleware: [
+ // Fonts are in /lib/fonts/, but css tries to load from
+ // /polygerrit-ui/app/fonts/. In production this works because our build
+ // copies them over, see /polygerrit-ui/BUILD
+ async (context, next) => {
+ if (context.url.startsWith("/polygerrit-ui/app/fonts/")) {
+ context.url = context.url.replace("/polygerrit-ui/app/", "/lib/");
+ }
+ await next();
+ },
+ ],
+ testRunnerHtml: (testFramework) => `
+ <!DOCTYPE html>
+ <html>
+ <head>
+ <link rel="stylesheet" href="polygerrit-ui/app/styles/main.css">
+ <link rel="stylesheet" href="polygerrit-ui/app/styles/fonts.css">
+ <link
+ rel="stylesheet"
+ href="polygerrit-ui/app/styles/material-icons.css">
+ </head>
+ <body>
+ <script type="module" src="${testFramework}"></script>
+ </body>
+ </html>
+ `,
};
export default config;
diff --git a/polygerrit-ui/yarn.lock b/polygerrit-ui/yarn.lock
index 68b6fff..ca6943d 100644
--- a/polygerrit-ui/yarn.lock
+++ b/polygerrit-ui/yarn.lock
@@ -1526,6 +1526,13 @@
resolved "https://registry.yarnpkg.com/@types/minimatch/-/minimatch-3.0.5.tgz#1001cc5e6a3704b83c236027e77f2f58ea010f40"
integrity sha512-Klz949h02Gz2uZCMGwDUSDS1YBlTdDDgbWHi+81l29tQALUtvz4rAYi5uoVhE5Lagoq6DeqAUlbrHvW/mXDgdQ==
+"@types/mkdirp@^1.0.1":
+ version "1.0.2"
+ resolved "https://registry.yarnpkg.com/@types/mkdirp/-/mkdirp-1.0.2.tgz#8d0bad7aa793abe551860be1f7ae7f3198c16666"
+ integrity sha512-o0K1tSO0Dx5X6xlU5F1D6625FawhC3dU3iqr25lluNv/+/QIVH8RLNEiVokgIZo+mz+87w/3Mkg/VvQS+J51fQ==
+ dependencies:
+ "@types/node" "*"
+
"@types/mocha@^8.2.0":
version "8.2.3"
resolved "https://registry.yarnpkg.com/@types/mocha/-/mocha-8.2.3.tgz#bbeb55fbc73f28ea6de601fbfa4613f58d785323"
@@ -1551,6 +1558,20 @@
resolved "https://registry.yarnpkg.com/@types/path-is-inside/-/path-is-inside-1.0.0.tgz#02d6ff38975d684bdec96204494baf9f29f0e17f"
integrity sha512-hfnXRGugz+McgX2jxyy5qz9sB21LRzlGn24zlwN2KEgoPtEvjzNRrLtUkOOebPDPZl3Rq7ywKxYvylVcEZDnEw==
+"@types/pixelmatch@^5.2.2":
+ version "5.2.4"
+ resolved "https://registry.yarnpkg.com/@types/pixelmatch/-/pixelmatch-5.2.4.tgz#ca145cc5ede1388c71c68edf2d1f5190e5ddd0f6"
+ integrity sha512-HDaSHIAv9kwpMN7zlmwfTv6gax0PiporJOipcrGsVNF3Ba+kryOZc0Pio5pn6NhisgWr7TaajlPEKTbTAypIBQ==
+ dependencies:
+ "@types/node" "*"
+
+"@types/pngjs@^6.0.0":
+ version "6.0.1"
+ resolved "https://registry.yarnpkg.com/@types/pngjs/-/pngjs-6.0.1.tgz#c711ec3fbbf077fed274ecccaf85dd4673130072"
+ integrity sha512-J39njbdW1U/6YyVXvC9+1iflZghP8jgRf2ndYghdJb5xL49LYDB+1EuAxfbuJ2IBbWIL3AjHPQhgaTxT3YaYeg==
+ dependencies:
+ "@types/node" "*"
+
"@types/qs@*":
version "6.9.7"
resolved "https://registry.yarnpkg.com/@types/qs/-/qs-6.9.7.tgz#63bb7d067db107cc1e457c303bc25d511febf6cb"
@@ -1780,6 +1801,14 @@
"@web/test-runner-core" "^0.10.27"
mkdirp "^1.0.4"
+"@web/test-runner-commands@^0.6.4":
+ version "0.6.5"
+ resolved "https://registry.yarnpkg.com/@web/test-runner-commands/-/test-runner-commands-0.6.5.tgz#69a2a06b52fd9d329f9cf1e172cd8fb1d5ffc521"
+ integrity sha512-W+wLg10jEAJY9N6tNWqG1daKmAzxGmTbO/H9fFfcgOgdxdn+hHiR4r2/x1iylKbFLujHUQlnjNQeu2d6eDPFqg==
+ dependencies:
+ "@web/test-runner-core" "^0.10.27"
+ mkdirp "^1.0.4"
+
"@web/test-runner-core@^0.10.20":
version "0.10.22"
resolved "https://registry.yarnpkg.com/@web/test-runner-core/-/test-runner-core-0.10.22.tgz#34bb67d12a79b01dc79c816f3d76f3419ef50eaf"
@@ -1862,6 +1891,20 @@
"@types/mocha" "^8.2.0"
"@web/test-runner-core" "^0.10.20"
+"@web/test-runner-visual-regression@^0.6.6":
+ version "0.6.6"
+ resolved "https://registry.yarnpkg.com/@web/test-runner-visual-regression/-/test-runner-visual-regression-0.6.6.tgz#4a4dc734f360cba66a005e07b4a1c0a9ef956444"
+ integrity sha512-010J3zE6z2v7eLLey/l5cYa9/WhPsgzZb3Z6K5nn4Mn5W5LGPs/f+XG3N6+Tx8Q1/RvDqLdFvRs/T6c4ul4dgQ==
+ dependencies:
+ "@types/mkdirp" "^1.0.1"
+ "@types/pixelmatch" "^5.2.2"
+ "@types/pngjs" "^6.0.0"
+ "@web/test-runner-commands" "^0.6.4"
+ "@web/test-runner-core" "^0.10.20"
+ mkdirp "^1.0.4"
+ pixelmatch "^5.2.1"
+ pngjs "^6.0.0"
+
"@web/test-runner@^0.14.0":
version "0.14.0"
resolved "https://registry.yarnpkg.com/@web/test-runner/-/test-runner-0.14.0.tgz#fc7b206f3fdc5a1d774cfc8f60159a574d30b185"
@@ -4569,6 +4612,13 @@
resolved "https://registry.yarnpkg.com/pify/-/pify-3.0.0.tgz#e5a4acd2c101fdf3d9a4d07f0dbc4db49dd28176"
integrity sha512-C3FsVNH1udSEX48gGX1xfvwTWfsYWj5U+8/uK15BGzIGrKoUpghX8hWZwa/OFnakBiiVNmBvemTJR5mcy7iPcg==
+pixelmatch@^5.2.1:
+ version "5.3.0"
+ resolved "https://registry.yarnpkg.com/pixelmatch/-/pixelmatch-5.3.0.tgz#5e5321a7abedfb7962d60dbf345deda87cb9560a"
+ integrity sha512-o8mkY4E/+LNUf6LzX96ht6k6CEDi65k9G2rjMtBe9Oo+VPKSvl+0GKHuH/AlG+GA5LPG/i5hrekkxUc3s2HU+Q==
+ dependencies:
+ pngjs "^6.0.0"
+
pkg-dir@4.2.0:
version "4.2.0"
resolved "https://registry.yarnpkg.com/pkg-dir/-/pkg-dir-4.2.0.tgz#f099133df7ede422e81d1d8448270eeb3e4261f3"
@@ -4576,6 +4626,11 @@
dependencies:
find-up "^4.0.0"
+pngjs@^6.0.0:
+ version "6.0.0"
+ resolved "https://registry.yarnpkg.com/pngjs/-/pngjs-6.0.0.tgz#ca9e5d2aa48db0228a52c419c3308e87720da821"
+ integrity sha512-TRzzuFRRmEoSW/p1KVAmiOgPco2Irlah+bGFCeNfJXxxYGwSw7YwAOAcd7X28K/m5bjBWKsC29KyoMfHbypayg==
+
polyfills-loader@^1.7.4:
version "1.7.6"
resolved "https://registry.yarnpkg.com/polyfills-loader/-/polyfills-loader-1.7.6.tgz#5cff98bfc9689cf10e44bdd32f498cfeb4374c51"
