fix(navbar): some styling + components inconsistencies

Author: gabotorresruiz

superset-frontend/src/features/home/Menu.tsx

@@ -18,9 +18,8 @@
  */
 import { useState, useEffect } from 'react';
 import { styled, css, useTheme } from '@apache-superset/core/ui';
-import { debounce } from 'lodash';
 import { getUrlParam } from 'src/utils/urlUtils';
-import { MainNav, MenuMode } from '@superset-ui/core/components/Menu';
+import { MainNav, MenuItem } from '@superset-ui/core/components/Menu';
 import { Tooltip, Grid, Row, Col, Image } from '@superset-ui/core/components';
 import { GenericLink } from 'src/components';
 import { NavLink, useLocation } from 'react-router-dom';
@@ -93,27 +92,7 @@ const StyledMainNav = styled(MainNav)`
       margin-inline-start: 0;
     }
 
-    @media (max-width: 767px) {
-      .ant-menu-item {
-        padding: 0 ${theme.sizeUnit * 6}px 0 ${theme.sizeUnit * 3}px !important;
-      }
-
-      .ant-menu > .ant-menu-item > span > a {
-        padding: 0;
-      }
-
-      &.main-nav .ant-menu-submenu-title > svg:nth-of-type(1) {
-        display: none;
-      }
-    }
-  `}
-`;
-
-const { SubMenu } = MainNav;
-
-const StyledSubMenu = styled(SubMenu)`
-  ${({ theme }) => css`
-    &.ant-menu-submenu.ant-menu-submenu-horizontal {
+    .ant-menu-submenu.ant-menu-submenu-horizontal {
       display: flex;
       align-items: center;
       height: 100%;
@@ -156,7 +135,7 @@ const StyledSubMenu = styled(SubMenu)`
       }
     }
 
