refactor: Use Antd Menu in Menu component (#11528)

* Menu dropdown refactored

* MenuObject refactored

* Fix unit tests

* Style menu

* Use theme variables

superset-frontend/spec/javascripts/components/Menu_spec.jsx

@@ -18,7 +18,8 @@
  */
 import React from 'react';
 import { shallow, mount } from 'enzyme';
-import { Nav, MenuItem } from 'react-bootstrap';
+import { Nav } from 'react-bootstrap';
+import { Menu as DropdownMenu } from 'src/common/components';
 import NavDropdown from 'src/components/NavDropdown';
 import { supersetTheme, ThemeProvider } from '@superset-ui/core';
 
@@ -165,7 +166,7 @@ describe('Menu', () => {
       wrappingComponentProps: { theme: supersetTheme },
     });
 
-    expect(versionedWrapper.find('.version-info div')).toHaveLength(2);
+    expect(versionedWrapper.find('.version-info span')).toHaveLength(2);
   });
 
   it('renders a NavDropdown (settings)', () => {
@@ -173,6 +174,6 @@ describe('Menu', () => {
   });
 
   it('renders MenuItems in NavDropdown (settings)', () => {
-    expect(wrapper.find(NavDropdown).find(MenuItem)).toHaveLength(6);
+    expect(wrapper.find(NavDropdown).find(DropdownMenu.Item)).toHaveLength(3);
   });
 });

superset-frontend/src/common/components/index.tsx

