Merge changes I59c7dfba,I37f06df0,I4e379c1d,I927e987d,Icde05fa0, ...
* changes:
Fork gr-diff
Remove unused `addDraftAtLine()`
Remove file imports of gr-diff-cursor
Break up gr-diff-utils into two files
Remove unused `filterThreadElsForLocation()`
Stop re-exporting basic diff types from gr-diff-line.ts
Remove dependency on GrDiffBuilderImage from gr-diff-host_test
Move `gr-diff-mode-selector` out of embed/diff
diff --git a/plugins/delete-project b/plugins/delete-project
index 4b17366..0322a37 160000
--- a/plugins/delete-project
+++ b/plugins/delete-project
@@ -1 +1 @@
-Subproject commit 4b173665d13703332a941d9c347c3adddd45fb54
+Subproject commit 0322a37009071da525bfd8569e98538c2e8891d5
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 0f3810a..d778546 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -47,6 +47,7 @@
otherPrimaryLinks,
secondaryLinks,
tooltipForLink,
+ computeIsExpandable,
} from '../../models/checks/checks-util';
import {assertIsDefined, assert, unique} from '../../utils/common-util';
import {modifierPressed, toggleClass, whenVisible} from '../../utils/dom-util';
@@ -322,20 +323,12 @@
];
}
- override updated(changedProperties: PropertyValues) {
+ override willUpdate(changedProperties: PropertyValues) {
if (changedProperties.has('result')) {
- this.isExpandable = this.computeIsExpandable();
+ this.isExpandable = computeIsExpandable(this.result);
}
}
- private computeIsExpandable() {
- const hasSummary = !!this.result?.summary;
- const hasMessage = !!this.result?.message;
- const hasMultipleLinks = (this.result?.links ?? []).length > 1;
- const hasPointers = (this.result?.codePointers ?? []).length > 0;
- return hasSummary && (hasMessage || hasMultipleLinks || hasPointers);
- }
-
override focus() {
if (this.nameEl) this.nameEl.focus();
}
diff --git a/polygerrit-ui/app/elements/checks/gr-diff-check-result.ts b/polygerrit-ui/app/elements/checks/gr-diff-check-result.ts
index efc6efe..1999e1f 100644
--- a/polygerrit-ui/app/elements/checks/gr-diff-check-result.ts
+++ b/polygerrit-ui/app/elements/checks/gr-diff-check-result.ts
@@ -9,6 +9,7 @@
import {customElement, property, state} from 'lit/decorators.js';
import {RunResult} from '../../models/checks/checks-model';
import {
+ computeIsExpandable,
createFixAction,
createPleaseFixComment,
iconFor,
@@ -244,9 +245,9 @@
`;
}
- override updated(changedProperties: PropertyValues) {
+ override willUpdate(changedProperties: PropertyValues) {
if (changedProperties.has('result')) {
- this.isExpandable = !!this.result?.summary && !!this.result?.message;
+ this.isExpandable = computeIsExpandable(this.result);
}
}
diff --git a/polygerrit-ui/app/elements/checks/gr-diff-check-result_test.ts b/polygerrit-ui/app/elements/checks/gr-diff-check-result_test.ts
index 0377e0e..51ee41d 100644
--- a/polygerrit-ui/app/elements/checks/gr-diff-check-result_test.ts
+++ b/polygerrit-ui/app/elements/checks/gr-diff-check-result_test.ts
@@ -45,6 +45,13 @@
There is a lot to be said. A lot. I say, a lot.
So please keep reading.
</div>
+ <div aria-checked="false"
+ aria-label="Expand result row"
+ class="show-hide"
+ role="switch"
+ tabindex="0">
+ <gr-icon icon="expand_more"></gr-icon>
+ </div>
</div>
<div class="details"></div>
</div>
diff --git a/polygerrit-ui/app/models/checks/checks-util.ts b/polygerrit-ui/app/models/checks/checks-util.ts
index 026e5e5..86c4b49 100644
--- a/polygerrit-ui/app/models/checks/checks-util.ts
+++ b/polygerrit-ui/app/models/checks/checks-util.ts
@@ -517,3 +517,12 @@
export function secondaryLinks(result?: CheckResultApi): Link[] {
return (result?.links ?? []).filter(link => !link.primary);
}
+
+export function computeIsExpandable(result?: CheckResultApi) {
+ if (!result?.summary) return false;
+ const hasMessage = !!result?.message;
+ const hasMultipleLinks = (result?.links ?? []).length > 1;
+ const hasPointers = (result?.codePointers ?? []).length > 0;
+ const hasFixes = (result?.fixes ?? []).length > 0;
+ return hasMessage || hasMultipleLinks || hasPointers || hasFixes;
+}
diff --git a/polygerrit-ui/app/models/checks/checks-util_test.ts b/polygerrit-ui/app/models/checks/checks-util_test.ts
index 822435d..83b4cd6 100644
--- a/polygerrit-ui/app/models/checks/checks-util_test.ts
+++ b/polygerrit-ui/app/models/checks/checks-util_test.ts
@@ -10,6 +10,7 @@
ALL_ATTEMPTS,
AttemptChoice,
LATEST_ATTEMPT,
+ computeIsExpandable,
rectifyFix,
sortAttemptChoices,
stringToAttemptChoice,
@@ -17,6 +18,12 @@
import {Fix, Replacement} from '../../api/checks';
import {PROVIDED_FIX_ID} from '../../utils/comment-util';
import {CommentRange} from '../../api/rest-api';
+import {
+ createCheckFix,
+ createCheckLink,
+ createCheckResult,
+ createRange,
+} from '../../test/test-data-generators';
suite('checks-util tests', () => {
setup(() => {});
@@ -107,4 +114,62 @@
];
assert.deepEqual(unsorted.sort(sortAttemptChoices), sortedExpected);
});
+
+ suite('computeIsExpandable', () => {
+ test('no message', () => {
+ assert.isFalse(computeIsExpandable(createCheckResult()));
+ });
+
+ test('no summary', () => {
+ assert.isFalse(
+ computeIsExpandable({
+ ...createCheckResult(),
+ message: 'asdf',
+ summary: undefined as unknown as string,
+ })
+ );
+ });
+
+ test('has message', () => {
+ assert.isTrue(
+ computeIsExpandable({...createCheckResult(), message: 'asdf'})
+ );
+ });
+
+ test('has just one link', () => {
+ assert.isFalse(
+ computeIsExpandable({
+ ...createCheckResult(),
+ links: [createCheckLink()],
+ })
+ );
+ });
+
+ test('has more than one link', () => {
+ assert.isTrue(
+ computeIsExpandable({
+ ...createCheckResult(),
+ links: [createCheckLink(), createCheckLink()],
+ })
+ );
+ });
+
+ test('has code pointer', () => {
+ assert.isTrue(
+ computeIsExpandable({
+ ...createCheckResult(),
+ codePointers: [{path: 'asdf', range: createRange()}],
+ })
+ );
+ });
+
+ test('has fix', () => {
+ assert.isTrue(
+ computeIsExpandable({
+ ...createCheckResult(),
+ fixes: [createCheckFix()],
+ })
+ );
+ });
+ });
});
diff --git a/polygerrit-ui/app/models/comments/comments-model.ts b/polygerrit-ui/app/models/comments/comments-model.ts
index eca8b7c..c9b0bd9 100644
--- a/polygerrit-ui/app/models/comments/comments-model.ts
+++ b/polygerrit-ui/app/models/comments/comments-model.ts
@@ -438,6 +438,7 @@
private readonly navigation: NavigationService
) {
super(initialState);
+ console.info('CommentsModel constrcutor');
this.subscriptions.push(
this.savingInProgress$.subscribe(savingInProgress => {
if (savingInProgress) {
@@ -477,6 +478,7 @@
);
this.subscriptions.push(
this.changeViewModel.changeNum$.subscribe(changeNum => {
+ console.info(`CommentsModel reload ${changeNum}`);
this.changeNum = changeNum;
this.setState({...initialState});
this.reloadAllComments();
@@ -518,8 +520,18 @@
this.setState(reducer({...this.getState()}));
}
+ override setState(state: CommentState) {
+ const commentsUndefPrev = this.getState().comments === undefined;
+ const commentsUndefNext = state.comments === undefined;
+ console.info(
+ `CommentsModel setState ${commentsUndefPrev} ${commentsUndefNext} ${this.stateUpdateInProgress}`
+ );
+ super.setState(state);
+ }
+
async reloadComments(changeNum: NumericChangeId): Promise<void> {
const comments = await this.restApiService.getDiffComments(changeNum);
+ console.info(`CommentsModel setComments ${comments === undefined}`);
this.modifyState(s => setComments(s, comments));
}
diff --git a/polygerrit-ui/app/models/model.ts b/polygerrit-ui/app/models/model.ts
index 19b52fc..2d0ff42 100644
--- a/polygerrit-ui/app/models/model.ts
+++ b/polygerrit-ui/app/models/model.ts
@@ -27,7 +27,7 @@
* another `next()` call. So make sure that state updates complete before
* starting another one.
*/
- private stateUpdateInProgress = false;
+ protected stateUpdateInProgress = false;
private subject$: BehaviorSubject<T>;
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index dce8e4b..4a0f6b8 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -104,7 +104,7 @@
SubmitRequirementStatus,
} from '../api/rest-api';
import {CheckResult, CheckRun, RunResult} from '../models/checks/checks-model';
-import {Category, RunStatus} from '../api/checks';
+import {Category, Fix, Link, LinkIcon, RunStatus} from '../api/checks';
import {DiffInfo} from '../api/diff';
import {SearchViewState} from '../models/views/search';
import {ChangeChildView, ChangeViewState} from '../models/views/change';
@@ -1147,6 +1147,29 @@
};
}
+export function createCheckFix(partial: Partial<Fix> = {}): Fix {
+ return {
+ description: 'this is a test fix',
+ replacements: [
+ {
+ path: 'testpath',
+ range: createRange(),
+ replacement: 'testreplacement',
+ },
+ ],
+ ...partial,
+ };
+}
+
+export function createCheckLink(partial: Partial<Link> = {}): Link {
+ return {
+ url: 'http://test/url',
+ primary: true,
+ icon: LinkIcon.EXTERNAL,
+ ...partial,
+ };
+}
+
export function createDetailedLabelInfo(): DetailedLabelInfo {
return {
values: {
