Better use of horizontal space for check runs Change the appearance of the collapsed version of the check runs panel: Do not completely collapse it, but show a 90px version without buttons and event listeners, etc. Allow the user to click anywhere for expanding it. Automatically switch between the collapsed and expanded state of the check runs panel when the threshold of 1200 pixels screen width is crossed. In expanded state set the width of the runs panel to 20%, which is smaller than it was before. This allows users with medium screen width (1200-1500px) to see more content of check results. Drop the max-width:1600px rule for check results. So for really large screen we just use all the available spaec for showing the results table. Release-Notes: skip Google-Bug-Id: b/235185417 Change-Id: Ie915a1e56a8ea80b705562641fa25b36180f4224
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts index 629aa5c..99ce030 100644 --- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts +++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -861,7 +861,6 @@ } .headerTopRow, .headerBottomRow { - max-width: 1600px; display: flex; justify-content: space-between; align-items: flex-end; @@ -987,7 +986,6 @@ } .noResultsMessage { width: 100%; - max-width: 1600px; margin-top: var(--spacing-m); background-color: var(--background-color-primary); box-shadow: var(--elevation-level-1); @@ -996,7 +994,6 @@ } table.resultsTable { width: 100%; - max-width: 1600px; table-layout: fixed; margin-top: var(--spacing-m); background-color: var(--background-color-primary);
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts index 2ed0468..ef69e49 100644 --- a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts +++ b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
@@ -56,6 +56,7 @@ import {checksModelToken} from '../../models/checks/checks-model'; import {Interaction} from '../../constants/reporting'; import {Deduping} from '../../api/reporting'; +import {when} from 'lit/directives/when'; @customElement('gr-checks-run') export class GrChecksRun extends LitElement { @@ -67,6 +68,14 @@ display: block; --thick-border: 6px; } + :host([condensed]) .eta, + :host([condensed]) .middle, + :host([condensed]) .right { + display: none; + } + :host([condensed]) * { + pointer-events: none; + } .chip { display: flex; justify-content: space-between; @@ -131,7 +140,7 @@ gr-icon.check_circle { color: var(--success-foreground); } - div.chip:hover { + :host(:not([condensed])) div.chip:hover { background-color: var(--hover-background-color); } div.chip:focus-within { @@ -195,6 +204,9 @@ @property({attribute: false}) deselected = false; + @property({type: Boolean}) + condensed = false; + @state() shouldRender = false; @@ -445,6 +457,9 @@ () => this.getChecksModel().loginCallbackLatest$, x => (this.loginCallback = x) ); + this.addEventListener('click', () => { + if (this.collapsed) this.toggleCollapsed(); + }); } static override get styles() { @@ -456,12 +471,20 @@ display: block; } :host(:not([collapsed])) { - min-width: 320px; + width: 20%; padding: var(--spacing-l) var(--spacing-xl) var(--spacing-xl) var(--spacing-xl); } :host([collapsed]) { - padding: var(--spacing-l) 0; + width: 90px; + padding: var(--spacing-l) var(--spacing-l) var(--spacing-xl) + var(--spacing-l); + } + :host([collapsed]) * { + pointer-events: none; + } + :host([collapsed]:hover) { + cursor: pointer; } .title { display: flex; @@ -476,25 +499,28 @@ .title gr-button.expandButton { --gr-button-padding: var(--spacing-xs) var(--spacing-s); } - :host(:not([collapsed])) .expandButton { + :host .expandButton { margin-right: calc(0px - var(--spacing-m)); } - .expandIcon { - width: var(--line-height-h3); - height: var(--line-height-h3); + :host([collapsed]:hover) .expandButton { + background: var(--gray-background-hover); + border-radius: var(--border-radius); } .sectionHeader { padding-top: var(--spacing-l); text-transform: capitalize; cursor: default; } + :host([collapsed]) .sectionHeader { + cursor: pointer; + } .sectionHeader h3 { display: inline-block; } - .collapsed .sectionRuns { + :host(:not([collapsed])) .collapsed .sectionRuns { display: none; } - .collapsed { + :host(:not([collapsed])) .collapsed { border-bottom: 1px solid var(--border-color); padding-bottom: var(--spacing-m); } @@ -584,9 +610,6 @@ } override render() { - if (this.collapsed) { - return html`${this.renderCollapseButton()}`; - } return html` <h2 class="title"> <div class="heading-2">Runs</div> @@ -608,35 +631,39 @@ } private renderZeroState() { + if (this.collapsed) return; if (this.runs.length > 0) return; return html`<div class="zero">No Check Run to show</div>`; } private renderErrors() { - return Object.entries(this.errorMessages).map( - ([plugin, message]) => - html` - <div class="error"> - <div class="left"> - <gr-icon icon="error" filled></gr-icon> - </div> - <div class="right"> - <div class="message"> - Error while fetching results for ${plugin}:<br />${message} - </div> - </div> + return Object.entries(this.errorMessages).map(([plugin, message]) => { + const msg = this.collapsed + ? 'Error' + : `Error while fetching results for ${plugin}:<br />${message}`; + return html` + <div class="error"> + <div class="left"> + <gr-icon icon="error" filled></gr-icon> </div> - ` - ); + <div class="right"> + <div class="message">${msg}</div> + </div> + </div> + `; + }); } private renderSignIn() { if (!this.loginCallback) return; + const message = this.collapsed + ? 'Sign in' + : 'Sign in to Checks Plugin to see runs and results'; return html` <div class="login"> <div> <gr-icon icon="info"></gr-icon> - Sign in to Checks Plugin to see runs and results + ${message} </div> <div class="buttonRow"> <gr-button @click=${this.loginCallback} link>Sign in</gr-button> @@ -646,6 +673,7 @@ } private renderTitleButtons() { + if (this.collapsed) return; if (this.selectedRuns.length < 2) return; const actions = this.selectedRuns.map(selected => { const run = this.runs.find( @@ -702,24 +730,28 @@ > <gr-button link - class="expandButton" + class="expandButton font-normal" role="switch" aria-checked=${this.collapsed ? 'true' : 'false'} aria-label=${this.collapsed ? 'Expand runs panel' : 'Collapse runs panel'} @click=${this.toggleCollapsed} - ><gr-icon - icon=${this.collapsed ? 'chevron_right' : 'chevron_left'} - class="expandIcon" - > - </gr-icon> + > + <div> + <gr-icon + icon=${this.collapsed ? 'chevron_right' : 'chevron_left'} + class="expandIcon" + > + </gr-icon> + </div> </gr-button> </gr-tooltip-content> `; } - private toggleCollapsed() { + private toggleCollapsed(event?: Event) { + if (event) event.stopPropagation(); this.collapsed = !this.collapsed; this.reporting.reportInteraction(Interaction.CHECKS_RUNS_PANEL_TOGGLE, { collapsed: this.collapsed, @@ -776,11 +808,16 @@ if (runs.some(r => r.status === RunStatus.SCHEDULED)) { header = `${header} / ${headerForStatus(RunStatus.SCHEDULED)}`; } + const count = when(!this.collapsed, () => html` (${runs.length})`); + const grIcon = when( + !this.collapsed, + () => html`<gr-icon icon=${icon} class="expandIcon"></gr-icon>` + ); return html` <div class="${status.toLowerCase()} ${expandedClass}"> <div class="sectionHeader" @click=${() => this.toggleExpanded(status)}> - <gr-icon icon=${icon} class="expandIcon"></gr-icon> - <h3 class="heading-3">${header} (${runs.length})</h3> + ${grIcon} + <h3 class="heading-3">${header}${count}</h3> </div> <div class="sectionRuns">${runs.map(run => this.renderRun(run))}</div> </div> @@ -788,6 +825,7 @@ } toggleExpanded(status: RunStatus) { + if (this.collapsed) return; const expanded = this.isSectionExpanded.get(status) ?? true; this.isSectionExpanded.set(status, !expanded); this.reporting.reportInteraction(Interaction.CHECKS_RUN_SECTION_TOGGLE, { @@ -803,6 +841,7 @@ const deselected = !selectedRun && this.selectedRuns.length > 0; return html`<gr-checks-run .run=${run} + ?condensed=${this.collapsed} .selected=${selectedRun} .selectedAttempt=${selectedAttempt} .deselected=${deselected} @@ -810,11 +849,13 @@ } showFilter(): boolean { + if (this.collapsed) return false; return this.runs.length > 10 || !!this.filterRegExp; } renderFakeControls() { if (!this.flagService.isEnabled(KnownExperimentId.CHECKS_DEVELOPER)) return; + if (this.collapsed) return; return html` <div class="testing"> <div>Toggle fake runs by clicking buttons:</div>
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-runs_test.ts b/polygerrit-ui/app/elements/checks/gr-checks-runs_test.ts index da745a8..ba14e65 100644 --- a/polygerrit-ui/app/elements/checks/gr-checks-runs_test.ts +++ b/polygerrit-ui/app/elements/checks/gr-checks-runs_test.ts
@@ -41,11 +41,13 @@ <gr-button aria-checked="false" aria-label="Collapse runs panel" - class="expandButton" + class="expandButton font-normal" link="" role="switch" > - <gr-icon icon="chevron_left" class="expandIcon"></gr-icon> + <div> + <gr-icon icon="chevron_left" class="expandIcon"></gr-icon> + </div> </gr-button> </gr-tooltip-content> </h2> @@ -95,17 +97,56 @@ assert.equal(element.runs.length, 44); expect(element).shadowDom.to.equal( /* HTML */ ` - <gr-tooltip-content has-tooltip="" title="Expand runs panel"> - <gr-button - aria-checked="true" - aria-label="Expand runs panel" - class="expandButton" - link="" - role="switch" - > - <gr-icon icon="chevron_right" class="expandIcon"></gr-icon> - </gr-button> - </gr-tooltip-content> + <h2 class="title"> + <div class="heading-2">Runs</div> + <div class="flex-space"></div> + <gr-tooltip-content has-tooltip="" title="Expand runs panel"> + <gr-button + aria-checked="true" + aria-label="Expand runs panel" + class="expandButton font-normal" + link="" + role="switch" + > + <div> + <gr-icon icon="chevron_right" class="expandIcon"></gr-icon> + </div> + </gr-button> + </gr-tooltip-content> + </h2> + <input + hidden + id="filterInput" + placeholder="Filter runs by regular expression" + type="text" + /> + <div class="expanded running"> + <div class="sectionHeader"> + <h3 class="heading-3">Running / Scheduled</h3> + </div> + <div class="sectionRuns"> + <gr-checks-run condensed></gr-checks-run> + <gr-checks-run condensed></gr-checks-run> + </div> + </div> + <div class="completed expanded"> + <div class="sectionHeader"> + <h3 class="heading-3">Completed</h3> + </div> + <div class="sectionRuns"> + <gr-checks-run condensed></gr-checks-run> + <gr-checks-run condensed></gr-checks-run> + <gr-checks-run condensed></gr-checks-run> + </div> + </div> + <div class="expanded runnable"> + <div class="sectionHeader"> + <h3 class="heading-3">Not run</h3> + </div> + <div class="sectionRuns"> + <gr-checks-run condensed></gr-checks-run> + </div> + </div> `, {ignoreAttributes: ['tabindex', 'aria-disabled']} );
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts index f62673b..c890d18 100644 --- a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts +++ b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
@@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ import {LitElement, css, html, PropertyValues} from 'lit'; -import {customElement, property, state} from 'lit/decorators'; +import {customElement, property, query, state} from 'lit/decorators'; import { CheckResult, CheckRun, @@ -21,6 +21,7 @@ import {Deduping} from '../../api/reporting'; import {Interaction} from '../../constants/reporting'; import {resolve} from '../../models/dependency'; +import {GrChecksRuns} from './gr-checks-runs'; /** * The "Checks" tab on the Gerrit change page. Gets its data from plugins that @@ -28,6 +29,9 @@ */ @customElement('gr-checks-tab') export class GrChecksTab extends LitElement { + @query('.runs') + checksRuns?: GrChecksRuns; + @state() runs: CheckRun[] = []; @@ -62,6 +66,8 @@ private readonly reporting = getAppContext().reportingService; + private offsetWidthBefore = 0; + constructor() { super(); subscribe( @@ -89,6 +95,14 @@ () => this.getChangeModel().changeNum$, x => (this.changeNum = x) ); + const observer = new ResizeObserver(() => { + if (!this.checksRuns) return; + // The appearance of a scroll bar (<40px width) should not trigger. + if (Math.abs(this.offsetWidth - this.offsetWidthBefore) < 40) return; + this.offsetWidthBefore = this.offsetWidth; + this.checksRuns.collapsed = this.offsetWidth < 1200; + }); + observer.observe(this); } static override get styles() { @@ -102,9 +116,10 @@ .runs { min-height: 400px; border-right: 1px solid var(--border-color); + flex: 0 0 auto; } .results { - flex-grow: 1; + flex: 1 1 auto; } `; }