Nav: Design changes in MegaMenu (#76735)

* refactor: move expand button to front

* refactor: add grid + adjust level 2

* refactor: remove grid + fix alignment for all three levels

* refactor: first iteration alignment

* refactor: styling of MegaMenuItemIcon

* refactor: remove styling of div in MegaMenuItemIcon

* refactor: styling object

* refactor: alignment of first level

* refactor: alignment of second level

* refactor: alignment of third level and active state

* feat: add connecting line for level 3

* feat: add comment

* refactor: remove unused code

* refactor: clean up styling

* refactor: clean up styling

* refactor: clean up styling

* some small tweaks

* remove unused props from getStyles

* fix dock button position and text ellipsing

* add padding to container to prevent icon button overlapping on left side

* add active overlay

* adjust font-weight, add padding to RHS

* add border-box for overlay

* adjust menu width to 300, don't use non-integer levels

* use height: 100%

* don't think we need minWidth

* make chevrons right/down

* make url required, cut down css

* move active state to container

* remove unused class

* add padding at top and bottom of menu as well

* adjust chevron size

---------

Co-authored-by: Ashley Harrison <ashley.harrison@grafana.com>

public/app/core/components/AppChrome/DockedMegaMenu/FeatureHighlight.tsx

@@ -11,10 +11,10 @@ export interface Props {
 export const FeatureHighlight = ({ children }: Props): JSX.Element => {
   const styles = useStyles2(getStyles);
   return (
-    <div>
+    <>
       {children}
       <span className={styles.highlight} />
-    </div>
+    </>
   );
 };
 

public/app/core/components/AppChrome/DockedMegaMenu/MegaMenu.tsx

@@ -13,7 +13,7 @@ import { useSelector } from 'app/types';
 import { MegaMenuItem } from './MegaMenuItem';
 import { enrichWithInteractionTracking, getActiveItem } from './utils';
 
-export const MENU_WIDTH = '350px';
+export const MENU_WIDTH = '300px';
 
 export interface Props extends DOMAttributes {
   onClose: () => void;
@@ -104,17 +104,18 @@ const getStyles = (theme: GrafanaTheme2) => ({
     },
   }),
   itemList: css({
+    boxSizing: 'border-box',
     display: 'flex',
     flexDirection: 'column',
     listStyleType: 'none',
-    minWidth: MENU_WIDTH,
+    padding: theme.spacing(1),
     [theme.breakpoints.up('md')]: {
       width: MENU_WIDTH,
     },
   }),
   dockMenuButton: css({
+    color: theme.colors.text.disabled,
     display: 'none',
-    marginRight: theme.spacing(2),
 
     [theme.breakpoints.up('md')]: {
       display: 'inline-flex',

public/app/core/components/AppChrome/DockedMegaMenu/MegaMenuItem.tsx

@@ -3,7 +3,7 @@ import React from 'react';
 import { useLocalStorage } from 'react-use';
 
 import { GrafanaTheme2, NavModelItem, toIconName } from '@grafana/data';
-import { Button, Icon, useStyles2, Text } from '@grafana/ui';
+import { useStyles2, Text, IconButton, Icon } from '@grafana/ui';
 
 import { Indent } from '../../Indent/Indent';
 
@@ -18,55 +18,63 @@ interface Props {
   level?: number;
 }
 
-// max level depth to render
 const MAX_DEPTH = 2;
 
 export function MegaMenuItem({ link, activeItem, level = 0, onClick }: Props) {
-  const styles = useStyles2(getStyles);
   const FeatureHighlightWrapper = link.highlightText ? FeatureHighlight : React.Fragment;
   const isActive = link === activeItem;
   const hasActiveChild = hasChildMatch(link, activeItem);
   const [sectionExpanded, setSectionExpanded] =
     useLocalStorage(`grafana.navigation.expanded[${link.text}]`, false) ?? Boolean(hasActiveChild);
-  const showExpandButton = level < MAX_DEPTH && (linkHasChildren(link) || link.emptyMessage);
+  const showExpandButton = level < MAX_DEPTH && Boolean(linkHasChildren(link) || link.emptyMessage);
+
+  const styles = useStyles2(getStyles);
+
+  if (!link.url) {
+    return null;
+  }
 
   return (
     <li className={styles.listItem}>
-      <div className={styles.collapsibleSectionWrapper}>
-        <MegaMenuItemText
-          isActive={isActive}
-          onClick={() => {
-            link.onClick?.();
-            onClick?.();
-          }}
-          target={link.target}
-          url={link.url}
-        >
-          <div
-            className={cx(styles.labelWrapper, {
-              [styles.isActive]: isActive,
-            })}
+      <div className={styles.menuItem}>
+        {level !== 0 && <Indent level={level === MAX_DEPTH ? level - 1 : level} spacing={3} />}
+        {level === MAX_DEPTH && <div className={styles.itemConnector} />}
+        <div className={styles.collapseButtonWrapper}>
+          {showExpandButton && (
+            <IconButton
+              aria-label={`${sectionExpanded ? 'Collapse' : 'Expand'} section ${link.text}`}
+              className={styles.collapseButton}
+              onClick={() => setSectionExpanded(!sectionExpanded)}
+              name={sectionExpanded ? 'angle-down' : 'angle-right'}
+              size="md"
+            />
+          )}
+        </div>
+        <div className={styles.collapsibleSectionWrapper}>
+          <MegaMenuItemText
+            isActive={isActive}
+            onClick={() => {
+              link.onClick?.();
+              onClick?.();
+            }}
+            target={link.target}
+            url={link.url}
           >
-            <FeatureHighlightWrapper>
-              <div className={styles.iconWrapper}>
-                {level === 0 && link.icon && <Icon name={toIconName(link.icon) ?? 'link'} size="xl" />}
-              </div>
-            </FeatureHighlightWrapper>
-            <Indent level={Math.max(0, level - 1)} spacing={2} />
-            <Text truncate>{link.text}</Text>
-          </div>
-        </MegaMenuItemText>
-        {showExpandButton && (
-          <Button
-            aria-label={`${sectionExpanded ? 'Collapse' : 'Expand'} section ${link.text}`}
-            variant="secondary"
-            fill="text"
-            className={styles.collapseButton}
-            onClick={() => setSectionExpanded(!sectionExpanded)}
-          >
-            <Icon name={sectionExpanded ? 'angle-up' : 'angle-down'} size="xl" />
-          </Button>
-        )}
+            <div
+              className={cx(styles.labelWrapper, {
+                [styles.hasActiveChild]: hasActiveChild,
+                [styles.hasIcon]: Boolean(level === 0 && link.icon),
+              })}
+            >
+              {level === 0 && link.icon && (
+                <FeatureHighlightWrapper>
+                  <Icon className={styles.icon} name={toIconName(link.icon) ?? 'link'} size="lg" />
+                </FeatureHighlightWrapper>
+              )}
+              <Text truncate>{link.text}</Text>
+            </div>
+          </MegaMenuItemText>
+        </div>
       </div>
       {showExpandButton && sectionExpanded && (
         <ul className={styles.children}>
@@ -92,57 +100,73 @@ export function MegaMenuItem({ link, activeItem, level = 0, onClick }: Props) {
 }
 
 const getStyles = (theme: GrafanaTheme2) => ({
-  children: css({
+  icon: css({
+    width: theme.spacing(3),
+  }),
+  listItem: css({
+    flex: 1,
+    maxWidth: '100%',
+  }),
+  menuItem: css({
     display: 'flex',
-    listStyleType: 'none',
-    flexDirection: 'column',
+    alignItems: 'center',
+    gap: theme.spacing(1),
+    height: theme.spacing(4),
+    position: 'relative',
+  }),
+  collapseButtonWrapper: css({
+    display: 'flex',
+    justifyContent: 'center',
+    width: theme.spacing(3),
+    flexShrink: 0,
+  }),
+  itemConnector: css({
+    position: 'relative',
+    height: '100%',
+    width: theme.spacing(1.5),
+    '&::before': {
+      borderLeft: `1px solid ${theme.colors.border.medium}`,
+      content: '""',
+      height: '100%',
+      right: 0,
+      position: 'absolute',
+      transform: 'translateX(50%)',
+    },
+  }),
+  collapseButton: css({
+    color: theme.colors.text.disabled,
+    margin: 0,
   }),
   collapsibleSectionWrapper: css({
     alignItems: 'center',
     display: 'flex',
+    flex: 1,
+    height: '100%',
+    minWidth: 0,
   }),
-  collapseButton: css({
-    color: theme.colors.text.disabled,
-    padding: theme.spacing(0, 0.5),
-    marginRight: theme.spacing(1),
+  labelWrapper: css({
+    display: 'flex',
+    alignItems: 'center',
+    gap: theme.spacing(2),
+    paddingLeft: theme.spacing(1),
+    minWidth: 0,
+  }),
+  hasIcon: css({
+    paddingLeft: theme.spacing(0),
+  }),
+  hasActiveChild: css({
+    color: theme.colors.text.primary,
+  }),
+  children: css({
+    display: 'flex',
+    listStyleType: 'none',
+    flexDirection: 'column',
   }),
   emptyMessage: css({
     color: theme.colors.text.secondary,
     fontStyle: 'italic',
     padding: theme.spacing(1, 1.5, 1, 7),
   }),
-  iconWrapper: css({
-    display: 'inline-flex',
-    alignItems: 'center',
-    justifyContent: 'center',
-  }),
-  labelWrapper: css({
-    display: 'grid',
-    fontSize: theme.typography.pxToRem(14),
-    gridAutoFlow: 'column',
-    gridTemplateColumns: `${theme.spacing(7)} auto`,
-    alignItems: 'center',
-    fontWeight: theme.typography.fontWeightMedium,
-  }),
-  listItem: css({
-    flex: 1,
-  }),
-  isActive: css({
-    color: theme.colors.text.primary,
-
-    '&::before': {
-      display: 'block',
-      content: '" "',
-      height: theme.spacing(3),
-      position: 'absolute',
-      left: theme.spacing(1),
-      top: '50%',
-      transform: 'translateY(-50%)',
-      width: theme.spacing(0.5),
-      borderRadius: theme.shape.radius.default,
-      backgroundImage: theme.colors.gradients.brandVertical,
-    },
-  }),
 });
 
 function linkHasChildren(link: NavModelItem): link is NavModelItem & { children: NavModelItem[] } {

public/app/core/components/AppChrome/DockedMegaMenu/MegaMenuItemText.tsx

@@ -10,67 +10,78 @@ export interface Props {
   isActive?: boolean;
   onClick?: () => void;
   target?: HTMLAnchorElement['target'];
-  url?: string;
+  url: string;
 }
 
 export function MegaMenuItemText({ children, isActive, onClick, target, url }: Props) {
   const theme = useTheme2();
   const styles = getStyles(theme, isActive);
+  const LinkComponent = !target && url.startsWith('/') ? Link : 'a';
 
   const linkContent = (
     <div className={styles.linkContent}>
       {children}
 
-      {target === '_blank' && (
-        <Icon data-testid="external-link-icon" name="external-link-alt" className={styles.externalLinkIcon} />
-      )}
+      {
+        // As nav links are supposed to link to internal urls this option should be used with caution
+        target === '_blank' && <Icon data-testid="external-link-icon" name="external-link-alt" />
+      }
     </div>
   );
 
-  let element = (
-    <button
+  return (
+    <LinkComponent
       data-testid={selectors.components.NavMenu.item}
-      className={cx(styles.button, styles.element)}
+      className={cx(styles.container, {
+        [styles.containerActive]: isActive,
+      })}
+      href={url}
+      target={target}
       onClick={onClick}
     >
       {linkContent}
-    </button>
+    </LinkComponent>
   );
-
-  if (url) {
-    element =
-      !target && url.startsWith('/') ? (
-        <Link
-          data-testid={selectors.components.NavMenu.item}
-          className={styles.element}
-          href={url}
-          target={target}
-          onClick={onClick}
-        >
-          {linkContent}
-        </Link>
-      ) : (
-        <a
-          data-testid={selectors.components.NavMenu.item}
-          href={url}
-          target={target}
-          className={styles.element}
-          onClick={onClick}
-        >
-          {linkContent}
-        </a>
-      );
-  }
-
-  return <div className={styles.wrapper}>{element}</div>;
 }
 
 MegaMenuItemText.displayName = 'MegaMenuItemText';
 
 const getStyles = (theme: GrafanaTheme2, isActive: Props['isActive']) => ({
-  button: css({
-    backgroundColor: 'unset',
-    borderStyle: 'unset',
+  container: css({
+    alignItems: 'center',
+    color: isActive ? theme.colors.text.primary : theme.colors.text.secondary,
+    height: '100%',
+    position: 'relative',
+    width: '100%',
+
+    '&:hover, &:focus-visible': {
+      color: theme.colors.text.primary,
+      textDecoration: 'underline',
+    },
+
+    '&:focus-visible': {
+      boxShadow: 'none',
+      outline: `2px solid ${theme.colors.primary.main}`,
+      outlineOffset: '-2px',
+      transition: 'none',
+    },
+  }),
+  containerActive: css({
+    backgroundColor: theme.colors.background.secondary,
+    borderTopRightRadius: theme.shape.radius.default,
+    borderBottomRightRadius: theme.shape.radius.default,
+    position: 'relative',
+
+    '&::before': {
+      backgroundImage: theme.colors.gradients.brandVertical,
+      borderRadius: theme.shape.radius.default,
+      content: '" "',
+      display: 'block',
+      height: '100%',
+      position: 'absolute',
+      transform: 'translateX(-50%)',
+      width: theme.spacing(0.5),
+    },
   }),
   linkContent: css({
     alignItems: 'center',
@@ -79,43 +90,4 @@ const getStyles = (theme: GrafanaTheme2, isActive: Props['isActive']) => ({
     height: '100%',
     width: '100%',
   }),
-  externalLinkIcon: css({
-    color: theme.colors.text.secondary,
-  }),
-  element: css({
-    alignItems: 'center',
-    boxSizing: 'border-box',
-    position: 'relative',
-    color: isActive ? theme.colors.text.primary : theme.colors.text.secondary,
-    padding: theme.spacing(1, 1, 1, 0),
-    width: '100%',
-    '&:hover, &:focus-visible': {
-      textDecoration: 'underline',
-      color: theme.colors.text.primary,
-    },
-    '&:focus-visible': {
-      boxShadow: 'none',
-      outline: `2px solid ${theme.colors.primary.main}`,
-      outlineOffset: '-2px',
-      transition: 'none',
-    },
-    '&::before': {
-      display: isActive ? 'block' : 'none',
-      content: '" "',
-      height: theme.spacing(3),
-      position: 'absolute',
-      left: theme.spacing(1),
-      top: '50%',
-      transform: 'translateY(-50%)',
-      width: theme.spacing(0.5),
-      borderRadius: theme.shape.radius.default,
-      backgroundImage: theme.colors.gradients.brandVertical,
-    },
-  }),
-  wrapper: css({
-    boxSizing: 'border-box',
-    position: 'relative',
-    display: 'flex',
-    width: '100%',
-  }),
 });