Revise the summary of the new Checks UI
We are introducing a dedicated chip for SUCCESS that is similar to the
chips for ERROR, WARNING and INFO. That mirrors what the results panel
in the checks panel shows.
We are introducing an overall "quota" for expanded chips. We want to
expand up to 3 chips, but for each category we don't want to show
expanded and collapsed chips. So for 4 errors we only show the collapsed
version.
We are from left to right with consuming the "quota". So if there only
running runs, then those runs can also consume the quota and be shown
expanded.
A tricky part is also that the expanded version is based on
a run and a check name, but the count of the collapsed chip refers to
results.
We are removing the "Runs" label separating INFO and SUCCESS. Instead
we are introducing a padding separator between SUCCESS and RUNNING,
which does not show, if there is only a running chip.
https://imgur.com/a/jBrCAHz
Change-Id: Ic633ac0ff6d91afdd3dfcec260ca1c95ee45cb66
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index eded5a1..86b714c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -14,24 +14,32 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {html, TemplateResult} from 'lit-html';
+import {html} from 'lit-html';
import {css, customElement, property} from 'lit-element';
import {GrLitElement} from '../../lit/gr-lit-element';
import {sharedStyles} from '../../../styles/shared-styles';
import {appContext} from '../../../services/app-context';
import {KnownExperimentId} from '../../../services/flags/flags';
-import {Category, CheckRun, Link} from '../../../api/checks';
import {
allRuns$,
aPluginHasRegistered,
- RunResult,
} from '../../../services/checks/checks-model';
+import {
+ Category,
+ CheckResult,
+ CheckRun,
+ Link,
+ RunStatus,
+} from '../../../api/checks';
import {fireShowPrimaryTab} from '../../../utils/event-util';
import '../../shared/gr-avatar/gr-avatar';
import {
- hasCompleted,
+ getResultsOf,
+ hasCompletedWithoutResults,
+ hasResultsOf,
+ iconForCategory,
+ iconForStatus,
isRunning,
- isRunningOrHasCompleted,
} from '../../../services/checks/checks-util';
import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
import {
@@ -45,18 +53,6 @@
import {notUndefined} from '../../../types/types';
import {uniqueDefinedAvatar} from '../../../utils/account-util';
-function filterResults(runs: CheckRun[], category: Category): RunResult[] {
- return runs.filter(isRunningOrHasCompleted).reduce((results, run) => {
- return results.concat(
- (run.results ?? [])
- .filter(result => result.category === category)
- .map(result => {
- return {...run, ...result};
- })
- );
- }, [] as RunResult[]);
-}
-
export enum SummaryChipStyles {
INFO = 'info',
WARNING = 'warning',
@@ -119,12 +115,10 @@
render() {
const chipClass = `summaryChip font-small ${this.styleType}`;
const grIcon = this.icon ? `gr-icons:${this.icon}` : '';
- return html`
- <div class="${chipClass}" role="button">
- ${this.icon && html`<iron-icon icon="${grIcon}"></iron-icon>`}
- <slot></slot>
- </div>
- `;
+ return html`<div class="${chipClass}" role="button">
+ ${this.icon && html`<iron-icon icon="${grIcon}"></iron-icon>`}
+ <slot></slot>
+ </div>`;
}
}
@@ -134,18 +128,15 @@
icon = '';
@property()
- expandMax = 0;
-
- @property()
- runs: CheckRun[] = [];
-
- @property()
- results: RunResult[] = [];
+ text = '';
static get styles() {
return [
sharedStyles,
css`
+ :host {
+ display: inline-block;
+ }
.checksChip {
color: var(--chip-color);
cursor: pointer;
@@ -157,7 +148,7 @@
border: 1px solid gray;
vertical-align: top;
}
- .checksChip .checkName {
+ .checksChip .text {
display: inline-block;
max-width: 120px;
white-space: nowrap;
@@ -170,11 +161,8 @@
height: var(--line-height-small);
vertical-align: top;
}
- div.checksChip iron-icon.launch {
- color: var(--gray-foreground);
- }
.checksChip.error {
- color: var(--error-color);
+ color: var(--error-foreground);
border-color: var(--error-foreground);
background-color: var(--error-background);
}
@@ -195,12 +183,14 @@
.checksChip.info-outline iron-icon {
color: var(--info-foreground);
}
- .checksChip.check {
- border-color: var(--gray-foreground);
- background-color: var(--gray-background);
+ .checksChip.check-circle {
+ border-color: var(--success-foreground);
+ background-color: var(--success-background);
}
- .checksChip.check iron-icon {
- color: var(--gray-foreground);
+ .checksChip.check-circle iron-icon {
+ color: var(--success-foreground);
+ }
+ .checksChip.timelapse {
}
.checksChip.timelapse {
border-color: var(--gray-foreground);
@@ -214,61 +204,31 @@
}
render() {
- const count = this.runs.length || this.results.length;
- if (count === 0) return;
- if (count > this.expandMax || !this.results.length) {
- return this.renderChip(html`${count}`);
- }
- return this.results.map(result =>
- this.renderChip(this.renderNameAndLinks(result))
- );
- }
-
- private renderChip(content: TemplateResult) {
+ if (!this.text) return;
const chipClass = `checksChip font-small ${this.icon}`;
const grIcon = `gr-icons:${this.icon}`;
return html`
<div class="${chipClass}" role="button" @click="${this.handleClick}">
<iron-icon icon="${grIcon}"></iron-icon>
- ${content}
+ <div class="text">${this.text}</div>
+ <slot></slot>
</div>
`;
}
- private renderNameAndLinks(result: RunResult) {
- return html`
- <div class="checkName">${result.checkName}</div>
- ${this.renderResultLinks(result.links ?? [])}
- `;
- }
-
- private renderResultLinks(links: Link[]) {
- return links
- .filter(link => link.primary)
- .slice(0, 2)
- .map(
- link => html`
- <a
- href="${link.url}"
- target="_blank"
- @click="${this.handleClickLink}"
- >
- <iron-icon class="launch" icon="gr-icons:launch"></iron-icon>
- </a>
- `
- );
- }
-
- private handleClick() {
+ private handleClick(e: MouseEvent) {
+ e.stopPropagation();
+ e.preventDefault();
fireShowPrimaryTab(this, 'checks');
}
-
- private handleClickLink(e: Event) {
- // Prevents handleClick() from reacting to <a> link clicks.
- e.stopPropagation();
- }
}
+/** What is the maximum number of expanded checks chips? */
+const DETAILS_QUOTA = 3;
+
+/** What is the maximum number of links renderend within one chip? */
+const MAX_LINKS_PER_CHIP = 3;
+
@customElement('gr-change-summary')
export class GrChangeSummary extends GrLitElement {
private readonly newChangeSummaryUiEnabled = appContext.flagsService.isEnabled(
@@ -290,6 +250,9 @@
@property()
showChecksSummary = false;
+ /** Is reset when rendering beings and decreases while chips are rendered. */
+ private detailsQuota = DETAILS_QUOTA;
+
constructor() {
super();
this.subscribe('runs', allRuns$);
@@ -314,9 +277,11 @@
td.value {
padding-right: var(--spacing-l);
}
- .runs {
- margin-right: var(--spacing-s);
- margin-left: var(--spacing-m);
+ iron-icon.launch {
+ color: var(--gray-foreground);
+ width: var(--line-height-small);
+ height: var(--line-height-small);
+ vertical-align: top;
}
gr-avatar {
height: var(--line-height-small, 16px);
@@ -328,11 +293,77 @@
];
}
+ renderChecksChipForCategory(category: Category) {
+ const icon = iconForCategory(category);
+ const runs = this.runs.filter(run => hasResultsOf(run, category));
+ const count = (run: CheckRun) => getResultsOf(run, category);
+ return this.renderChecksChip(icon, runs, count);
+ }
+
+ renderChecksChipForStatus(
+ status: RunStatus,
+ filter: (run: CheckRun) => boolean
+ ) {
+ const icon = iconForStatus(status);
+ const runs = this.runs.filter(filter);
+ return this.renderChecksChip(icon, runs, () => []);
+ }
+
+ renderChecksChip(
+ icon: string,
+ runs: CheckRun[],
+ resultFilter: (run: CheckRun) => CheckResult[]
+ ) {
+ if (runs.length === 0) {
+ return html``;
+ }
+ if (runs.length <= this.detailsQuota) {
+ this.detailsQuota -= runs.length;
+ return runs.map(run => {
+ const links = resultFilter(run)
+ .reduce((links, result) => {
+ return links.concat(result.links ?? []);
+ }, [] as Link[])
+ .filter(link => link.primary)
+ .slice(0, MAX_LINKS_PER_CHIP);
+ const count = resultFilter(run).length;
+ const countText = count > 1 ? ` ${count}` : '';
+ const text = `${run.checkName}${countText}`;
+ return html`<gr-checks-chip
+ class="${icon}"
+ .icon="${icon}"
+ .text="${text}"
+ >${links.map(
+ link => html`
+ <a href="${link.url}" target="_blank" @click="${this.onClick}"
+ ><iron-icon class="launch" icon="gr-icons:launch"></iron-icon
+ ></a>
+ `
+ )}
+ </gr-checks-chip>`;
+ });
+ }
+ // runs.length > this.detailsQuota
+ this.detailsQuota = 0;
+ const sum = runs.reduce(
+ (sum, run) => sum + (resultFilter(run).length || 1),
+ 0
+ );
+ if (sum === 0) return;
+ return html`<gr-checks-chip
+ class="${icon}"
+ .icon="${icon}"
+ .text="${sum}"
+ ></gr-checks-chip>`;
+ }
+
+ private onClick(e: MouseEvent) {
+ // Prevents handleClick() from reacting to <a> link clicks.
+ e.stopPropagation();
+ }
+
render() {
- const runs: CheckRun[] = this.runs;
- const errors = filterResults(runs, Category.ERROR);
- const warnings = filterResults(runs, Category.WARNING);
- const infos = filterResults(runs, Category.INFO);
+ this.detailsQuota = DETAILS_QUOTA;
const countResolvedComments =
this.commentThreads?.filter(isResolved).length ?? 0;
const unresolvedThreads = this.commentThreads?.filter(isUnresolved) ?? [];
@@ -345,31 +376,16 @@
<tr ?hidden=${!this.showChecksSummary}>
<td class="key">Checks</td>
<td class="value">
- <gr-checks-chip
- icon="error"
- .results="${errors}"
- expandMax="2"
- ></gr-checks-chip>
- <gr-checks-chip
- icon="warning"
- .results="${warnings}"
- expandMax="${2 - errors.length}"
- ></gr-checks-chip>
- <gr-checks-chip
- icon="info-outline"
- .results="${infos}"
- ></gr-checks-chip>
- <span ?hidden=${!runs.some(isRunningOrHasCompleted)} class="runs"
- >Runs</span
- >
- <gr-checks-chip
- icon="check"
- .runs="${runs.filter(hasCompleted)}"
- ></gr-checks-chip>
- <gr-checks-chip
- icon="timelapse"
- .runs="${runs.filter(isRunning)}"
- ></gr-checks-chip>
+ ${this.renderChecksChipForCategory(
+ Category.ERROR
+ )}${this.renderChecksChipForCategory(
+ Category.WARNING
+ )}${this.renderChecksChipForCategory(
+ Category.INFO
+ )}${this.renderChecksChipForStatus(
+ RunStatus.COMPLETED,
+ hasCompletedWithoutResults
+ )}${this.renderChecksChipForStatus(RunStatus.RUNNING, isRunning)}
</td>
</tr>
<tr ?hidden=${!this.newChangeSummaryUiEnabled}>
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index e800fbd..4bb11b3 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -19,22 +19,11 @@
import {GrLitElement} from '../lit/gr-lit-element';
import {Category, CheckRun, Link, RunStatus, Tag} from '../../api/checks';
import {sharedStyles} from '../../styles/shared-styles';
-import {assertNever} from '../../utils/common-util';
import {RunResult} from '../../services/checks/checks-model';
-import {hasCompletedWithoutResults} from '../../services/checks/checks-util';
-
-export function iconForCategory(category: Category) {
- switch (category) {
- case Category.ERROR:
- return 'error';
- case Category.INFO:
- return 'info-outline';
- case Category.WARNING:
- return 'warning';
- default:
- assertNever(category, `Unsupported category: ${category}`);
- }
-}
+import {
+ hasCompletedWithoutResults,
+ iconForCategory,
+} from '../../services/checks/checks-util';
@customElement('gr-result-row')
class GrResultRow extends GrLitElement {
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
index 2939089..82c346a 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
@@ -19,12 +19,10 @@
import {GrLitElement} from '../lit/gr-lit-element';
import {CheckRun, RunStatus} from '../../api/checks';
import {sharedStyles} from '../../styles/shared-styles';
-import {iconForCategory} from './gr-checks-results';
import {
compareByWorstCategory,
- worstCategory,
+ iconForRun,
} from '../../services/checks/checks-util';
-import {assertNever} from '../../utils/common-util';
import {
allRuns$,
fakeRun0,
@@ -36,33 +34,13 @@
} from '../../services/checks/checks-model';
function renderRun(run: CheckRun) {
- return html`<div class="runChip ${iconClass(run)}">
- ${renderIcon(run)}
+ const icon = iconForRun(run);
+ return html`<div class="runChip ${icon}">
+ <iron-icon icon="gr-icons:${icon}" class="${icon}"></iron-icon>
<span>${run.checkName}</span>
</div>`;
}
-function renderIcon(run: CheckRun) {
- const icon = iconClass(run);
- if (!icon) return;
- return html`<iron-icon icon="gr-icons:${icon}" class="${icon}"></iron-icon>`;
-}
-
-function iconClass(run: CheckRun) {
- const category = worstCategory(run);
- if (category) return iconForCategory(category);
- switch (run.status) {
- case RunStatus.COMPLETED:
- return 'check-circle';
- case RunStatus.RUNNABLE:
- return 'placeholder';
- case RunStatus.RUNNING:
- return 'timelapse';
- default:
- assertNever(run.status, `Unsupported status: ${run.status}`);
- }
-}
-
@customElement('gr-checks-runs')
export class GrChecksRuns extends GrLitElement {
@property()
diff --git a/polygerrit-ui/app/services/checks/checks-model.ts b/polygerrit-ui/app/services/checks/checks-model.ts
index 1116db0..9546d0b 100644
--- a/polygerrit-ui/app/services/checks/checks-model.ts
+++ b/polygerrit-ui/app/services/checks/checks-model.ts
@@ -113,6 +113,13 @@
],
tags: [{name: 'OBSOLETE'}, {name: 'E2E'}],
},
+ {
+ category: Category.ERROR,
+ summary: 'Running the mighty test has failed by crashing.',
+ links: [
+ {primary: true, url: 'https://www.google.com', icon: LinkIcon.EXTERNAL},
+ ],
+ },
],
status: RunStatus.COMPLETED,
};
diff --git a/polygerrit-ui/app/services/checks/checks-util.ts b/polygerrit-ui/app/services/checks/checks-util.ts
index de64e7c..344983f 100644
--- a/polygerrit-ui/app/services/checks/checks-util.ts
+++ b/polygerrit-ui/app/services/checks/checks-util.ts
@@ -15,16 +15,47 @@
* limitations under the License.
*/
import {Category, CheckRun, RunStatus} from '../../api/checks';
+import {assertNever} from '../../utils/common-util';
export function worstCategory(run: CheckRun) {
- const results = run.results ?? [];
- if (results.some(r => r.category === Category.ERROR)) return Category.ERROR;
- if (results.some(r => r.category === Category.WARNING))
- return Category.WARNING;
- if (results.some(r => r.category === Category.INFO)) return Category.INFO;
+ if (hasResultsOf(run, Category.ERROR)) return Category.ERROR;
+ if (hasResultsOf(run, Category.WARNING)) return Category.WARNING;
+ if (hasResultsOf(run, Category.INFO)) return Category.INFO;
return undefined;
}
+export function iconForCategory(category: Category) {
+ switch (category) {
+ case Category.ERROR:
+ return 'error';
+ case Category.INFO:
+ return 'info-outline';
+ case Category.WARNING:
+ return 'warning';
+ default:
+ assertNever(category, `Unsupported category: ${category}`);
+ }
+}
+
+export function iconForRun(run: CheckRun) {
+ const category = worstCategory(run);
+ return category ? iconForCategory(category) : iconForStatus(run.status);
+}
+
+export function iconForStatus(status: RunStatus) {
+ switch (status) {
+ // Note that this is only for COMPLETED without results!
+ case RunStatus.COMPLETED:
+ return 'check-circle';
+ case RunStatus.RUNNABLE:
+ return 'placeholder';
+ case RunStatus.RUNNING:
+ return 'timelapse';
+ default:
+ assertNever(status, `Unsupported status: ${status}`);
+ }
+}
+
export function hasCompleted(run: CheckRun) {
return run.status === RunStatus.COMPLETED;
}
@@ -41,6 +72,18 @@
return run.status === RunStatus.COMPLETED && (run.results ?? []).length === 0;
}
+export function hasCompletedWith(run: CheckRun, category: Category) {
+ return hasCompleted(run) && hasResultsOf(run, category);
+}
+
+export function hasResultsOf(run: CheckRun, category: Category) {
+ return getResultsOf(run, category).length > 0;
+}
+
+export function getResultsOf(run: CheckRun, category: Category) {
+ return (run.results ?? []).filter(r => r.category === category);
+}
+
export function compareByWorstCategory(a: CheckRun, b: CheckRun) {
return level(worstCategory(b)) - level(worstCategory(a));
}
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index 7636734..15099e2 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -77,6 +77,7 @@
--info-deemphasized-foreground: var(--gray-300);
--info-deemphasized-background: var(--gray-50);
--success-foreground: var(--green-700);
+ --success-background: var(--green-50);
--gray-foreground: var(--gray-700);
--gray-background: var(--gray-100);
--tag-background: var(--cyan-100);
