AppChrome: Add proper menu icon for menu, logo icon becomes home (#114713)

* AppChrome: Add proper menu icon for menu, logo icon becomes home

* Update

* Update

* fix merge issue

* Aligning icons

* Simplify styling and fix issues

* fixes

* style fix

* Fixed unit test

* review updates

* update

* Update

* Remove feature highlight

* fix lint

* remove unused parts

e2e-playwright/various-suite/prometheus-variable-editor.spec.ts

@@ -95,7 +95,7 @@ test.describe(
       await createNewPanelButton.click();
 
       // Close the data source picker modal
-      const closeButton = page.getByRole('button', { name: 'Close menu' });
+      const closeButton = page.getByRole('button', { name: 'Close', exact: true });
       await closeButton.click({ force: true });
 
       // Select prom data source from the data source list

packages/grafana-ui/src/components/ToolbarButton/ToolbarButton.tsx

@@ -206,7 +206,7 @@ const getStyles = (theme: GrafanaTheme2) => {
       background: 'transparent',
       border: `1px solid transparent`,
 
-      '&:hover, &:focus': {
+      '&:hover': {
         color: theme.colors.text.primary,
         background: theme.colors.action.hover,
       },

public/app/core/components/AppChrome/AppChromeService.tsx

@@ -119,7 +119,7 @@ export class AppChromeService {
   };
 
   private getUpdatedHistory(newState: AppChromeState): HistoryEntry[] {
-    const breadcrumbs = buildBreadcrumbs(newState.sectionNav.node, newState.pageNav, { text: 'Home', url: '/' }, true);
+    const breadcrumbs = buildBreadcrumbs(newState.sectionNav.node, newState.pageNav, { text: 'Home', url: '/' });
     const newPageNav = newState.pageNav || newState.sectionNav.node;
 
     let entries = store.getObject<HistoryEntry[]>(HISTORY_LOCAL_STORAGE_KEY, []);

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

@@ -1,34 +0,0 @@
-import { css } from '@emotion/css';
-import type { JSX } from 'react';
-
-import { GrafanaTheme2 } from '@grafana/data';
-import { useStyles2 } from '@grafana/ui';
-
-export interface Props {
-  children: JSX.Element;
-}
-
-export const FeatureHighlight = ({ children }: Props): JSX.Element => {
-  const styles = useStyles2(getStyles);
-  return (
-    <>
-      {children}
-      <span className={styles.highlight} />
-    </>
-  );
-};
-
-const getStyles = (theme: GrafanaTheme2) => {
-  return {
-    highlight: css({
-      backgroundColor: theme.colors.success.main,
-      borderRadius: theme.shape.radius.circle,
-      width: '6px',
-      height: '6px',
-      display: 'inline-block;',
-      position: 'absolute',
-      top: '50%',
-      transform: 'translateY(-50%)',
-    }),
-  };
-};

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

@@ -62,10 +62,6 @@ export const MegaMenu = memo(
 
     const activeItem = getActiveItem(navItems, state.sectionNav.node, location.pathname);
 
-    const handleMegaMenu = () => {
-      chrome.setMegaMenuOpen(!state.megaMenuOpen);
-    };
-
     const handleDockedMenu = () => {
       chrome.setMegaMenuDocked(!state.megaMenuDocked);
       if (state.megaMenuDocked) {
@@ -108,7 +104,7 @@ export const MegaMenu = memo(
 
     return (
       <div data-testid={selectors.components.NavMenu.Menu} ref={ref} {...restProps}>
-        <MegaMenuHeader handleDockedMenu={handleDockedMenu} handleMegaMenu={handleMegaMenu} onClose={onClose} />
+        <MegaMenuHeader handleDockedMenu={handleDockedMenu} onClose={onClose} />
         <nav className={styles.content}>
           <ScrollContainer height="100%" overflowX="hidden" showScrollIndicators>
             <>
@@ -159,7 +155,7 @@ const getStyles = (theme: GrafanaTheme2) => {
       display: 'flex',
       flexDirection: 'column',
       listStyleType: 'none',
-      padding: theme.spacing(1, 1, 2, 1),
+      padding: theme.spacing(1, 1, 2, 0.5),
       [theme.breakpoints.up('md')]: {
         width: MENU_WIDTH,
       },

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

@@ -2,15 +2,16 @@ import { css } from '@emotion/css';
 
 import { GrafanaTheme2 } from '@grafana/data';
 import { t } from '@grafana/i18n';
-import { IconButton, Stack, ToolbarButton, useTheme2 } from '@grafana/ui';
+import { IconButton, Stack, useTheme2 } from '@grafana/ui';
 import { useGrafana } from 'app/core/context/GrafanaContext';
+import { HOME_NAV_ID } from 'app/core/reducers/navModel';
+import { useSelector } from 'app/types/store';
 
-import { Branding } from '../../Branding/Branding';
+import { HomeLink } from '../../Branding/Branding';
 import { OrganizationSwitcher } from '../OrganizationSwitcher/OrganizationSwitcher';
 import { getChromeHeaderLevelHeight } from '../TopBar/useChromeHeaderHeight';
 
 export interface Props {
-  handleMegaMenu: () => void;
   handleDockedMenu: () => void;
   onClose: () => void;
 }
@@ -18,26 +19,20 @@ export interface Props {
 export const DOCK_MENU_BUTTON_ID = 'dock-menu-button';
 export const MEGA_MENU_HEADER_TOGGLE_ID = 'mega-menu-header-toggle';
 
-export function MegaMenuHeader({ handleMegaMenu, handleDockedMenu, onClose }: Props) {
+export function MegaMenuHeader({ handleDockedMenu, onClose }: Props) {
   const theme = useTheme2();
   const { chrome } = useGrafana();
   const state = chrome.useState();
-
+  const homeNav = useSelector((state) => state.navIndex)[HOME_NAV_ID];
   const styles = getStyles(theme);
 
   return (
     <div className={styles.header}>
-      <Stack alignItems="center" minWidth={0} gap={0.25}>
-        <ToolbarButton
-          narrow
-          id={MEGA_MENU_HEADER_TOGGLE_ID}
-          onClick={handleMegaMenu}
-          tooltip={t('navigation.megamenu.close', 'Close menu')}
-        >
-          <Branding.MenuLogo className={styles.img} />
-        </ToolbarButton>
+      <Stack alignItems="center" minWidth={0} gap={1}>
+        <HomeLink homeNav={homeNav} inMegaMenuOverlay={!state.megaMenuDocked} />
         <OrganizationSwitcher />
       </Stack>
+      <div className={styles.flexGrow} />
       <IconButton
         id={DOCK_MENU_BUTTON_ID}
         className={styles.dockMenuButton}
@@ -51,11 +46,10 @@ export function MegaMenuHeader({ handleMegaMenu, handleDockedMenu, onClose }: Pr
         variant="secondary"
       />
       <IconButton
-        className={styles.mobileCloseButton}
         tooltip={t('navigation.megamenu.close', 'Close menu')}
         name="times"
         onClick={onClose}
-        size="xl"
+        size="lg"
         variant="secondary"
       />
     </div>
@@ -78,18 +72,9 @@ const getStyles = (theme: GrafanaTheme2) => ({
     display: 'flex',
     gap: theme.spacing(1),
     justifyContent: 'space-between',
-    padding: theme.spacing(0, 1, 0, 0.75),
+    padding: theme.spacing(0, 1, 0, 1),
     height: getChromeHeaderLevelHeight(),
     flexShrink: 0,
   }),
-  img: css({
-    alignSelf: 'center',
-    height: theme.spacing(3),
-    width: theme.spacing(3),
-  }),
-  mobileCloseButton: css({
-    [theme.breakpoints.up('md')]: {
-      display: 'none',
-    },
-  }),
+  flexGrow: css({ flexGrow: 1 }),
 });

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

@@ -11,7 +11,6 @@ import { useGrafana } from 'app/core/context/GrafanaContext';
 
 import { Indent } from '../../Indent/Indent';
 
-import { FeatureHighlight } from './FeatureHighlight';
 import { MegaMenuItemText } from './MegaMenuItemText';
 import { hasChildMatch } from './utils';
 
@@ -31,7 +30,6 @@ export function MegaMenuItem({ link, activeItem, level = 0, onClick, onPin, isPi
   const state = chrome.useState();
   const menuIsDocked = state.megaMenuDocked;
   const location = useLocation();
-  const FeatureHighlightWrapper = link.highlightText ? FeatureHighlight : React.Fragment;
   const hasActiveChild = hasChildMatch(link, activeItem);
   const isActive = link === activeItem || (level === MAX_DEPTH && hasActiveChild);
   const [sectionExpanded, setSectionExpanded] = useLocalStorage(
@@ -81,11 +79,7 @@ export function MegaMenuItem({ link, activeItem, level = 0, onClick, onPin, isPi
 
   return (
     <li ref={item} className={styles.listItem}>
-      <div
-        className={cx(styles.menuItem, {
-          [styles.menuItemWithIcon]: Boolean(level === 0 && iconElement),
-        })}
-      >
+      <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.collapsibleSectionWrapper}>
@@ -106,7 +100,7 @@ export function MegaMenuItem({ link, activeItem, level = 0, onClick, onPin, isPi
                 [styles.labelWrapperWithIcon]: Boolean(level === 0 && iconElement),
               })}
             >
-              {level === 0 && iconElement && <FeatureHighlightWrapper>{iconElement}</FeatureHighlightWrapper>}
+              {level === 0 && iconElement}
               <Text truncate element="p">
                 {link.text}
               </Text>
@@ -179,12 +173,8 @@ const getStyles = (theme: GrafanaTheme2) => ({
     alignItems: 'center',
     gap: theme.spacing(1.5),
     height: theme.spacing(4),
-    paddingLeft: theme.spacing(0.5),
     position: 'relative',
   }),
-  menuItemWithIcon: css({
-    paddingLeft: theme.spacing(0),
-  }),
   collapseButtonWrapper: css({
     display: 'flex',
     justifyContent: 'center',
@@ -217,17 +207,17 @@ const getStyles = (theme: GrafanaTheme2) => ({
   labelWrapper: css({
     display: 'flex',
     alignItems: 'center',
-    gap: theme.spacing(0.75),
-    minWidth: 0,
+    gap: theme.spacing(1),
     paddingLeft: theme.spacing(1),
-  }),
-  labelWrapperWithIcon: css({
-    paddingLeft: theme.spacing(0.5),
-    gap: theme.spacing(0.75),
+    minWidth: 0,
   }),
   hasActiveChild: css({
     color: theme.colors.text.primary,
   }),
+  labelWrapperWithIcon: css({
+    minWidth: theme.spacing(7),
+    paddingLeft: theme.spacing(0.5),
+  }),
   children: css({
     display: 'flex',
     listStyleType: 'none',

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

@@ -35,11 +35,7 @@ export function MegaMenuItemText({ children, isActive, onClick, target, url, onP
   );
 
   return (
-    <div
-      className={cx(styles.wrapper, {
-        [styles.wrapperActive]: isActive,
-      })}
-    >
+    <div className={cx(styles.wrapper, isActive && styles.wrapperActive)}>
       <LinkComponent
         data-testid={selectors.components.NavMenu.item}
         className={styles.container}
@@ -92,6 +88,7 @@ const getStyles = (theme: GrafanaTheme2, isActive: Props['isActive']) => ({
     borderTopRightRadius: theme.shape.radius.default,
     borderBottomRightRadius: theme.shape.radius.default,
     position: 'relative',
+    color: theme.colors.text.primary,
 
     '&::before': {
       backgroundImage: theme.colors.gradients.brandVertical,
@@ -101,7 +98,8 @@ const getStyles = (theme: GrafanaTheme2, isActive: Props['isActive']) => ({
       height: '100%',
       position: 'absolute',
       transform: 'translateX(-50%)',
-      width: theme.spacing(0.5),
+      left: 0,
+      width: theme.spacing(0.25),
     },
   }),
   container: css({

public/app/core/components/AppChrome/TopBar/SingleTopBar.tsx

@@ -15,7 +15,7 @@ import { contextSrv } from 'app/core/services/context_srv';
 import { ScopesSelector } from 'app/features/scopes/selector/ScopesSelector';
 import { useSelector } from 'app/types/store';
 
-import { Branding } from '../../Branding/Branding';
+import { HomeLink } from '../../Branding/Branding';
 import { Breadcrumbs } from '../../Breadcrumbs/Breadcrumbs';
 import { buildBreadcrumbs } from '../../Breadcrumbs/utils';
 import { ExtensionToolbarItem } from '../ExtensionSidebar/ExtensionToolbarItem';
@@ -77,11 +77,11 @@ export const SingleTopBar = memo(function SingleTopBar({
               tooltip={t('navigation.megamenu.open', 'Open menu')}
             >
               <Stack gap={0} alignItems="center">
-                <Branding.MenuLogo className={styles.img} />
-                <Icon size="sm" name="angle-down" />
+                <Icon name="bars" size="xl" />
               </Stack>
             </ToolbarButton>
           )}
+          {!menuDockedAndOpen && <HomeLink homeNav={homeNav} />}
           {topLevelScopes ? <ScopesSelector /> : undefined}
           <Breadcrumbs breadcrumbs={breadcrumbs} className={styles.breadcrumbsWrapper} />
           {!showToolbarLevel && breadcrumbActions}

public/app/core/components/Branding/Branding.tsx

@@ -1,8 +1,10 @@
 import { css, cx } from '@emotion/css';
 import { FC, type JSX } from 'react';
 
-import { colorManipulator } from '@grafana/data';
-import { useTheme2 } from '@grafana/ui';
+import { colorManipulator, GrafanaTheme2, NavModelItem } from '@grafana/data';
+import { selectors } from '@grafana/e2e-selectors';
+import { reportInteraction } from '@grafana/runtime';
+import { Tooltip, useStyles2, useTheme2 } from '@grafana/ui';
 import g8LoginDarkSvg from 'img/g8_login_dark.svg';
 import g8LoginLightSvg from 'img/g8_login_light.svg';
 import grafanaIconSvg from 'img/grafana_icon.svg';
@@ -52,6 +54,56 @@ const MenuLogo: FC<BrandComponentProps> = ({ className }) => {
   return <img className={className} src={grafanaIconSvg} alt="Grafana" />;
 };
 
+/**
+ * inMegaMenuOverlay = true we just render the logo without link (used in mega menu)
+ */
+export function HomeLink({ homeNav, inMegaMenuOverlay }: { homeNav?: NavModelItem; inMegaMenuOverlay?: boolean }) {
+  const styles = useStyles2(homeLinkStyles);
+
+  const onHomeClicked = () => {
+    reportInteraction('grafana_home_clicked');
+  };
+
+  if (inMegaMenuOverlay) {
+    return (
+      <div className={styles.homeLink}>
+        <Branding.MenuLogo />
+      </div>
+    );
+  }
+
+  return (
+    <Tooltip placement="bottom" content={homeNav?.text || 'Home'}>
+      <a
+        onClick={onHomeClicked}
+        data-testid={selectors.components.Breadcrumbs.breadcrumb('Home')}
+        className={styles.homeLink}
+        title={homeNav?.text || 'Home'}
+        href={homeNav?.url}
+      >
+        <Branding.MenuLogo />
+      </a>
+    </Tooltip>
+  );
+}
+
+function homeLinkStyles(theme: GrafanaTheme2) {
+  return {
+    homeLink: css({
+      display: 'flex',
+      alignItems: 'center',
+      justifyContent: 'center',
+      height: theme.spacing(3),
+      width: theme.spacing(3),
+      margin: theme.spacing(0, 0.5),
+      img: {
+        maxHeight: '100%',
+        maxWidth: '100%',
+      },
+    }),
+  };
+}
+
 const LoginBoxBackground = () => {
   const theme = useTheme2();
   return css({

public/app/core/components/Breadcrumbs/utils.test.ts

@@ -91,10 +91,7 @@ describe('breadcrumb utils', () => {
           url: '/my-parent-section',
         },
       };
-      expect(buildBreadcrumbs(sectionNav, pageNav, mockHomeNav)).toEqual([
-        { text: 'Home', href: '/home' },
-        { text: 'My page', href: '/my-page' },
-      ]);
+      expect(buildBreadcrumbs(sectionNav, pageNav, mockHomeNav)).toEqual([{ text: 'My page', href: '/my-page' }]);
     });
 
     it('matches the home nav ignoring query parameters', () => {
@@ -114,10 +111,7 @@ describe('breadcrumb utils', () => {
           url: '/my-parent-section',
         },
       };
-      expect(buildBreadcrumbs(sectionNav, pageNav, mockHomeNav)).toEqual([
-        { text: 'Home', href: '/home?orgId=1' },
-        { text: 'My page', href: '/my-page' },
-      ]);
+      expect(buildBreadcrumbs(sectionNav, pageNav, mockHomeNav)).toEqual([{ text: 'My page', href: '/my-page' }]);
     });
 
     it('does ignore duplicates', () => {

public/app/core/components/Breadcrumbs/utils.ts

@@ -2,12 +2,7 @@ import { NavModelItem } from '@grafana/data';
 
 import { Breadcrumb } from './types';
 
-export function buildBreadcrumbs(
-  sectionNav: NavModelItem,
-  pageNav?: NavModelItem,
-  homeNav?: NavModelItem,
-  skipHome?: boolean
-) {
+export function buildBreadcrumbs(sectionNav: NavModelItem, pageNav?: NavModelItem, homeNav?: NavModelItem) {
   const crumbs: Breadcrumb[] = [];
   let foundHome = false;
   let lastPath: string | undefined = undefined;
@@ -27,9 +22,6 @@ export function buildBreadcrumbs(
 
     // Check if we found home/root if if so return early
     if (homeNav && urlToMatch === homeNav.url) {
-      if (!skipHome) {
-        crumbs.unshift({ text: homeNav.text, href: node.url ?? '' });
-      }
       foundHome = true;
       return;
     }

public/app/core/components/Page/Page.test.tsx

@@ -21,6 +21,7 @@ const setup = (props: Partial<PageProps>) => {
     {
       id: HOME_NAV_ID,
       text: 'Home',
+      url: '/',
     },
     {
       text: 'Section name',

public/app/core/components/Page/usePageTitle.ts

@@ -10,13 +10,17 @@ import { buildBreadcrumbs } from '../Breadcrumbs/utils';
 export function usePageTitle(navModel?: NavModel, pageNav?: NavModelItem) {
   const homeNav = useSelector((state) => state.navIndex)?.[HOME_NAV_ID];
   useEffect(() => {
-    const sectionNav = (navModel?.node !== navModel?.main ? navModel?.node : navModel?.main) ?? { text: 'Grafana' };
+    const sectionNav = (navModel?.node !== navModel?.main ? navModel?.node : navModel?.main) ?? {
+      text: Branding.AppTitle,
+    };
+
     const parts: string[] = buildBreadcrumbs(sectionNav, pageNav, homeNav)
       .map((crumb) => crumb.text)
       .reverse();
 
-    // Override `Home` with the custom brand title
-    parts[parts.length - 1] = Branding.AppTitle;
+    if (parts[parts.length - 1] !== Branding.AppTitle) {
+      parts.push(Branding.AppTitle);
+    }
 
     document.title = parts.join(' - ');
   }, [homeNav, navModel, pageNav]);

public/app/features/dashboard/containers/DashboardPage.test.tsx

@@ -85,7 +85,7 @@ const mockCleanUpDashboardAndVariables = jest.fn();
 function setup(propOverrides?: Partial<Props>) {
   config.bootData.navTree = [
     { text: 'Dashboards', id: 'dashboards/browse' },
-    { text: 'Home', id: HOME_NAV_ID },
+    { text: 'Home', id: HOME_NAV_ID, url: '/' },
     {
       text: 'Help',
       id: 'help',
@@ -101,9 +101,9 @@ function setup(propOverrides?: Partial<Props>) {
       'dashboards/browse': {
         text: 'Dashboards',
         id: 'dashboards/browse',
-        parentItem: { text: 'Home', id: HOME_NAV_ID },
+        parentItem: { text: 'Home', id: HOME_NAV_ID, url: '/' },
       },
-      [HOME_NAV_ID]: { text: 'Home', id: HOME_NAV_ID },
+      [HOME_NAV_ID]: { text: 'Home', id: HOME_NAV_ID, url: '/' },
     },
     initPhase: DashboardInitPhase.NotStarted,
     initError: null,