@@ -44,6 +44,11 @@ export const MenuItem = styled(AntdMenu.Item)`
   > a {
     text-decoration: none;
   }
+
+  &.ant-menu-item {
+    height: ${({ theme }) => theme.gridUnit * 7}px;
+    line-height: ${({ theme }) => theme.gridUnit * 7}px;
+  }
 `;
 
 export const Menu = Object.assign(AntdMenu, {

superset-frontend/src/components/Menu/Menu.tsx

@@ -18,8 +18,9 @@
  */
 import React from 'react';
 import { t, styled } from '@superset-ui/core';
-import { Nav, Navbar, NavItem, MenuItem } from 'react-bootstrap';
+import { Nav, Navbar, NavItem } from 'react-bootstrap';
 import NavDropdown from 'src/components/NavDropdown';
+import { Menu as DropdownMenu } from 'src/common/components';
 import MenuObject, {
   MenuObjectProps,
   MenuObjectChildProps,
@@ -71,7 +72,10 @@ const StyledHeader = styled.header`
   }
 
   .version-info {
-    padding: 5px 20px;
+    padding: ${({ theme }) => theme.gridUnit * 1.5}px
+      ${({ theme }) => theme.gridUnit * 4}px
+      ${({ theme }) => theme.gridUnit * 1.5}px
+      ${({ theme }) => theme.gridUnit * 7}px;
     color: ${({ theme }) => theme.colors.grayscale.base};
     font-size: ${({ theme }) => theme.typography.sizes.xs}px;
 
@@ -107,7 +111,6 @@ const StyledHeader = styled.header`
       left: 50%;
       width: 0;
       height: 3px;
-      background-color: ${({ theme }) => theme.colors.primary.base};
       opacity: 0;
       transform: translateX(-50%);
       transition: all ${({ theme }) => theme.transitionTiming}s;
@@ -127,10 +130,6 @@ const StyledHeader = styled.header`
     }
   }
 
-  .settings-divider {
-    margin-bottom: 8px;
-    border-bottom: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
-  }
   .navbar-right {
     display: flex;
     align-items: center;
@@ -140,34 +139,6 @@ const StyledHeader = styled.header`
 export function Menu({
   data: { menu, brand, navbar_right: navbarRight, settings },
 }: MenuProps) {
-  // Flatten settings
-  const flatSettings: any[] = [];
-
-  if (settings) {
-    settings.forEach((section: object, index: number) => {
-      const newSection: MenuObjectProps = {
-        ...section,
-        index,
-        isHeader: true,
-      };
-
-      flatSettings.push(newSection);
-
-      // Filter out '-'
-      if (newSection.childs) {
-        newSection.childs.forEach((child: any) => {
-          if (child !== '-') {
-            flatSettings.push(child);
-          }
-        });
-      }
-
-      if (index !== settings.length - 1) {
-        flatSettings.push('-');
-      }
-    });
-  }
-
   return (
     <StyledHeader className="top" id="main-menu">
       <Navbar inverse fluid staticTop role="navigation">
@@ -188,76 +159,56 @@ export function Menu({
           {!navbarRight.user_is_anonymous && <NewMenu />}
           {settings && settings.length > 0 && (
             <NavDropdown id="settings-dropdown" title={t('Settings')}>
-              {flatSettings.map((section, index) => {
-                if (section === '-') {
-                  return (
-                    <MenuItem
-                      key={`$${index}`}
-                      divider
-                      disabled
-                      className="settings-divider"
-                    />
-                  );
-                }
-                if (section.isHeader) {
-                  return (
-                    <MenuItem key={`${section.label}`} header>
-                      {section.label}
-                    </MenuItem>
-                  );
-                }
-
-                return (
-                  <MenuItem
+              <DropdownMenu>
+                {settings.map((section, index) => [
+                  <DropdownMenu.ItemGroup
                     key={`${section.label}`}
-                    href={section.url}
-                    eventKey={index}
+                    title={section.label}
                   >
-                    {section.label}
-                  </MenuItem>
-                );
-              })}
+                    {section.childs?.map(child => {
+                      if (typeof child !== 'string') {
+                        return (
+                          <DropdownMenu.Item key={`${child.label}`}>
+                            <a href={child.url}>{child.label}</a>
+                          </DropdownMenu.Item>
+                        );
+                      }
+                      return null;
+                    })}
+                  </DropdownMenu.ItemGroup>,
+                  index < settings.length - 1 && <DropdownMenu.Divider />,
+                ])}
 
-              {!navbarRight.user_is_anonymous && (
-                <>
-                  <MenuItem
-                    key="user-divider"
-                    divider
-                    disabled
-                    className="settings-divider"
-                  />
-                  <MenuItem key="user-section" header>
-                    {t('User')}
-                  </MenuItem>
-                  <MenuItem href={navbarRight.user_info_url}>
-                    {t('Profile')}
-                  </MenuItem>
-                  <MenuItem href={navbarRight.user_logout_url}>
-                    {t('Logout')}
-                  </MenuItem>
-                </>
-              )}
-              {(navbarRight.version_string || navbarRight.version_sha) && (
-                <>
-                  <MenuItem
-                    key="user-divider"
-                    divider
-                    disabled
-                    className="settings-divider"
-                  />
-                  <MenuItem key="about-section" header>
-                    {t('About')}
-                  </MenuItem>
-                  <li className="version-info">
+                {!navbarRight.user_is_anonymous && [
+                  <DropdownMenu.Divider key="user-divider" />,
+                  <DropdownMenu.ItemGroup key="user-section" title={t('User')}>
+                    <DropdownMenu.Item key="profile">
+                      <a href={navbarRight.user_info_url}>{t('Profile')}</a>
+                    </DropdownMenu.Item>
+                    <DropdownMenu.Item key="logout">
+                      <a href={navbarRight.user_logout_url}>{t('Logout')}</a>
+                    </DropdownMenu.Item>
+                  </DropdownMenu.ItemGroup>,
+                ]}
+                {(navbarRight.version_string || navbarRight.version_sha) && [
+                  <DropdownMenu.Divider key="user-divider" />,
+                  <DropdownMenu.ItemGroup
+                    key="about-section"
+                    title={t('About')}
+                  >
                     {navbarRight.version_string && (
-                      <div>Version: {navbarRight.version_string}</div>
+                      <li className="version-info">
+                        <span>Version: {navbarRight.version_string}</span>
+                      </li>
                     )}
                     {navbarRight.version_sha && (
-                      <div>SHA: {navbarRight.version_sha}</div>
+                      <li className="version-info">
+                        <span>SHA: {navbarRight.version_sha}</span>
+                      </li>
                     )}
-                  </li>
-                </>
-              )}
+                  </DropdownMenu.ItemGroup>,
+                ]}
+              </DropdownMenu>
             </NavDropdown>
           )}
           {navbarRight.documentation_url && (

superset-frontend/src/components/Menu/MenuObject.tsx

@@ -17,7 +17,8 @@
  * under the License.
  */
 import React from 'react';
-import { NavItem, MenuItem } from 'react-bootstrap';
+import { NavItem } from 'react-bootstrap';
+import { Menu } from 'src/common/components';
 import NavDropdown from '../NavDropdown';
 
 export interface MenuObjectChildProps {
@@ -52,24 +53,22 @@ export default function MenuObject({
   }
 
   return (
-    <NavDropdown id={`menu-dropdown-${label}`} eventKey={index} title={label}>
-      {childs?.map((child: MenuObjectChildProps | string, index1: number) => {
-        if (typeof child === 'string' && child === '-') {
-          return <MenuItem key={`$${index1}`} divider />;
-        }
-        if (typeof child !== 'string') {
-          return (
-            <MenuItem
-              key={`${child.label}`}
-              href={child.url}
-              eventKey={parseFloat(`${index}.${index1}`)}
-            >
-              &nbsp; {child.label}
-            </MenuItem>
-          );
-        }
-        return null;
-      })}
+    <NavDropdown id={`menu-dropdown-${label}`} title={label}>
+      <Menu>
+        {childs?.map((child: MenuObjectChildProps | string, index1: number) => {
+          if (typeof child === 'string' && child === '-') {
+            return <Menu.Divider key={`$${index1}`} />;
+          }
+          if (typeof child !== 'string') {
+            return (
+              <Menu.Item key={`${child.label}`}>
+                <a href={child.url}>&nbsp; {child.label}</a>
+              </Menu.Item>
+            );
+          }
+          return null;
+        })}
+      </Menu>
     </NavDropdown>
   );
 }

superset-frontend/src/components/Menu/NewMenu.tsx

@@ -18,7 +18,7 @@
  */
 import React from 'react';
 import { t, styled } from '@superset-ui/core';
-import { MenuItem } from 'react-bootstrap';
+import { Menu } from 'src/common/components';
 import NavDropdown from 'src/components/NavDropdown';
 
 const dropdownItems = [
@@ -45,11 +45,15 @@ const StyledI = styled.div`
 export default function NewMenu() {
   return (
     <NavDropdown id="new-dropdown" title={<StyledI className="fa fa-plus" />}>
-      {dropdownItems.map((menu, i) => (
-        <MenuItem key={i} href={menu.url}>
-          <i className={`fa ${menu.icon}`} /> {menu.label}
-        </MenuItem>
-      ))}
+      <Menu>
+        {dropdownItems.map((menu, i) => (
+          <Menu.Item key={i}>
+            <a href={menu.url}>
+              <i className={`fa ${menu.icon}`} /> {menu.label}
+            </a>
+          </Menu.Item>
+        ))}
+      </Menu>
     </NavDropdown>
   );
 }

superset-frontend/src/components/NavDropdown/index.tsx

@@ -57,15 +57,6 @@ const NavDropdown = styled(ReactBootstrapNavDropdown)`
     padding: ${({ theme }) => theme.gridUnit}px 0;
     top: 100%;
     border: none;
-    & li a {
-      padding: ${({ theme }) => theme.gridUnit}px
-        ${({ theme }) => theme.gridUnit * 4}px;
-      transition: all ${({ theme }) => theme.transitionTiming}s;
-      &:hover {
-        background: ${({ theme }) => theme.colors.primary.light4};
-        color: ${({ theme }) => theme.colors.grayscale.dark1};
-      }
-    }
   }
 `;