Revert "Add keyboard shortcuts for admin menu links" This reverts commit 71c83d0a186c8ff07fb09d03f6df2b3096f1505f. Reason for revert: Breaks repo branch filter when repo has "/". Release-Notes: skip Change-Id: Ie68a920bca55ed2461dd7c173a849839c5337408
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts index d3562f2..27fe620 100644 --- a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts +++ b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
@@ -16,11 +16,7 @@ import {LitElement, PropertyValues, css, html} from 'lit'; import {customElement, query, property, state} from 'lit/decorators.js'; import {assertIsDefined} from '../../../utils/common-util'; -import { - AdminChildView, - AdminViewState, - createAdminUrl, -} from '../../../models/views/admin'; +import {AdminViewState} from '../../../models/views/admin'; import {createGroupUrl} from '../../../models/views/group'; import {whenVisible} from '../../../utils/dom-util'; import {modalStyles} from '../../../styles/gr-modal-styles'; @@ -33,6 +29,8 @@ @customElement('gr-admin-group-list') export class GrAdminGroupList extends LitElement { + readonly path = '/admin/groups'; + @query('#createModal') private createModal?: HTMLDialogElement; @query('#createNewModal') private createNewModal?: GrCreateGroupDialog; @@ -89,7 +87,7 @@ .itemsPerPage=${this.groupsPerPage} .loading=${this.loading} .offset=${this.offset} - .path=${createAdminUrl({adminView: AdminChildView.GROUPS})} + .path=${this.path} @create-clicked=${() => this.handleCreateClicked()} > <table id="list" class="genericList"> @@ -169,14 +167,18 @@ * * private but used in test */ - async maybeOpenCreateModal(params?: AdminViewState) { + maybeOpenCreateModal(params?: AdminViewState) { if (params?.openCreateModal) { - await this.updateComplete; - if (!this.createModal?.open) this.createModal?.showModal(); + assertIsDefined(this.createModal, 'createModal'); + this.createModal.showModal(); } } - // private but used in test + /** + * Generates groups link (/admin/groups/<uuid>) + * + * private but used in test + */ computeGroupUrl(encodedId: string) { const groupId = decodeURIComponent(encodedId) as GroupId; return createGroupUrl({groupId});
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.ts b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.ts index e9b7ea0..1b128aa 100644 --- a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.ts +++ b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.ts
@@ -120,17 +120,17 @@ assert.equal(element.groups.slice(0, SHOWN_ITEMS_COUNT).length, 25); }); - test('maybeOpenCreateModal', async () => { + test('maybeOpenCreateModal', () => { const modalOpen = sinon.stub( queryAndAssert<HTMLDialogElement>(element, '#createModal'), 'showModal' ); - await element.maybeOpenCreateModal(); + element.maybeOpenCreateModal(); assert.isFalse(modalOpen.called); - await element.maybeOpenCreateModal(undefined); + element.maybeOpenCreateModal(undefined); assert.isFalse(modalOpen.called); value.openCreateModal = true; - await element.maybeOpenCreateModal(value); + element.maybeOpenCreateModal(value); assert.isTrue(modalOpen.called); }); });
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..4808d00 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,8 +6,9 @@ import '@polymer/iron-input/iron-input'; import '../../../styles/gr-form-styles'; import '../../../styles/shared-styles'; +import {encodeURL, getBaseUrl} from '../../../utils/url-util'; import {page} from '../../../utils/page-wrapper-utils'; -import {GroupId, GroupName} from '../../../types/common'; +import {GroupName} from '../../../types/common'; import {getAppContext} from '../../../services/app-context'; import {formStyles} from '../../../styles/gr-form-styles'; import {sharedStyles} from '../../../styles/shared-styles'; @@ -15,7 +16,6 @@ import {customElement, query, property} from 'lit/decorators.js'; import {BindValueChangeEvent} from '../../../types/events'; import {fireEvent} from '../../../utils/event-util'; -import {createGroupUrl} from '../../../models/views/group'; declare global { interface HTMLElementTagNameMap { @@ -75,6 +75,10 @@ fireEvent(this, 'has-new-group-name'); } + private computeGroupUrl(groupId: string) { + return getBaseUrl() + '/admin/groups/' + encodeURL(groupId, true); + } + override focus() { this.input.focus(); } @@ -85,9 +89,7 @@ if (groupRegistered.status !== 201) return; 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})); + page.show(this.computeGroupUrl(String(group.group_id!))); }); }); }
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..a90abbd 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,6 +7,7 @@ import '../../shared/gr-autocomplete/gr-autocomplete'; import '../../shared/gr-button/gr-button'; import '../../shared/gr-select/gr-select'; +import {encodeURL, getBaseUrl} from '../../../utils/url-util'; import {page} from '../../../utils/page-wrapper-utils'; import { BranchName, @@ -23,7 +24,6 @@ import {customElement, query, property, state} from 'lit/decorators.js'; 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'; declare global { interface HTMLElementTagNameMap { @@ -183,6 +183,10 @@ `; } + _computeRepoUrl(repoName: string) { + return getBaseUrl() + '/admin/repos/' + encodeURL(repoName, true); + } + override focus() { this.input?.focus(); } @@ -195,8 +199,7 @@ ); if (repoRegistered.status === 201) { this.repoCreated = true; - // TODO: Use navigation service instead of `page.show()` directly. - page.show(createRepoUrl({repo: this.repoConfig.name})); + page.show(this._computeRepoUrl(this.repoConfig.name)); } return repoRegistered; }
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts index 52e0b3f..389e7c4 100644 --- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts +++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
@@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ import '../gr-access-section/gr-access-section'; -import {singleDecodeURL} from '../../../utils/url-util'; +import {encodeURL, getBaseUrl, singleDecodeURL} from '../../../utils/url-util'; import {navigationToken} from '../../core/gr-navigation/gr-navigation'; import {toSortedPermissionsArray} from '../../../utils/access-util'; import { @@ -44,7 +44,6 @@ import {resolve} from '../../../models/dependency'; import {createChangeUrl} from '../../../models/views/change'; import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper'; -import {createRepoUrl, RepoDetailView} from '../../../models/views/repo'; const NOTHING_TO_SAVE = 'No changes to save.'; @@ -727,10 +726,10 @@ computeParentHref() { if (!this.inheritsFrom?.name) return ''; - return createRepoUrl({ - repo: this.inheritsFrom.name, - detail: RepoDetailView.ACCESS, - }); + return `${getBaseUrl()}/admin/repos/${encodeURL( + this.inheritsFrom.name, + true + )},access`; } private handleEditInheritFromTextChanged(e: ValueChangedEvent) {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts index 632ec4c..3b193a6 100644 --- a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts +++ b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts
@@ -11,6 +11,7 @@ import '../../shared/gr-list-view/gr-list-view'; import '../gr-create-pointer-dialog/gr-create-pointer-dialog'; import '../gr-confirm-delete-item-dialog/gr-confirm-delete-item-dialog'; +import {encodeURL} from '../../../utils/url-util'; import {GrCreatePointerDialog} from '../gr-create-pointer-dialog/gr-create-pointer-dialog'; import { BranchInfo, @@ -28,16 +29,12 @@ import {formStyles} from '../../../styles/gr-form-styles'; import {tableStyles} from '../../../styles/gr-table-styles'; import {sharedStyles} from '../../../styles/shared-styles'; -import {LitElement, PropertyValues, css, html, nothing} from 'lit'; +import {LitElement, PropertyValues, css, html} from 'lit'; import {customElement, query, property, state} from 'lit/decorators.js'; import {BindValueChangeEvent} from '../../../types/events'; import {assertIsDefined} from '../../../utils/common-util'; import {ifDefined} from 'lit/directives/if-defined.js'; -import { - createRepoUrl, - RepoDetailView, - RepoViewState, -} from '../../../models/views/repo'; +import {RepoDetailView, RepoViewState} from '../../../models/views/repo'; import {modalStyles} from '../../../styles/gr-modal-styles'; const PGP_START = '-----BEGIN PGP SIGNATURE-----'; @@ -142,7 +139,6 @@ } override render() { - if (!this.repo || !this.detailType) return nothing; return html` <gr-list-view .createNew=${this.loggedIn} @@ -151,7 +147,7 @@ .items=${this.items} .loading=${this.loading} .offset=${this.offset} - .path=${createRepoUrl({repo: this.repo, detail: this.detailType})} + .path=${this.getPath(this.repo, this.detailType)} @create-clicked=${() => { this.handleCreateClicked(); }} @@ -445,6 +441,10 @@ return Promise.reject(new Error('unknown detail type')); } + private getPath(repo?: RepoName, detailType?: RepoDetailView) { + return `/admin/repos/${encodeURL(repo ?? '', false)},${detailType}`; + } + private computeWeblink(repo: ProjectInfo | BranchInfo | TagInfo) { if (!repo.web_links) return []; const webLinks = repo.web_links;
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 ec1bb82..28fc751 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
@@ -2434,18 +2434,6 @@ }); suite('create new', () => { - setup(async () => { - stubRestApi('getRepoBranches').resolves(createBranchesList(3)); - - element.params = { - view: GerritView.REPO, - repo: 'test' as RepoName, - detail: RepoDetailView.BRANCHES, - }; - await element.paramsChanged(); - await element.updateComplete; - }); - test('handleCreateClicked called when create-click fired', () => { const handleCreateClickedStub = sinon.stub( element,
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts index 2fd7e79..c6c6efd 100644 --- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts +++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
@@ -6,23 +6,23 @@ import '../../shared/gr-dialog/gr-dialog'; import '../../shared/gr-list-view/gr-list-view'; import '../gr-create-repo-dialog/gr-create-repo-dialog'; -import {ProjectInfoWithName, WebLinkInfo} from '../../../types/common'; +import { + RepoName, + ProjectInfoWithName, + WebLinkInfo, +} from '../../../types/common'; import {GrCreateRepoDialog} from '../gr-create-repo-dialog/gr-create-repo-dialog'; import {RepoState, SHOWN_ITEMS_COUNT} from '../../../constants/constants'; import {fireTitleChange} from '../../../utils/event-util'; import {getAppContext} from '../../../services/app-context'; +import {encodeURL, getBaseUrl} from '../../../utils/url-util'; import {tableStyles} from '../../../styles/gr-table-styles'; import {sharedStyles} from '../../../styles/shared-styles'; import {LitElement, PropertyValues, css, html} from 'lit'; import {customElement, property, query, state} from 'lit/decorators.js'; -import { - AdminChildView, - AdminViewState, - createAdminUrl, -} from '../../../models/views/admin'; +import {AdminViewState} from '../../../models/views/admin'; import {createSearchUrl} from '../../../models/views/search'; import {modalStyles} from '../../../styles/gr-modal-styles'; -import {createRepoUrl} from '../../../models/views/repo'; declare global { interface HTMLElementTagNameMap { @@ -32,6 +32,8 @@ @customElement('gr-repo-list') export class GrRepoList extends LitElement { + readonly path = '/admin/repos'; + @query('#createModal') private createModal?: HTMLDialogElement; @query('#createNewModal') private createNewModal?: GrCreateRepoDialog; @@ -101,7 +103,7 @@ .items=${this.repos} .loading=${this.loading} .offset=${this.offset} - .path=${createAdminUrl({adminView: AdminChildView.REPOS})} + .path=${this.path} @create-clicked=${() => this.handleCreateClicked()} > <table id="list" class="genericList"> @@ -155,11 +157,11 @@ return html` <tr class="table"> <td class="name"> - <a href=${createRepoUrl({repo: item.name})}>${item.name}</a> + <a href=${this.computeRepoUrl(item.name)}>${item.name}</a> </td> <td class="repositoryBrowser">${this.renderWebLinks(item)}</td> <td class="changesLink"> - <a href=${createSearchUrl({repo: item.name})}>view all</a> + <a href=${this.computeChangesLink(item.name)}>view all</a> </td> <td class="readOnly"> ${item.state === RepoState.READ_ONLY ? 'Y' : ''} @@ -208,6 +210,14 @@ } } + private computeRepoUrl(name: string) { + return `${getBaseUrl()}${this.path}/${encodeURL(name, true)}`; + } + + private computeChangesLink(name: string) { + return createSearchUrl({repo: name as RepoName}); + } + private async getCreateRepoCapability() { const account = await this.restApiService.getAccount();
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..7abfce9 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
@@ -282,14 +282,12 @@ // 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)); } // 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)); }
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 bcf6937..3fff1d4 100644 --- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts +++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -60,7 +60,6 @@ RepoViewState, } from '../../../models/views/repo'; import { - createGroupUrl, GroupDetailView, GroupViewModel, GroupViewState, @@ -1078,8 +1077,7 @@ } handleGroupInfoRoute(ctx: PageContext) { - const groupId = ctx.params[0] as GroupId; - this.redirect(createGroupUrl({groupId})); + this.redirect('/admin/groups/' + encodeURIComponent(ctx.params[0])); } handleGroupSelfRedirectRoute(_: PageContext) { @@ -1163,8 +1161,6 @@ } } - // TODO: Change the route pattern to match `repo` and `detailView` - // separately, and then use `createRepoUrl()` here. this.redirect(`/admin/repos/${params}`); }
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts index eceb05e..e0933e8 100644 --- a/polygerrit-ui/app/elements/gr-app-element.ts +++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -73,7 +73,6 @@ import {createDashboardUrl} from '../models/views/dashboard'; import {userModelToken} from '../models/user/user-model'; import {modalStyles} from '../styles/gr-modal-styles'; -import {AdminChildView, createAdminUrl} from '../models/views/admin'; import {ChangeChildView, changeViewModelToken} from '../models/views/change'; interface ErrorInfo { @@ -216,16 +215,6 @@ createSearchUrl({query: 'is:watched is:open'}) ) ); - this.shortcuts.addAbstract(Shortcut.GO_TO_REPOS, () => - this.getNavigation().setUrl( - createAdminUrl({adminView: AdminChildView.REPOS}) - ) - ); - this.shortcuts.addAbstract(Shortcut.GO_TO_GROUPS, () => - this.getNavigation().setUrl( - createAdminUrl({adminView: AdminChildView.GROUPS}) - ) - ); subscribe( this,
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 449c041..fd43869 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
@@ -177,11 +177,9 @@ () => { 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)}`); return; } - // TODO: Use navigation service instead of `page.show()` directly. page.show(this.path); }, REQUEST_DEBOUNCE_INTERVAL_MS
diff --git a/polygerrit-ui/app/models/views/admin.ts b/polygerrit-ui/app/models/views/admin.ts index 3380637..2ad95a2 100644 --- a/polygerrit-ui/app/models/views/admin.ts +++ b/polygerrit-ui/app/models/views/admin.ts
@@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ import {GerritView} from '../../services/router/router-model'; -import {getBaseUrl} from '../../utils/url-util'; import {define} from '../dependency'; import {Model} from '../model'; import {ViewState} from './base'; @@ -22,17 +21,6 @@ offset?: number | string; } -export function createAdminUrl(state: Omit<AdminViewState, 'view'>) { - switch (state.adminView) { - case AdminChildView.REPOS: - return `${getBaseUrl()}/admin/repos`; - case AdminChildView.GROUPS: - return `${getBaseUrl()}/admin/groups`; - case AdminChildView.PLUGINS: - return `${getBaseUrl()}/admin/plugins`; - } -} - export const adminViewModelToken = define<AdminViewModel>('admin-view-model'); export class AdminViewModel extends Model<AdminViewState | undefined> {
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts index 9ca2213..da61c41 100644 --- a/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts +++ b/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts
@@ -35,8 +35,6 @@ GO_TO_MERGED_CHANGES = 'GO_TO_MERGED_CHANGES', GO_TO_ABANDONED_CHANGES = 'GO_TO_ABANDONED_CHANGES', GO_TO_WATCHED_CHANGES = 'GO_TO_WATCHED_CHANGES', - GO_TO_REPOS = 'GO_TO_REPOS', - GO_TO_GROUPS = 'GO_TO_GROUPS', CURSOR_NEXT_CHANGE = 'CURSOR_NEXT_CHANGE', CURSOR_PREV_CHANGE = 'CURSOR_PREV_CHANGE', @@ -169,16 +167,6 @@ {key: 'w', combo: ComboKey.G} ); describe( - Shortcut.GO_TO_REPOS, - ShortcutSection.EVERYWHERE, - 'Go to Repositories', - {key: 'r', combo: ComboKey.G} - ); - describe(Shortcut.GO_TO_GROUPS, ShortcutSection.EVERYWHERE, 'Go to Groups', { - key: 'g', - combo: ComboKey.G, - }); - describe( Shortcut.TOGGLE_CHECKBOX, ShortcutSection.ACTIONS, 'Toggle checkbox',
diff --git a/polygerrit-ui/app/utils/admin-nav-util.ts b/polygerrit-ui/app/utils/admin-nav-util.ts index 7916799..6f81d57 100644 --- a/polygerrit-ui/app/utils/admin-nav-util.ts +++ b/polygerrit-ui/app/utils/admin-nav-util.ts
@@ -12,7 +12,7 @@ import {hasOwnProperty} from './common-util'; import {GerritView} from '../services/router/router-model'; import {MenuLink} from '../api/admin'; -import {AdminChildView, createAdminUrl} from '../models/views/admin'; +import {AdminChildView} from '../models/views/admin'; import {createGroupUrl, GroupDetailView} from '../models/views/group'; import {createRepoUrl, RepoDetailView} from '../models/views/repo'; @@ -20,7 +20,7 @@ { name: 'Repositories', noBaseUrl: true, - url: createAdminUrl({adminView: AdminChildView.REPOS}), + url: '/admin/repos', view: 'gr-repo-list' as GerritView, viewableToAll: true, }, @@ -28,7 +28,7 @@ name: 'Groups', section: 'Groups', noBaseUrl: true, - url: createAdminUrl({adminView: AdminChildView.GROUPS}), + url: '/admin/groups', view: 'gr-admin-group-list' as GerritView, }, { @@ -36,7 +36,7 @@ capability: 'viewPlugins', section: 'Plugins', noBaseUrl: true, - url: createAdminUrl({adminView: AdminChildView.PLUGINS}), + url: '/admin/plugins', view: 'gr-plugin-list' as GerritView, }, ];