From 15111db6c5707397dca878e63370eee6425c5a9e Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Wed, 4 Nov 2020 01:19:15 +0100 Subject: [PATCH] refactor: Use Antd Menu in Menu component (#11528) * Menu dropdown refactored * MenuObject refactored * Fix unit tests * Style menu * Use theme variables --- .../spec/javascripts/components/Menu_spec.jsx | 7 +- .../src/common/components/index.tsx | 5 + .../src/components/Menu/Menu.tsx | 147 ++++++------------ .../src/components/Menu/MenuObject.tsx | 37 +++-- .../src/components/Menu/NewMenu.tsx | 16 +- .../src/components/NavDropdown/index.tsx | 9 -- 6 files changed, 86 insertions(+), 135 deletions(-) diff --git a/superset-frontend/spec/javascripts/components/Menu_spec.jsx b/superset-frontend/spec/javascripts/components/Menu_spec.jsx index 15c9255416..4e4fbe8b6a 100644 --- a/superset-frontend/spec/javascripts/components/Menu_spec.jsx +++ b/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); }); }); diff --git a/superset-frontend/src/common/components/index.tsx b/superset-frontend/src/common/components/index.tsx index b6d61b60d7..2e6b90d9e3 100644 --- a/superset-frontend/src/common/components/index.tsx +++ b/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, { diff --git a/superset-frontend/src/components/Menu/Menu.tsx b/superset-frontend/src/components/Menu/Menu.tsx index 0497726265..90b2e577f3 100644 --- a/superset-frontend/src/components/Menu/Menu.tsx +++ b/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 ( @@ -188,76 +159,56 @@ export function Menu({ {!navbarRight.user_is_anonymous && } {settings && settings.length > 0 && ( - {flatSettings.map((section, index) => { - if (section === '-') { - return ( - - ); - } - if (section.isHeader) { - return ( - - {section.label} - - ); - } - - return ( - + {settings.map((section, index) => [ + - {section.label} - - ); - })} + {section.childs?.map(child => { + if (typeof child !== 'string') { + return ( + + {child.label} + + ); + } + return null; + })} + , + index < settings.length - 1 && , + ])} - {!navbarRight.user_is_anonymous && ( - <> - - - {t('User')} - - - {t('Profile')} - - - {t('Logout')} - - - )} - {(navbarRight.version_string || navbarRight.version_sha) && ( - <> - - - {t('About')} - -
  • + {!navbarRight.user_is_anonymous && [ + , + + + {t('Profile')} + + + {t('Logout')} + + , + ]} + {(navbarRight.version_string || navbarRight.version_sha) && [ + , + {navbarRight.version_string && ( -
    Version: {navbarRight.version_string}
    +
  • + Version: {navbarRight.version_string} +
    • )} {navbarRight.version_sha && ( -
    SHA: {navbarRight.version_sha}
    +
  • + SHA: {navbarRight.version_sha} +
    • )} - - - )} + , + ]} +     )} {navbarRight.documentation_url && ( diff --git a/superset-frontend/src/components/Menu/MenuObject.tsx b/superset-frontend/src/components/Menu/MenuObject.tsx index bf66a401ae..1fbe297b3e 100644 --- a/superset-frontend/src/components/Menu/MenuObject.tsx +++ b/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 ( - - {childs?.map((child: MenuObjectChildProps | string, index1: number) => { - if (typeof child === 'string' && child === '-') { - return ; - } - if (typeof child !== 'string') { - return ( - -   {child.label} - - ); - } - return null; - })} + + + {childs?.map((child: MenuObjectChildProps | string, index1: number) => { + if (typeof child === 'string' && child === '-') { + return ; + } + if (typeof child !== 'string') { + return ( + +   {child.label} + + ); + } + return null; + })} + ); } diff --git a/superset-frontend/src/components/Menu/NewMenu.tsx b/superset-frontend/src/components/Menu/NewMenu.tsx index 1c57c0d601..d43cd61f9c 100644 --- a/superset-frontend/src/components/Menu/NewMenu.tsx +++ b/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 ( }> - {dropdownItems.map((menu, i) => ( - - {menu.label} - - ))} + + {dropdownItems.map((menu, i) => ( + + + {menu.label} + + + ))} + ); } diff --git a/superset-frontend/src/components/NavDropdown/index.tsx b/superset-frontend/src/components/NavDropdown/index.tsx index 2b5d82aabe..74d473c478 100644 --- a/superset-frontend/src/components/NavDropdown/index.tsx +++ b/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}; - } - } } `;