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

* Ability to open additional menu links in same tab (#275) Signed-off-by: Vitaliy Zabolotskyy <zablvit@gmail.com> * Add negative test case Signed-off-by: Vitaliy Zabolotskyy <zablvit@gmail.com> * Add helper function to create item links Signed-off-by: Vitaliy Zabolotskyy <zablvit@gmail.com> * Fix no-use-before-define lint error Signed-off-by: Vitaliy Zabolotskyy <zablvit@gmail.com> * Use anchorTarget in custom menu configuration Signed-off-by: Joe Farro <joef@uber.com> * Fix typo in test case Signed-off-by: Joe Farro <joef@uber.com>
jaegertracing · Jan 9, 2019 · d61767a · d61767a
1 parent 7a6190e
commit d61767a
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 28 deletions.
diff --git a/packages/jaeger-ui/src/components/App/TopNav.js b/packages/jaeger-ui/src/components/App/TopNav.js
@@ -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) {
+  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>
@@ -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>
           );
         })}

diff --git a/packages/jaeger-ui/src/components/App/TopNav.test.js b/packages/jaeger-ui/src/components/App/TopNav.test.js
@@ -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 = [
     {
@@ -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: {
@@ -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-menu items', () => {
+        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');
+        });
+      });
+    });
   });
 });
diff --git a/packages/jaeger-ui/src/types/config.js b/packages/jaeger-ui/src/types/config.js
@@ -17,6 +17,7 @@
 export type ConfigMenuItem = {
   label: string,
   url: string,
+  anchorTarget?: '_self' | '_blank' | '_parent' | '_top',
 };
 
 export type ConfigMenuGroup = {