Merge changes I6e06517d,I2a20d0f0 * changes: Add a `checksRunsSelected` URL parameter Add a `tab` URL parameter that is reflected by change view state
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts index 8293daa..0ad4a07 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -104,12 +104,7 @@ import {GrIncludedInDialog} from '../gr-included-in-dialog/gr-included-in-dialog'; import {GrDownloadDialog} from '../gr-download-dialog/gr-download-dialog'; import {GrChangeMetadata} from '../gr-change-metadata/gr-change-metadata'; -import { - assertIsDefined, - assert, - query as queryEl, - queryAll, -} from '../../../utils/common-util'; +import {assertIsDefined, assert, queryAll} from '../../../utils/common-util'; import {GrEditControls} from '../../edit/gr-edit-controls/gr-edit-controls'; import { CommentThread, @@ -121,14 +116,12 @@ import {GrFileList} from '../gr-file-list/gr-file-list'; import {EditRevisionInfo, ParsedChangeInfo} from '../../../types/types'; import { - ChecksTabState, CloseFixPreviewEvent, EditableContentSaveEvent, EventType, OpenFixPreviewEvent, ShowAlertEventDetail, SwitchTabEvent, - SwitchTabEventDetail, TabState, ValueChangedEvent, } from '../../../types/events'; @@ -486,9 +479,12 @@ @state() private changeViewAriaHidden = false; + /** + * This can be a string only for plugin provided tabs. + */ // visible for testing @state() - activeTab = Tab.FILES; + activeTab: Tab | string = Tab.FILES; @property({type: Boolean}) unresolvedOnly = true; @@ -702,6 +698,11 @@ ); subscribe( this, + () => this.getViewModel().tab$, + t => (this.activeTab = t ?? Tab.FILES) + ); + subscribe( + this, () => this.getChecksModel().aPluginHasRegistered$, b => { this.showChecksTab = b; @@ -1496,7 +1497,10 @@ private renderTabHeaders() { return html` - <paper-tabs id="tabs" @selected-changed=${this.setActiveTab}> + <paper-tabs + id="tabs" + @selected-changed=${this.onPaperTabSelectionChanged} + > <paper-tab @click=${this.onPaperTabClick} data-name=${Tab.FILES} ><span>Files</span></paper-tab > @@ -1729,38 +1733,38 @@ } } - setActiveTab(e: SwitchTabEvent) { - const paperTabs = queryEl<PaperTabsElement>(this, '#tabs'); - if (!paperTabs) return; - const tabs = [...queryAll<HTMLElement>(paperTabs, 'paper-tab')]; + onPaperTabSelectionChanged(e: ValueChangedEvent) { + if (!this.tabs) return; + const tabs = [...queryAll<HTMLElement>(this.tabs, 'paper-tab')]; if (!tabs) return; - let tabName = e.detail.tab; - let tabIndex = e.detail.value; + const tabIndex = Number(e.detail.value); + assert( + Number.isInteger(tabIndex) && 0 <= tabIndex && tabIndex < tabs.length, + `${tabIndex} must be integer` + ); + const tab = tabs[tabIndex].dataset['name']; - if (tabIndex === undefined) { - assert(tabName !== undefined, 'tabName or tabIndex must be defined'); - tabIndex = tabs.findIndex(t => t.dataset['name'] === tabName); - assert(tabIndex !== -1, `tab ${tabName} not found`); + this.getViewModel().updateState({tab}); + } + + setActiveTab(e: SwitchTabEvent) { + if (!this.tabs) return; + const tabs = [...queryAll<HTMLElement>(this.tabs, 'paper-tab')]; + if (!tabs) return; + + const tab = e.detail.tab; + const tabIndex = tabs.findIndex(t => t.dataset['name'] === tab); + assert(tabIndex !== -1, `tab ${tab} not found`); + + if (this.tabs.selected !== tabIndex) { + this.tabs.selected = tabIndex; } - if (tabName === undefined) { - tabName = tabs[tabIndex].dataset['name']; - } - - if (paperTabs.selected !== tabIndex) { - // paperTabs.selected is undefined during rendering - if (paperTabs.selected !== undefined) { - const src = (e.composedPath()?.[0] as Element | undefined)?.tagName; - this.reporting.reportInteraction(Interaction.SHOW_TAB, {tabName, src}); - } - paperTabs.selected = tabIndex; - } - - this.activeTab = tabName as Tab; + this.getViewModel().updateState({tab}); if (e.detail.tabState) this.tabState = e.detail.tabState; - if (e.detail.scrollIntoView) paperTabs.scrollIntoView({block: 'center'}); + if (e.detail.scrollIntoView) this.tabs.scrollIntoView({block: 'center'}); } /** @@ -2246,19 +2250,7 @@ } else if (this.viewState?.commentId) { tab = Tab.COMMENT_THREADS; } - const detail: SwitchTabEventDetail = { - tab, - }; - if (tab === Tab.CHECKS) { - const state: ChecksTabState = {}; - detail.tabState = {checksTab: state}; - } - - this.setActiveTab( - new CustomEvent(EventType.SHOW_TAB, { - detail, - }) - ); + this.setActiveTab(new CustomEvent(EventType.SHOW_TAB, {detail: {tab}})); } // Private but used in tests.
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts index a4e26fe..ddc3716 100644 --- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts +++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -77,6 +77,7 @@ import {DropdownItem} from '../shared/gr-dropdown-list/gr-dropdown-list'; import './gr-checks-attempt'; import {createDiffUrl} from '../../models/views/diff'; +import {changeViewModelToken} from '../../models/views/change'; /** * Firing this event sets the regular expression of the results filter. @@ -766,7 +767,7 @@ * Check names of runs that are selected in the runs panel. When this array * is empty, then no run is selected and all runs should be shown. */ - @property({attribute: false}) + @state() selectedRuns: string[] = []; @state() @@ -808,6 +809,8 @@ */ private isSectionExpandedByUser = new Map<Category, boolean>(); + private readonly getViewModel = resolve(this, changeViewModelToken); + private readonly getChangeModel = resolve(this, changeModelToken); private readonly getChecksModel = resolve(this, checksModelToken); @@ -853,6 +856,11 @@ () => this.getChecksModel().someProvidersAreLoadingSelected$, x => (this.someProvidersAreLoading = x) ); + subscribe( + this, + () => this.getViewModel().checksRunsSelected$, + x => (this.selectedRuns = x) + ); } static override get styles() {
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts index 09a2558..b068426 100644 --- a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts +++ b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
@@ -44,7 +44,7 @@ } from '../../models/checks/checks-fakes'; import {assertIsDefined} from '../../utils/common-util'; import {modifierPressed, whenVisible} from '../../utils/dom-util'; -import {fireRunSelected, fireRunSelectionReset} from './gr-checks-util'; +import {fireRunSelected, RunSelectedEvent} from './gr-checks-util'; import {ChecksTabState} from '../../types/events'; import {charsOnly} from '../../utils/string-util'; import {getAppContext} from '../../services/app-context'; @@ -57,6 +57,7 @@ import {Interaction} from '../../constants/reporting'; import {Deduping} from '../../api/reporting'; import {when} from 'lit/directives/when.js'; +import {changeViewModelToken} from '../../models/views/change'; @customElement('gr-checks-run') export class GrChecksRun extends LitElement { @@ -403,7 +404,7 @@ @property({type: Boolean, reflect: true}) collapsed = false; - @property({attribute: false}) + @state() selectedRuns: string[] = []; @state() @@ -424,6 +425,8 @@ private getChecksModel = resolve(this, checksModelToken); + private readonly getViewModel = resolve(this, changeViewModelToken); + private readonly reporting = getAppContext().reportingService; constructor() { @@ -453,6 +456,11 @@ () => this.getChecksModel().runFilterRegexp$, x => (this.filterRegExp = x) ); + subscribe( + this, + () => this.getViewModel().checksRunsSelected$, + x => (this.selectedRuns = x) + ); this.addEventListener('click', () => { if (this.collapsed) this.toggleCollapsed(); }); @@ -676,7 +684,8 @@ <gr-button class="font-normal" link - @click=${() => fireRunSelectionReset(this)} + @click=${() => + this.getViewModel().updateState({checksRunsSelected: []})} >Unselect All</gr-button > <gr-tooltip-content @@ -827,9 +836,16 @@ ?condensed=${this.collapsed} .selected=${selectedRun} .deselected=${deselected} + @run-selected=${this.handleRunSelected} ></gr-checks-run>`; } + handleRunSelected(e: RunSelectedEvent) { + if (e.detail.checkName) { + this.getViewModel().toggleSelectedCheckRun(e.detail.checkName); + } + } + showFilter(): boolean { if (this.collapsed) return false; return this.runs.length > 10 || !!this.filterRegExp;
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts index 82893bc..c1bcdc1 100644 --- a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts +++ b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
@@ -14,7 +14,6 @@ import './gr-checks-runs'; import './gr-checks-results'; import {NumericChangeId, PatchSetNumber} from '../../types/common'; -import {RunSelectedEvent} from './gr-checks-util'; import {TabState} from '../../types/events'; import {getAppContext} from '../../services/app-context'; import {subscribe} from '../lit/subscription-controller'; @@ -50,9 +49,6 @@ @state() changeNum: NumericChangeId | undefined = undefined; - @state() - selectedRuns: string[] = []; - private readonly getChangeModel = resolve(this, changeModelToken); private readonly getChecksModel = resolve(this, checksModelToken); @@ -132,41 +128,16 @@ class="runs" ?collapsed=${this.offsetWidth < 1000} .runs=${this.runs} - .selectedRuns=${this.selectedRuns} .tabState=${this.tabState?.checksTab} - @run-selected=${this.handleRunSelected} ></gr-checks-runs> <gr-checks-results class="results" .tabState=${this.tabState?.checksTab} .runs=${this.runs} - .selectedRuns=${this.selectedRuns} ></gr-checks-results> </div> `; } - - handleRunSelected(e: RunSelectedEvent) { - this.reporting.reportInteraction(Interaction.CHECKS_RUN_SELECTED, { - checkName: e.detail.checkName, - reset: e.detail.reset, - }); - if (e.detail.reset) { - this.selectedRuns = []; - return; - } - if (e.detail.checkName) { - this.toggleSelected(e.detail.checkName); - } - } - - toggleSelected(checkName: string) { - if (this.selectedRuns.includes(checkName)) { - this.selectedRuns = this.selectedRuns.filter(r => r !== checkName); - } else { - this.selectedRuns = [...this.selectedRuns, checkName]; - } - } } declare global {
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-util.ts b/polygerrit-ui/app/elements/checks/gr-checks-util.ts index 939a87e..c7477c4 100644 --- a/polygerrit-ui/app/elements/checks/gr-checks-util.ts +++ b/polygerrit-ui/app/elements/checks/gr-checks-util.ts
@@ -11,7 +11,6 @@ } from '../../models/checks/checks-util'; export interface RunSelectedEventDetail { - reset: boolean; checkName?: string; } @@ -33,16 +32,6 @@ ); } -export function fireRunSelectionReset(target: EventTarget) { - target.dispatchEvent( - new CustomEvent('run-selected', { - detail: {reset: true}, - composed: true, - bubbles: true, - }) - ); -} - export function isAttemptSelected( selectedAttempt: AttemptChoice, run: CheckRun
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts index 74589d3..1701464 100644 --- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts +++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -1408,6 +1408,8 @@ if (filter) state.filter = filter; const attempt = stringToAttemptChoice(queryMap.get('attempt')); if (attempt && attempt !== LATEST_ATTEMPT) state.attempt = attempt; + const selected = queryMap.get('checksRunsSelected'); + if (selected) state.checksRunsSelected = selected.split(','); assertIsDefined(state.project, 'project'); this.reporting.setRepoName(state.project);
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts index ce3ed0d..9669ec7 100644 --- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts +++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
@@ -1149,6 +1149,7 @@ queryMap.set('filter', 'fff'); queryMap.set('select', 'sss'); queryMap.set('attempt', '1'); + queryMap.set('checksRunsSelected', 'asdf,qwer'); ctx.querystring = queryMap.toString(); assertctxToParams(ctx, 'handleChangeRoute', { view: GerritView.CHANGE, @@ -1159,6 +1160,7 @@ attempt: 1, filter: 'fff', tab: 'checks', + checksRunsSelected: ['asdf', 'qwer'], }); }); });
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts index 826ec66..0200d75 100644 --- a/polygerrit-ui/app/models/views/change.ts +++ b/polygerrit-ui/app/models/views/change.ts
@@ -11,8 +11,10 @@ ChangeInfo, PatchSetNumber, } from '../../api/rest-api'; +import {Tab} from '../../constants/constants'; import {GerritView} from '../../services/router/router-model'; import {UrlEncodedCommentId} from '../../types/common'; +import {toggle} from '../../utils/common-util'; import {select} from '../../utils/observable-util'; import { encodeURL, @@ -33,7 +35,8 @@ patchNum?: RevisionPatchSetNum; basePatchNum?: BasePatchSetNum; commentId?: UrlEncodedCommentId; - tab?: string; + /** This can be a string only for plugin provided tabs. */ + tab?: Tab | string; /** Checks related view state */ @@ -43,6 +46,8 @@ filter?: string; /** selected attempt for check runs (undefined=latest) */ attempt?: AttemptChoice; + /** selected check runs identified by `checkName` */ + checksRunsSelected?: string[]; /** State properties that trigger one-time actions */ @@ -107,6 +112,12 @@ if (state.filter) { queries.push(`filter=${state.filter}`); } + if (state.checksRunsSelected && state.checksRunsSelected.length > 0) { + queries.push(`checksRunsSelected=${[...state.checksRunsSelected].sort()}`); + } + if (state.tab && state.tab !== Tab.FILES) { + queries.push(`tab=${state.tab}`); + } if (state.forceReload) { queries.push('forceReload=true'); } @@ -151,6 +162,11 @@ public readonly filter$ = select(this.state$, state => state?.filter); + public readonly checksRunsSelected$ = select( + this.state$, + state => state?.checksRunsSelected ?? [] + ); + constructor() { super(undefined); this.state$.subscribe(s => { @@ -163,4 +179,9 @@ } }); } + + toggleSelectedCheckRun(checkName: string) { + const selected = this.getState()?.checksRunsSelected ?? []; + this.updateState({checksRunsSelected: toggle(selected, checkName)}); + } }
diff --git a/polygerrit-ui/app/models/views/change_test.ts b/polygerrit-ui/app/models/views/change_test.ts index 2f05e9f..583ff6e 100644 --- a/polygerrit-ui/app/models/views/change_test.ts +++ b/polygerrit-ui/app/models/views/change_test.ts
@@ -14,13 +14,15 @@ import '../../test/common-test-setup'; import {createChangeUrl, ChangeViewState} from './change'; +const STATE: ChangeViewState = { + view: GerritView.CHANGE, + changeNum: 1234 as NumericChangeId, + project: 'test' as RepoName, +}; + suite('change view state tests', () => { test('createChangeUrl()', () => { - const state: ChangeViewState = { - view: GerritView.CHANGE, - changeNum: 1234 as NumericChangeId, - project: 'test' as RepoName, - }; + const state: ChangeViewState = {...STATE}; assert.equal(createChangeUrl(state), '/c/test/+/1234'); @@ -34,6 +36,18 @@ assert.equal(createChangeUrl(state), '/c/test/+/1234/5..10#123'); }); + test('createChangeUrl() checksRunsSelected', () => { + const state: ChangeViewState = { + ...STATE, + checksRunsSelected: ['asdf'], + }; + + assert.equal( + createChangeUrl(state), + '/c/test/+/1234?checksRunsSelected=asdf' + ); + }); + test('createChangeUrl() with repo name encoding', () => { const state: ChangeViewState = { view: GerritView.CHANGE,
diff --git a/polygerrit-ui/app/types/events.ts b/polygerrit-ui/app/types/events.ts index 597bb6f..496513d 100644 --- a/polygerrit-ui/app/types/events.ts +++ b/polygerrit-ui/app/types/events.ts
@@ -209,9 +209,7 @@ // Type for the custom event to switch tab. export interface SwitchTabEventDetail { // name of the tab to set as active, from custom event - tab?: string; - // index of tab to set as active, from paper-tabs event - value?: number; + tab: string; // scroll into the tab afterwards, from custom event scrollIntoView?: boolean; // define state of tab after opening
diff --git a/polygerrit-ui/app/utils/common-util.ts b/polygerrit-ui/app/utils/common-util.ts index 05c054b..03306f5 100644 --- a/polygerrit-ui/app/utils/common-util.ts +++ b/polygerrit-ui/app/utils/common-util.ts
@@ -129,6 +129,14 @@ return array.indexOf(item) === index; } +export function toggle<T>(array: T[], item: T): T[] { + if (array.includes(item)) { + return array.filter(r => r !== item); + } else { + return array.concat([item]); + } +} + /** * Returns the elements that are present in every sub-array. If a compareBy * predicate is passed in, it will be used instead of strict equality. A new
diff --git a/polygerrit-ui/app/utils/common-util_test.ts b/polygerrit-ui/app/utils/common-util_test.ts index 0d85f34..76c8a6c 100644 --- a/polygerrit-ui/app/utils/common-util_test.ts +++ b/polygerrit-ui/app/utils/common-util_test.ts
@@ -11,6 +11,7 @@ containsAll, intersection, difference, + toggle, } from './common-util'; suite('common-util tests', () => { @@ -97,4 +98,11 @@ assert.deepEqual(difference([1, 2, 3], [1, 2, 3]), []); assert.deepEqual(difference([1, 2, 3], [4, 5, 6]), [1, 2, 3]); }); + + test('toggle', () => { + assert.deepEqual(toggle([], 1), [1]); + assert.deepEqual(toggle([1], 1), []); + assert.deepEqual(toggle([1, 2, 3], 1), [2, 3]); + assert.deepEqual(toggle([2, 3], 1), [2, 3, 1]); + }); });