Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to open additional menu links in same tab (Resolves #275) #278

Merged
merged 9 commits into from
Jan 9, 2019
43 changes: 17 additions & 26 deletions packages/jaeger-ui/src/components/App/TopNav.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,19 @@ if (getConfigValue('dependencies.menuEnabled')) {
});
}

function CustomNavDropdown({ label, items }: ConfigMenuGroup) {
const menuItems = (
<Menu>
{items.map(item => {
const { label: itemLabel, url } = item;
return (
<Menu.Item key={itemLabel}>
<a href={url} target="_blank" rel="noopener noreferrer">
{itemLabel}
</a>
</Menu.Item>
);
})}
</Menu>
function getItemLink(item: ConfigMenuItem) {
tiffon marked this conversation as resolved.
Show resolved Hide resolved
const { label, anchorTarget, url } = item;
return (
<Menu.Item key={label}>
<a href={url} target={anchorTarget || '_blank'} rel="noopener noreferrer">
{label}
</a>
</Menu.Item>
);
}

function CustomNavDropdown({ label, items }: ConfigMenuGroup) {
const menuItems = <Menu>{items.map(getItemLink)}</Menu>;
return (
<Dropdown overlay={menuItems} placement="bottomRight">
<a>
Expand All @@ -84,20 +82,13 @@ export function TopNavImpl(props: Props) {
<div>
<Menu theme="dark" mode="horizontal" selectable={false} className="ub-right" selectedKeys={[pathname]}>
{menuItems.map(m => {
if (m.items != null) {
const group = ((m: any): ConfigMenuGroup);
return (
<Menu.Item key={group.label}>
<CustomNavDropdown key={group.label} {...group} />
</Menu.Item>
);
if (!m.items) {
return getItemLink(m);
}
const item = ((m: any): ConfigMenuItem);
const group = ((m: any): ConfigMenuGroup);
return (
<Menu.Item key={item.label}>
<a href={item.url} target="_blank" rel="noopener noreferrer">
{item.label}
</a>
<Menu.Item key={group.label}>
<CustomNavDropdown key={group.label} {...group} />
</Menu.Item>
tiffon marked this conversation as resolved.
Show resolved Hide resolved
);
})}
Expand Down
40 changes: 38 additions & 2 deletions packages/jaeger-ui/src/components/App/TopNav.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { TopNavImpl as TopNav } from './TopNav';
describe('<TopNav>', () => {
const labelGitHub = 'GitHub';
const githubUrl = 'https://github.com/uber/jaeger';
const blogUrl = 'https://medium.com/jaegertracing/';
const labelAbout = 'About Jaeger';
const dropdownItems = [
{
Expand All @@ -30,20 +31,28 @@ describe('<TopNav>', () => {
{
label: 'Twitter',
url: 'https://twitter.com/JaegerTracing',
anchorTarget: '_self',
},
];

const configMenuGroup = {
label: labelAbout,
items: dropdownItems,
};

const defaultProps = {
config: {
menu: [
{
label: labelGitHub,
url: githubUrl,
anchorTarget: '_self',
},
{
label: labelAbout,
items: dropdownItems,
label: 'Blog',
url: blogUrl,
},
configMenuGroup,
],
},
router: {
Expand Down Expand Up @@ -87,5 +96,32 @@ describe('<TopNav>', () => {
expect(item.prop('label')).toBe(labelAbout);
expect(item.prop('items')).toBe(dropdownItems);
});

it('adds target=_self to top-level item', () => {
const item = wrapper.find(`[href="${githubUrl}"]`);
expect(item.length).toBe(1);
expect(item.find(`[target="_self"]`).length).toBe(1);
});

it('sets target=_blank by default', () => {
const item = wrapper.find(`[href="${blogUrl}"]`);
expect(item.length).toBe(1);
expect(item.find(`[target="_blank"]`).length).toBe(1);
});

describe('<CustomNavDropdown>', () => {
beforeEach(() => {
wrapper = shallow(<TopNav.CustomNavDropdown {...configMenuGroup} />);
});

it('renders sub-memu items', () => {
tiffon marked this conversation as resolved.
Show resolved Hide resolved
const subMenu = shallow(wrapper.find('Dropdown').props().overlay);
dropdownItems.forEach(itemConfig => {
const item = subMenu.find(`[href="${itemConfig.url}"]`);
expect(item.length).toBe(1);
expect(item.prop('target')).toBe(itemConfig.anchorTarget || '_blank');
});
});
});
});
});
1 change: 1 addition & 0 deletions packages/jaeger-ui/src/types/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
export type ConfigMenuItem = {
label: string,
url: string,
anchorTarget?: '_self' | '_blank' | '_parent' | '_top',
};

export type ConfigMenuGroup = {
Expand Down