Stop using page.js directly from random components Instead use the navigation service abstraction. `navService.setUrl()` is a 1:1 replacement for `page.show()`. With change the only direct dependency on page.js will be in gr-router. That is beneficial because of general encapsulation, but more concretely it will allow us to stop using the *global* page.js instance. The router will then be able to instantiate its own instance, which is also beneficial for testing. Release-Notes: skip Change-Id: Icc1470e50eca1624482be171498abc017a408aa1
diff --git a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts index 8d5689c..96688e9 100644 --- a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts +++ b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts
@@ -6,7 +6,6 @@ import '@polymer/iron-input/iron-input'; import '../../../styles/gr-form-styles'; import '../../../styles/shared-styles'; -import {page} from '../../../utils/page-wrapper-utils'; import {GroupId, GroupName} from '../../../types/common'; import {getAppContext} from '../../../services/app-context'; import {formStyles} from '../../../styles/gr-form-styles'; @@ -16,6 +15,8 @@ import {BindValueChangeEvent} from '../../../types/events'; import {fireEvent} from '../../../utils/event-util'; import {createGroupUrl} from '../../../models/views/group'; +import {resolve} from '../../../models/dependency'; +import {navigationToken} from '../../core/gr-navigation/gr-navigation'; declare global { interface HTMLElementTagNameMap { @@ -32,6 +33,8 @@ private readonly restApiService = getAppContext().restApiService; + private readonly getNavigation = resolve(this, navigationToken); + static override get styles() { return [ formStyles, @@ -86,8 +89,7 @@ return this.restApiService.getGroupConfig(name).then(group => { if (!group) return; const groupId = String(group.group_id!) as GroupId; - // TODO: Use navigation service instead of `page.show()` directly. - page.show(createGroupUrl({groupId})); + this.getNavigation().setUrl(createGroupUrl({groupId})); }); }); }
diff --git a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog_test.ts b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog_test.ts index 2a0b539..6e36d8c 100644 --- a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog_test.ts +++ b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog_test.ts
@@ -6,7 +6,6 @@ import '../../../test/common-test-setup'; import './gr-create-group-dialog'; import {GrCreateGroupDialog} from './gr-create-group-dialog'; -import {page} from '../../../utils/page-wrapper-utils'; import { mockPromise, queryAndAssert, @@ -15,6 +14,8 @@ import {IronInputElement} from '@polymer/iron-input'; import {GroupId} from '../../../types/common'; import {fixture, html, assert} from '@open-wc/testing'; +import {testResolver} from '../../../test/common-test-setup'; +import {navigationToken} from '../../core/gr-navigation/gr-navigation'; suite('gr-create-group-dialog tests', () => { let element: GrCreateGroupDialog; @@ -68,9 +69,9 @@ Promise.resolve({id: 'testId551' as GroupId, group_id: 551}) ); - const showStub = sinon.stub(page, 'show'); + const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl'); await element.handleCreateGroup(); - assert.isTrue(showStub.calledWith('/admin/groups/551')); + assert.isTrue(setUrlStub.calledWith('/admin/groups/551')); }); test('test for unsuccessful group creation', async () => { @@ -81,8 +82,8 @@ Promise.resolve({id: 'testId551' as GroupId, group_id: 551}) ); - const showStub = sinon.stub(page, 'show'); + const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl'); await element.handleCreateGroup(); - assert.isFalse(showStub.called); + assert.isFalse(setUrlStub.called); }); });
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts index bb59ccc..ed57830 100644 --- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts +++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
@@ -7,7 +7,6 @@ import '../../shared/gr-autocomplete/gr-autocomplete'; import '../../shared/gr-button/gr-button'; import '../../shared/gr-select/gr-select'; -import {page} from '../../../utils/page-wrapper-utils'; import { BranchName, GroupId, @@ -24,6 +23,8 @@ import {fireEvent} from '../../../utils/event-util'; import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper'; import {createRepoUrl} from '../../../models/views/repo'; +import {resolve} from '../../../models/dependency'; +import {navigationToken} from '../../core/gr-navigation/gr-navigation'; declare global { interface HTMLElementTagNameMap { @@ -71,6 +72,8 @@ private readonly restApiService = getAppContext().restApiService; + private readonly getNavigation = resolve(this, navigationToken); + constructor() { super(); this.query = (input: string) => this.getRepoSuggestions(input); @@ -195,8 +198,7 @@ ); if (repoRegistered.status === 201) { this.repoCreated = true; - // TODO: Use navigation service instead of `page.show()` directly. - page.show(createRepoUrl({repo: this.repoConfig.name})); + this.getNavigation().setUrl(createRepoUrl({repo: this.repoConfig.name})); } return repoRegistered; }
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts index 9b9cf29..9a67fda 100644 --- a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts +++ b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts
@@ -6,7 +6,6 @@ import '../../../test/common-test-setup'; import './gr-repo-detail-list'; import {GrRepoDetailList} from './gr-repo-detail-list'; -import {page} from '../../../utils/page-wrapper-utils'; import { addListenerForTest, mockPromise, @@ -34,6 +33,8 @@ import {GrListView} from '../../shared/gr-list-view/gr-list-view'; import {fixture, html, assert} from '@open-wc/testing'; import {RepoDetailView} from '../../../models/views/repo'; +import {testResolver} from '../../../test/common-test-setup'; +import {navigationToken} from '../../core/gr-navigation/gr-navigation'; function branchGenerator(counter: number) { return { @@ -95,7 +96,7 @@ html`<gr-repo-detail-list></gr-repo-detail-list>` ); element.detailType = RepoDetailView.BRANCHES; - sinon.stub(page, 'show'); + sinon.stub(testResolver(navigationToken), 'setUrl'); }); suite('list of repo branches', () => { @@ -2338,7 +2339,7 @@ html`<gr-repo-detail-list></gr-repo-detail-list>` ); element.detailType = RepoDetailView.TAGS; - sinon.stub(page, 'show'); + sinon.stub(testResolver(navigationToken), 'setUrl'); }); suite('list of repo tags', () => {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts index 65f5a8c..906b733 100644 --- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts +++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts
@@ -6,7 +6,6 @@ import '../../../test/common-test-setup'; import './gr-repo-list'; import {GrRepoList} from './gr-repo-list'; -import {page} from '../../../utils/page-wrapper-utils'; import { mockPromise, queryAndAssert, @@ -23,6 +22,8 @@ import {GrListView} from '../../shared/gr-list-view/gr-list-view'; import {fixture, html, assert} from '@open-wc/testing'; import {AdminChildView, AdminViewState} from '../../../models/views/admin'; +import {testResolver} from '../../../test/common-test-setup'; +import {navigationToken} from '../../core/gr-navigation/gr-navigation'; function createRepo(name: string, counter: number) { return { @@ -51,7 +52,7 @@ let repos: ProjectInfoWithName[]; setup(async () => { - sinon.stub(page, 'show'); + sinon.stub(testResolver(navigationToken), 'setUrl'); element = await fixture(html`<gr-repo-list></gr-repo-list>`); });
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 faaee0b..d2ba2c9 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
@@ -6,7 +6,6 @@ import '../gr-change-list/gr-change-list'; import '../gr-repo-header/gr-repo-header'; import '../gr-user-header/gr-user-header'; -import {page} from '../../../utils/page-wrapper-utils'; import { AccountDetailInfo, AccountId, @@ -28,6 +27,7 @@ import {resolve} from '../../../models/dependency'; import {subscribe} from '../../lit/subscription-controller'; import {userModelToken} from '../../../models/user/user-model'; +import {navigationToken} from '../../core/gr-navigation/gr-navigation'; const LIMIT_OPERATOR_PATTERN = /\blimit:(\d+)/i; @@ -81,6 +81,8 @@ private readonly getViewModel = resolve(this, searchViewModelToken); + private readonly getNavigation = resolve(this, navigationToken); + constructor() { super(); this.addEventListener('next-page', () => this.handleNextPage()); @@ -282,15 +284,13 @@ // private but used in test handleNextPage() { if (!this.nextArrow || !this.changesPerPage) return; - // TODO: Use navigation service instead of `page.show()` directly. - page.show(this.computeNavLink(1)); + this.getNavigation().setUrl(this.computeNavLink(1)); } // private but used in test handlePreviousPage() { if (!this.prevArrow || !this.changesPerPage) return; - // TODO: Use navigation service instead of `page.show()` directly. - page.show(this.computeNavLink(-1)); + this.getNavigation().setUrl(this.computeNavLink(-1)); } // private but used in test
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 f4bd8bd..decc253 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
@@ -6,7 +6,6 @@ import '../../../test/common-test-setup'; import './gr-change-list-view'; import {GrChangeListView} from './gr-change-list-view'; -import {page} from '../../../utils/page-wrapper-utils'; import {query, queryAndAssert} from '../../../test/test-utils'; import {createChange} from '../../../test/test-data-generators'; import {ChangeInfo} from '../../../api/rest-api'; @@ -14,6 +13,8 @@ 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'; +import {testResolver} from '../../../test/common-test-setup'; +import {navigationToken} from '../../core/gr-navigation/gr-navigation'; suite('gr-change-list-view tests', () => { let element: GrChangeListView; @@ -158,7 +159,7 @@ }); test('handleNextPage', async () => { - const showStub = sinon.stub(page, 'show'); + const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl'); element.changes = Array(25) .fill(0) .map(_ => createChange()); @@ -166,7 +167,7 @@ element.loading = false; await element.updateComplete; element.handleNextPage(); - assert.isFalse(showStub.called); + assert.isFalse(setUrlStub.called); element.changes = Array(25) .fill(0) @@ -174,11 +175,11 @@ element.loading = false; await element.updateComplete; element.handleNextPage(); - assert.isTrue(showStub.called); + assert.isTrue(setUrlStub.called); }); test('handlePreviousPage', async () => { - const showStub = sinon.stub(page, 'show'); + const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl'); element.offset = 0; element.changes = Array(25) .fill(0) @@ -187,11 +188,11 @@ element.loading = false; await element.updateComplete; element.handlePreviousPage(); - assert.isFalse(showStub.called); + assert.isFalse(setUrlStub.called); element.offset = 25; await element.updateComplete; element.handlePreviousPage(); - assert.isTrue(showStub.called); + assert.isTrue(setUrlStub.called); }); });
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts index 0092193..ea5e9f3 100644 --- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts +++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
@@ -6,10 +6,11 @@ import '../../../test/common-test-setup'; import './gr-documentation-search'; import {GrDocumentationSearch} from './gr-documentation-search'; -import {page} from '../../../utils/page-wrapper-utils'; import {queryAndAssert, stubRestApi} from '../../../test/test-utils'; import {DocResult} from '../../../types/common'; import {fixture, html, assert} from '@open-wc/testing'; +import {testResolver} from '../../../test/common-test-setup'; +import {navigationToken} from '../../core/gr-navigation/gr-navigation'; function documentationGenerator(counter: number) { return { @@ -31,7 +32,7 @@ let documentationSearches: DocResult[]; setup(async () => { - sinon.stub(page, 'show'); + sinon.stub(testResolver(navigationToken), 'setUrl'); element = await fixture( html`<gr-documentation-search></gr-documentation-search>` );
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts index dad802a..f5837e3 100644 --- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts +++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
@@ -7,13 +7,14 @@ import '../gr-button/gr-button'; import '../gr-icon/gr-icon'; import {encodeURL, getBaseUrl} from '../../../utils/url-util'; -import {page} from '../../../utils/page-wrapper-utils'; import {fireEvent} from '../../../utils/event-util'; import {debounce, DelayedTask} from '../../../utils/async-util'; import {sharedStyles} from '../../../styles/shared-styles'; import {LitElement, PropertyValues, css, html} from 'lit'; import {customElement, property} from 'lit/decorators.js'; import {BindValueChangeEvent} from '../../../types/events'; +import {resolve} from '../../../models/dependency'; +import {navigationToken} from '../../core/gr-navigation/gr-navigation'; const REQUEST_DEBOUNCE_INTERVAL_MS = 200; @@ -48,6 +49,8 @@ private reloadTask?: DelayedTask; + private readonly getNavigation = resolve(this, navigationToken); + override disconnectedCallback() { this.reloadTask?.cancel(); super.disconnectedCallback(); @@ -165,12 +168,12 @@ () => { if (!this.isConnected || !this.path) return; if (filter) { - // TODO: Use navigation service instead of `page.show()` directly. - page.show(`${this.path}/q/filter:${encodeURL(filter, false)}`); + this.getNavigation().setUrl( + `${this.path}/q/filter:${encodeURL(filter, false)}` + ); return; } - // TODO: Use navigation service instead of `page.show()` directly. - page.show(this.path); + this.getNavigation().setUrl(this.path); }, REQUEST_DEBOUNCE_INTERVAL_MS );
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts index bf94e8f..ecc1c25 100644 --- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts +++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts
@@ -6,10 +6,11 @@ import '../../../test/common-test-setup'; import './gr-list-view'; import {GrListView} from './gr-list-view'; -import {page} from '../../../utils/page-wrapper-utils'; import {queryAndAssert, stubBaseUrl} from '../../../test/test-utils'; import {GrButton} from '../gr-button/gr-button'; import {fixture, html, assert} from '@open-wc/testing'; +import {testResolver} from '../../../test/common-test-setup'; +import {navigationToken} from '../../core/gr-navigation/gr-navigation'; suite('gr-list-view tests', () => { let element: GrListView; @@ -84,7 +85,9 @@ let resolve: (url: string) => void; const promise = new Promise(r => (resolve = r)); element.path = '/admin/projects'; - sinon.stub(page, 'show').callsFake(r => resolve(r)); + sinon + .stub(testResolver(navigationToken), 'setUrl') + .callsFake(r => resolve(r)); element.filter = 'test'; await element.updateComplete;