Fix link target type and attribute binding
The type for the `target` attribute on various link components was
defined as a generic `string`. This is overly broad and does not enforce
the use of valid HTML values.
This change refines the type to only allow the valid values for an
anchor element's `target` attribute. It also updates the code to use
`undefined` instead of an empty string when no target is specified. This
prevents an empty `target=""` attribute from being rendered on the
element.
Additionally, several related explicit type annotations were added to
variables and function return values for passing type checks.
Release-Notes: skip
Change-Id: Ifbaa15bbb16717fab96a179e344b558c0f509cb1
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 157a341..81cb2c9 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -1367,7 +1367,7 @@
)
.slice(0, 4);
const overflowLinks = links.filter(a => !primaryLinks.includes(a));
- const overflowLinkItems = overflowLinks.map(link => {
+ const overflowLinkItems: DropdownLink[] = overflowLinks.map(link => {
return {
...link,
id: link.tooltip,
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
index 30bb7c8..6c45922 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.ts
@@ -92,7 +92,10 @@
];
// visible for testing
-export function getDocLinks(docBaseUrl: string, docLinks: MainHeaderLink[]) {
+export function getDocLinks(
+ docBaseUrl: string,
+ docLinks: MainHeaderLink[]
+): MainHeaderLink[] {
if (!docBaseUrl) return [];
return docLinks.map(link => {
return {
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.ts b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.ts
index 37bdfff..e84a2dc 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.ts
@@ -18,7 +18,11 @@
createServerInfo,
} from '../../../test/test-data-generators';
import {NavLink} from '../../../models/views/admin';
-import {ServerInfo, TopMenuItemInfo} from '../../../types/common';
+import {
+ ServerInfo,
+ TopMenuEntryInfo,
+ TopMenuItemInfo,
+} from '../../../types/common';
import {AuthType} from '../../../constants/constants';
import {assert, fixture, html} from '@open-wc/testing';
@@ -278,16 +282,14 @@
});
test('fix my menu item', () => {
- assert.deepEqual(
- [
- {url: 'https://awesometown.com/#hashyhash', name: '', target: ''},
- {url: '#/q/is:nice', name: '', target: '_blank'},
- ].map(element.createHeaderLink),
- [
- {url: 'https://awesometown.com/#hashyhash', name: '', target: ''},
- {url: '/q/is:nice', name: '', target: '_blank'},
- ]
- );
+ const menuLink: TopMenuItemInfo[] = [
+ {url: 'https://awesometown.com/#hashyhash', name: '', target: undefined},
+ {url: '#/q/is:nice', name: '', target: '_blank'},
+ ];
+ assert.deepEqual(menuLink.map(element.createHeaderLink), [
+ {url: 'https://awesometown.com/#hashyhash', name: '', target: undefined},
+ {url: '/q/is:nice', name: '', target: '_blank'},
+ ]);
});
test('user links', () => {
@@ -306,7 +308,7 @@
{
name: 'Facebook',
url: 'https://facebook.com',
- target: '',
+ target: undefined,
},
];
const adminLinks: NavLink[] = [
@@ -376,7 +378,7 @@
view: undefined,
},
];
- const topMenus = [
+ const topMenus: TopMenuEntryInfo[] = [
{
name: 'Plugins',
items: [
@@ -548,7 +550,7 @@
{
name: 'Facebook',
url: 'https://facebook.com',
- target: '',
+ target: undefined,
},
];
const topMenus = [
@@ -575,7 +577,7 @@
{
name: 'Facebook',
url: 'https://facebook.com',
- target: '',
+ target: undefined,
},
{
name: 'Manage',
@@ -594,7 +596,7 @@
view: undefined,
},
];
- const topMenus = [
+ const topMenus: TopMenuEntryInfo[] = [
{
name: 'Browse',
items: [
diff --git a/polygerrit-ui/app/elements/settings/gr-menu-editor/gr-menu-editor.ts b/polygerrit-ui/app/elements/settings/gr-menu-editor/gr-menu-editor.ts
index 6a85660..e7e86c6 100644
--- a/polygerrit-ui/app/elements/settings/gr-menu-editor/gr-menu-editor.ts
+++ b/polygerrit-ui/app/elements/settings/gr-menu-editor/gr-menu-editor.ts
@@ -250,7 +250,7 @@
this.menuItems.push({
name: this.newName,
url: this.newUrl,
- target: this.newTarget ? '_blank' : '',
+ target: this.newTarget ? '_blank' : undefined,
});
this.newName = '';
this.newUrl = '';
diff --git a/polygerrit-ui/app/models/views/admin.ts b/polygerrit-ui/app/models/views/admin.ts
index c959513..fd99123 100644
--- a/polygerrit-ui/app/models/views/admin.ts
+++ b/polygerrit-ui/app/models/views/admin.ts
@@ -43,7 +43,7 @@
viewableToAll?: boolean;
section?: string;
capability?: string;
- target?: string | null;
+ target?: '_blank' | '_parent' | '_self' | '_top' | null;
subsection?: SubsectionInterface;
children?: SubsectionInterface[];
}
@@ -150,7 +150,7 @@
// Append top-level links that are defined by plugins.
links.push(
- ...getAdminMenuLinks().map((link: MenuLink) => {
+ ...getAdminMenuLinks().map((link: MenuLink): NavLink => {
return {
url: link.url,
name: link.text,
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index f53a390..eedf50c 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -463,7 +463,7 @@
url?: string;
name?: string;
external?: boolean;
- target?: string | null;
+ target?: '_blank' | '_parent' | '_self' | '_top' | null;
download?: boolean;
id?: string;
tooltip?: string;
@@ -518,7 +518,7 @@
export interface TopMenuItemInfo {
url: string;
name: string;
- target?: string;
+ target?: '_blank' | '_parent' | '_self' | '_top' | null;
id?: string;
}
