Merge "Set cursor to the item when checkbox is selected."
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-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index 258ed9b..a4e026b 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -108,6 +108,9 @@
height: 1px;
min-width: 20px;
}
+ .repo {
+ margin-left: var(--spacing-m);
+ }
.repo,
.branch {
color: var(--primary-text-color);
@@ -291,8 +294,6 @@
) {
const truncatedRepo = truncatePath(change.project, 2);
return html`
- <span class="repo" .title=${change.project}>${truncatedRepo}</span
- ><span class="branch"> | ${change.branch} </span>
<gr-related-change
.label=${this.renderChangeTitle(change)}
.change=${change}
@@ -300,6 +301,8 @@
?show-submittable-check=${showSubmittabilityCheck}
>${change.subject}</gr-related-change
>
+ <span class="repo" .title=${change.project}>${truncatedRepo}</span
+ ><span class="branch"> | ${change.branch} </span>
`;
}
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
index 38bd6ec..09cdf5f 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
@@ -235,11 +235,11 @@
>
➔
</span>
- <span class="repo" title="test-project">test-project</span>
- <span class="branch"> | test-branch </span>
<gr-related-change show-submittable-check="">
Test subject
</gr-related-change>
+ <span class="repo" title="test-project">test-project</span>
+ <span class="branch"> | test-branch </span>
</div>
</gr-related-collapse>
<div class="note" hidden="">(+ )</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 2001d6d..acd1755 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
@@ -90,7 +90,10 @@
let lastId = 1;
const makeAccount = function () {
- return {_account_id: lastId++ as AccountId};
+ return {
+ _account_id: lastId++ as AccountId,
+ email: `${lastId}.com` as EmailAddress,
+ };
};
const makeGroup = function () {
return {id: `${lastId++}` as GroupId};
@@ -113,6 +116,7 @@
owner: {
_account_id: 999 as AccountId,
display_name: 'Kermit',
+ email: 'abcd' as EmailAddress,
},
labels: {
Verified: {
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.ts
index b1e74c1..16fa813 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.ts
@@ -56,6 +56,7 @@
_account_id: 1000001 as AccountId,
name: 'user',
username: 'user',
+ email: 'abcd' as EmailAddress,
},
patch_set: 4 as RevisionPatchSetNum,
id: 'ecf0b9fa_fe1a5f62' as UrlEncodedCommentId,
@@ -90,6 +91,7 @@
_account_id: 1000002 as AccountId,
name: 'user',
username: 'user',
+ email: 'abcd' as EmailAddress,
},
patch_set: 3 as RevisionPatchSetNum,
id: '09a9fb0a_1484e6cf' as UrlEncodedCommentId,
@@ -111,6 +113,7 @@
_account_id: 1000002 as AccountId,
name: 'user',
username: 'user',
+ email: 'abcd' as EmailAddress,
},
patch_set: 2 as RevisionPatchSetNum,
id: '8caddf38_44770ec1' as UrlEncodedCommentId,
@@ -132,6 +135,7 @@
_account_id: 1000003 as AccountId,
name: 'user',
username: 'user',
+ email: 'abcd' as EmailAddress,
},
patch_set: 2 as RevisionPatchSetNum,
id: 'scaddf38_44770ec1' as UrlEncodedCommentId,
@@ -206,6 +210,7 @@
_account_id: 1000000 as AccountId,
name: 'user',
username: 'user',
+ email: 'abcd' as EmailAddress,
},
patch_set: 4 as RevisionPatchSetNum,
id: 'rc1' as UrlEncodedCommentId,
@@ -230,6 +235,7 @@
_account_id: 1000000 as AccountId,
name: 'user',
username: 'user',
+ email: 'abcd' as EmailAddress,
},
patch_set: 4 as RevisionPatchSetNum,
id: 'rc2' as UrlEncodedCommentId,
@@ -245,6 +251,7 @@
_account_id: 1000000 as AccountId,
name: 'user',
username: 'user',
+ email: 'abcd' as EmailAddress,
},
patch_set: 4 as RevisionPatchSetNum,
id: 'c2_1' as UrlEncodedCommentId,
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 61bb809..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>
`);
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 11e7152..4fee574 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
@@ -7,8 +7,6 @@
import {CommentLinks} from '../../../types/common';
import {LitElement, css, html, TemplateResult} from 'lit';
import {customElement, property} from 'lit/decorators.js';
-import {getAppContext} from '../../../services/app-context';
-import {KnownExperimentId} from '../../../services/flags/flags';
const CODE_MARKER_PATTERN = /^(`{1,3})([^`]+?)\1$/;
const INLINE_PATTERN = /(\[.+?\]\(.+?\)|`[^`]+?`)/;
@@ -74,8 +72,6 @@
@property({type: Boolean, reflect: true})
noTrailingMargin = false;
- private readonly flagsService = getAppContext().flagsService;
-
static override get styles() {
return [
css`
@@ -140,15 +136,10 @@
override render() {
if (!this.content) return;
- if (this.flagsService.isEnabled(KnownExperimentId.RENDER_MARKDOWN)) {
- return html`<gr-markdown
- .markdown=${true}
- .content=${this.content}
- ></gr-markdown>`;
- } else {
- const blocks = this._computeBlocks(this.content);
- return html`${blocks.map(block => this.renderBlock(block))}`;
- }
+ return html`<gr-markdown
+ .markdown=${true}
+ .content=${this.content}
+ ></gr-markdown>`;
}
/**
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 62cb7c8..a3eff8e 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
@@ -4,9 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {fixture, html, assert} from '@open-wc/testing';
-import {KnownExperimentId} from '../../../services/flags/flags';
import '../../../test/common-test-setup';
-import {stubFlags} from '../../../test/test-utils';
import './gr-formatted-text';
import {
GrFormattedText,
@@ -24,7 +22,6 @@
suite('gr-formatted-text tests', () => {
let element: GrFormattedText;
- let markdownFlagStub: sinon.SinonStub<[experiment_id: string], boolean>;
function assertSpan(actual: InlineItem, expected: InlineItem) {
assert.equal(actual.type, expected.type);
@@ -73,24 +70,9 @@
}
setup(async () => {
- markdownFlagStub = stubFlags('isEnabled')
- .withArgs(KnownExperimentId.RENDER_MARKDOWN)
- .returns(false);
element = await fixture(html`<gr-formatted-text></gr-formatted-text>`);
});
- test('uses gr-markdown when flag is enabled', async () => {
- markdownFlagStub.returns(true);
- const elementUsingMarkdown = await fixture(
- html`<gr-formatted-text .content=${'# heading'}></gr-formatted-text>`
- );
-
- assert.shadowDom.equal(
- elementUsingMarkdown,
- /* HTML */ '<gr-markdown></gr-markdown>'
- );
- });
-
test('parse empty', () => {
assert.lengthOf(element._computeBlocks(''), 0);
});
@@ -99,15 +81,7 @@
element.content = 'text `code`';
await element.updateComplete;
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <p>
- <gr-markdown></gr-markdown>
- <span class="inline-code"> code </span>
- </p>
- `
- );
+ assert.shadowDom.equal(element, /* HTML */ ` <gr-markdown></gr-markdown> `);
});
for (const text of [
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/models/accounts-model/accounts-model.ts b/polygerrit-ui/app/models/accounts-model/accounts-model.ts
index 54aab24..3f35127 100644
--- a/polygerrit-ui/app/models/accounts-model/accounts-model.ts
+++ b/polygerrit-ui/app/models/accounts-model/accounts-model.ts
@@ -50,6 +50,8 @@
async fillDetails(account: AccountInfo) {
if (!isDetailedAccount(account)) {
if (account.email) return await this.getAccount({email: account.email});
+ else if (account._account_id)
+ return await this.getAccount({_account_id: account._account_id});
}
return account;
}
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index 3579bb3..9337500 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -184,6 +184,7 @@
export function createAccountWithId(id = 5): AccountInfo {
return {
_account_id: id as AccountId,
+ email: `${id}` as EmailAddress,
};
}
diff --git a/polygerrit-ui/app/utils/account-util.ts b/polygerrit-ui/app/utils/account-util.ts
index 6cc2e59..207152c 100644
--- a/polygerrit-ui/app/utils/account-util.ts
+++ b/polygerrit-ui/app/utils/account-util.ts
@@ -85,7 +85,11 @@
}
export function isDetailedAccount(account?: AccountInfo) {
- return account && account._account_id;
+ // In case ChangeInfo is requested without DetailedAccount option, the
+ // reviewer entry is returned as just {_account_id: 123}
+ // This object should also be treated as not detailed account if they have
+ // an AccountId and no email
+ return !!account?.email && !!account?._account_id;
}
/**
diff --git a/polygerrit-ui/app/utils/account-util_test.ts b/polygerrit-ui/app/utils/account-util_test.ts
index a307993..3e61255 100644
--- a/polygerrit-ui/app/utils/account-util_test.ts
+++ b/polygerrit-ui/app/utils/account-util_test.ts
@@ -9,6 +9,7 @@
extractMentionedUsers,
getAccountTemplate,
isAccountEmailOnly,
+ isDetailedAccount,
isServiceUser,
removeServiceUsers,
replaceTemplates,
@@ -255,6 +256,18 @@
);
});
+ test('isDetailedAccount', () => {
+ assert.isFalse(isDetailedAccount({_account_id: 12345 as AccountId}));
+ assert.isFalse(isDetailedAccount({email: 'abcd' as EmailAddress}));
+
+ assert.isTrue(
+ isDetailedAccount({
+ _account_id: 12345 as AccountId,
+ email: 'abcd' as EmailAddress,
+ })
+ );
+ });
+
test('fails gracefully when all is not included', async () => {
const change = {
...createChange(),