-    &.ant-menu-submenu-selected.ant-menu-submenu-horizontal::after {
+    .ant-menu-submenu-selected.ant-menu-submenu-horizontal::after {
       transform: scale(1);
     }
   `}
@@ -197,6 +176,10 @@ const StyledCol = styled(Col)`
   `}
 `;
 
+const StyledImage = styled(Image)`
+  object-fit: contain;
+`;
+
 const { useBreakpoint } = Grid;
 
 export function Menu({
@@ -209,23 +192,10 @@ export function Menu({
   },
   isFrontendRoute = () => false,
 }: MenuProps) {
-  const [showMenu, setMenu] = useState<MenuMode>('horizontal');
   const screens = useBreakpoint();
   const uiConfig = useUiConfig();
   const theme = useTheme();
 
-  useEffect(() => {
-    function handleResize() {
-      if (window.innerWidth <= 767) {
-        setMenu('inline');
-      } else setMenu('horizontal');
-    }
-    handleResize();
-    const windowResize = debounce(() => handleResize(), 10);
-    window.addEventListener('resize', windowResize);
-    return () => window.removeEventListener('resize', windowResize);
-  }, []);
-
   enum Paths {
     Explore = '/explore',
     Dashboard = '/dashboard',
@@ -261,73 +231,63 @@ export function Menu({
   const standalone = getUrlParam(URL_PARAMS.standalone);
   if (standalone || uiConfig.hideNav) return <></>;
 
-  const renderSubMenu = ({
+  const buildMenuItem = ({
     label,
     childs,
     url,
-    index,
     isFrontendRoute,
-  }: MenuObjectProps) => {
+  }: MenuObjectProps): MenuItem => {
     if (url && isFrontendRoute) {
-      return (
-        <MainNav.Item key={label} role="presentation">
+      return {
+        key: label,
+        label: (
           <NavLink role="button" to={url} activeClassName="is-active">
             {label}
           </NavLink>
-        </MainNav.Item>
-      );
+        ),
+      };
     }
+
     if (url) {
-      return (
-        <MainNav.Item key={label}>
-          <Typography.Link href={url}>{label}</Typography.Link>
-        </MainNav.Item>
-      );
+      return {
+        key: label,
+        label: <Typography.Link href={url}>{label}</Typography.Link>,
+      };
     }
-    return (
-      <StyledSubMenu
-        key={label}
-        title={label}
-        popupOffset={[0, -8]}
-        icon={
-          showMenu === 'inline' ? <></> : <Icons.DownOutlined iconSize="xs" />
-        }
-      >
-        {childs?.map((child: MenuObjectChildProps | string, index1: number) => {
-          if (typeof child === 'string' && child === '-' && label !== 'Data') {
-            return <MainNav.Divider key={`$${index1}`} />;
-          }
-          if (typeof child !== 'string') {
-            return (
-              <MainNav.Item key={`${child.label}`}>
-                {child.isFrontendRoute ? (
-                  <NavLink
-                    to={child.url || ''}
-                    exact
-                    activeClassName="is-active"
-                  >
-                    {child.label}
-                  </NavLink>
-                ) : (
-                  <Typography.Link href={child.url}>
-                    {child.label}
-                  </Typography.Link>
-                )}
-              </MainNav.Item>
-            );
-          }
-          return null;
-        })}
-      </StyledSubMenu>
-    );
+
+    const childItems: MenuItem[] = [];
+    childs?.forEach((child: MenuObjectChildProps | string, index1: number) => {
+      if (typeof child === 'string' && child === '-' && label !== 'Data') {
+        childItems.push({ type: 'divider', key: `divider-${index1}` });
+      } else if (typeof child !== 'string') {
+        childItems.push({
+          key: `${child.label}`,
+          label: child.isFrontendRoute ? (
+            <NavLink to={child.url || ''} exact activeClassName="is-active">
+              {child.label}
+            </NavLink>
+          ) : (
+            <Typography.Link href={child.url}>{child.label}</Typography.Link>
+          ),
+        });
+      }
+    });
+
+    return {
+      key: label,
+      label,
+      icon: <Icons.DownOutlined iconSize="xs" />,
+      popupOffset: [0, -8],
+      children: childItems,
+    };
   };
   const renderBrand = () => {
     let link;
     if (theme.brandLogoUrl) {
       link = (
         <StyledBrandWrapper margin={theme.brandLogoMargin}>
           <StyledBrandLink href={theme.brandLogoHref}>
-            <Image
+            <StyledImage
               preview={false}
               src={theme.brandLogoUrl}
               alt={theme.brandLogoAlt || 'Apache Superset'}
@@ -342,7 +302,7 @@ export function Menu({
       // Kept as is for backwards compatibility with the old theme system / superset_config.py
       link = (
         <GenericLink className="navbar-brand" to={brand.path}>
-          <Image preview={false} src={brand.icon} alt={brand.alt} />
+          <StyledImage preview={false} src={brand.icon} alt={brand.alt} />
         </GenericLink>
       );
     } else {
@@ -352,7 +312,7 @@ export function Menu({
           href={brand.path}
           tabIndex={-1}
         >
-          <Image preview={false} src={brand.icon} alt={brand.alt} />
+          <StyledImage preview={false} src={brand.icon} alt={brand.alt} />
         </Typography.Link>
       );
     }
@@ -377,15 +337,13 @@ export function Menu({
             </StyledBrandText>
           )}
           <StyledMainNav
-            mode={showMenu}
+            mode="horizontal"
             data-test="navbar-top"
             className="main-nav"
             selectedKeys={activeTabs}
             disabledOverflow
-          >
-            {menu.map((item, index) => {
+            items={menu.map(item => {
               const props = {
-                index,
                 ...item,
                 isFrontendRoute: isFrontendRoute(item.url),
                 childs: item.childs?.map(c => {
@@ -400,9 +358,9 @@ export function Menu({
                 }),
               };
 
-              return renderSubMenu(props);
+              return buildMenuItem(props);
             })}
-          </StyledMainNav>
+          />
         </StyledCol>
         <Col md={8} xs={24}>
           <RightMenu

superset-frontend/src/features/home/RightMenu.tsx

@@ -379,6 +379,7 @@ const RightMenu = ({
               label: menu.label,
               icon: menu.icon,
               children: childItems,
+              popupOffset: [0, -8],
             });
           } else if (menu.url) {
             if (
@@ -559,6 +560,7 @@ const RightMenu = ({
         className: 'submenu-with-caret',
         icon: <Icons.DownOutlined iconSize="xs" />,
         children: buildNewDropdownItems(),
+        popupOffset: [0, -8],
       });
     }
 
@@ -576,6 +578,7 @@ const RightMenu = ({
       icon: <Icons.DownOutlined iconSize="xs" />,
       children: buildSettingsMenuItems(),
       className: 'submenu-with-caret',
+      popupOffset: [0, -8],
     });
 
     return items;
@@ -660,6 +663,8 @@ const RightMenu = ({
           display: flex;
           flex-direction: row;
           align-items: center;
+          height: 100%;
+          border-bottom: none !important;
 
           /* Remove the underline from menu items */
           .ant-menu-item:after,
@@ -668,11 +673,14 @@ const RightMenu = ({
           }
 
           .submenu-with-caret {
+            height: 100%;
             padding: 0;
             .ant-menu-submenu-title {
+              align-items: center;
               display: flex;
               gap: ${theme.sizeUnit * 2}px;
               flex-direction: row-reverse;
+              height: 100%;
             }
             &.ant-menu-submenu::after {
               inset-inline: ${theme.sizeUnit}px;

superset-frontend/src/hooks/useThemeMenuItems.tsx

@@ -138,5 +138,6 @@ export const useThemeMenuItems = ({
     icon: <Icons.DownOutlined iconSize="xs" />,
     className: 'submenu-with-caret',
     children,
+    popupOffset: [0, -8],
   };
 };