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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
48ad897
Ability to open additional menu links in same tab (#275)
zablvit Nov 17, 2018
8610528
Add negative test case
zablvit Nov 18, 2018
3759d0a
Add helper function to create item links
zablvit Nov 19, 2018
a4967ac
Fix no-use-before-define lint error
zablvit Nov 19, 2018
79b7382
Merge branch 'master' into master
zablvit Nov 19, 2018
3d08f15
Use anchorTarget in custom menu configuration
tiffon Jan 6, 2019
bfebe3a
Merge branch 'master' into master
tiffon Jan 6, 2019
f810187
Fix typo in test case
tiffon Jan 9, 2019
3167785
Merge branch 'master' into master
tiffon Jan 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions packages/jaeger-ui/src/components/App/TopNav.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,11 @@ function CustomNavDropdown({ label, items }: ConfigMenuGroup) {
const menuItems = (
<Menu>
{items.map(item => {
const { label: itemLabel, url } = item;
return (
const { label: itemLabel, url, openInSameTab } = item;
const target = openInSameTab ? "_self" : "_blank";
return (
<Menu.Item key={itemLabel}>
<a href={url} target="_blank" rel="noopener noreferrer">
<a href={url} target={target} rel="noopener noreferrer">
{itemLabel}
</a>
</Menu.Item>
Expand Down Expand Up @@ -93,9 +94,10 @@ export function TopNavImpl(props: Props) {
);
}
const item = ((m: any): ConfigMenuItem);
const target = item.openInSameTab ? "_self" : "_blank";
return (
<Menu.Item key={item.label}>
<a href={item.url} target="_blank" rel="noopener noreferrer">
<a href={item.url} target={target} rel="noopener noreferrer">
{item.label}
</a>
</Menu.Item>
tiffon marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
8 changes: 8 additions & 0 deletions packages/jaeger-ui/src/components/App/TopNav.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ describe('<TopNav>', () => {
{
label: 'Twitter',
url: 'https://twitter.com/JaegerTracing',
openInSameTab: true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this make a difference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In renders the nested menu items test, there is assertion if this is passed further.
I tried to make a test for rendered dropdown, but I just don't know, how to do it.

},
];

Expand All @@ -39,6 +40,7 @@ describe('<TopNav>', () => {
{
label: labelGitHub,
url: githubUrl,
openInSameTab: true,
},
{
label: labelAbout,
Expand Down Expand Up @@ -87,5 +89,11 @@ 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);
tiffon marked this conversation as resolved.
Show resolved Hide resolved
});
});
});
1 change: 1 addition & 0 deletions packages/jaeger-ui/src/types/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
export type ConfigMenuItem = {
label: string,
url: string,
openInSameTab: boolean,
tiffon marked this conversation as resolved.
Show resolved Hide resolved
};

export type ConfigMenuGroup = {
Expand Down