From ca53f5c8dadddfae4055403d9f8993c0167870c4 Mon Sep 17 00:00:00 2001
From: Ashley Harrison <ashley.harrison@grafana.com>
Date: Mon, 13 Sep 2021 16:49:27 +0100
Subject: [PATCH] Navigation: Implement active state for items in the Sidemenu
 (#39030)

* Navigation: Implement active state for items in the Sidemenu

* Navigation: Improve logic for when link is active and extract isSearchActive into a util function

* Navigation: Implement custom rule for dashboards under /d/ and fix minor bugs

* Navigation: only show first matching active state + strip query params from link urls
---
 .../sidemenu/BottomSection.test.tsx           |   2 +
 .../components/sidemenu/BottomSection.tsx     |   4 +-
 .../app/core/components/sidemenu/SideMenu.tsx |   9 ++
 .../core/components/sidemenu/SideMenuItem.tsx |  48 ++++---
 .../components/sidemenu/TopSection.test.tsx   |  13 +-
 .../core/components/sidemenu/TopSection.tsx   |   7 +-
 .../core/components/sidemenu/utils.test.ts    | 121 +++++++++++++++++-
 public/app/core/components/sidemenu/utils.ts  |  36 ++++++
 8 files changed, 218 insertions(+), 22 deletions(-)

diff --git a/public/app/core/components/sidemenu/BottomSection.test.tsx b/public/app/core/components/sidemenu/BottomSection.test.tsx
index 8cd6273c5b2..7cb4ac70e8c 100644
--- a/public/app/core/components/sidemenu/BottomSection.test.tsx
+++ b/public/app/core/components/sidemenu/BottomSection.test.tsx
@@ -9,6 +9,8 @@ import BottomSection from './BottomSection';
 
 jest.mock('./utils', () => ({
   getForcedLoginUrl: () => '/mockForcedLoginUrl',
+  isLinkActive: () => false,
+  isSearchActive: () => false,
 }));
 jest.mock('../../app_events', () => ({
   publish: jest.fn(),
diff --git a/public/app/core/components/sidemenu/BottomSection.tsx b/public/app/core/components/sidemenu/BottomSection.tsx
index 4379c31652f..f8fa6bbd879 100644
--- a/public/app/core/components/sidemenu/BottomSection.tsx
+++ b/public/app/core/components/sidemenu/BottomSection.tsx
@@ -12,7 +12,7 @@ import { OrgSwitcher } from '../OrgSwitcher';
 import { getFooterLinks } from '../Footer/Footer';
 import { HelpModal } from '../help/HelpModal';
 import SideMenuItem from './SideMenuItem';
-import { getForcedLoginUrl } from './utils';
+import { getForcedLoginUrl, isLinkActive, isSearchActive } from './utils';
 
 export default function BottomSection() {
   const theme = useTheme2();
@@ -21,6 +21,7 @@ export default function BottomSection() {
   const bottomNav = navTree.filter((item) => item.hideFromMenu);
   const isSignedIn = contextSrv.isSignedIn;
   const location = useLocation();
+  const activeItemId = bottomNav.find((item) => isLinkActive(location.pathname, item))?.id;
   const forcedLoginUrl = getForcedLoginUrl(location.pathname + location.search);
   const user = contextSrv.user;
   const [showSwitcherModal, setShowSwitcherModal] = useState(false);
@@ -76,6 +77,7 @@ export default function BottomSection() {
         return (
           <SideMenuItem
             key={`${link.url}-${index}`}
+            isActive={!isSearchActive(location) && activeItemId === link.id}
             label={link.text}
             menuItems={menuItems}
             menuSubTitle={link.subTitle}
diff --git a/public/app/core/components/sidemenu/SideMenu.tsx b/public/app/core/components/sidemenu/SideMenu.tsx
index b61afe334f8..5a2f8c16fbb 100644
--- a/public/app/core/components/sidemenu/SideMenu.tsx
+++ b/public/app/core/components/sidemenu/SideMenu.tsx
@@ -77,10 +77,19 @@ const getStyles = (theme: GrafanaTheme2) => ({
     display: none;
     min-height: ${theme.components.sidemenu.width}px;
 
+    &:focus-visible,
     &:hover {
       background-color: ${theme.colors.action.hover};
     }
 
+    &:focus-visible {
+      box-shadow: none;
+      color: ${theme.colors.text.primary};
+      outline: 2px solid ${theme.colors.primary.main};
+      outline-offset: -2px;
+      transition: none;
+    }
+
     img {
       width: ${theme.spacing(3.5)};
     }
diff --git a/public/app/core/components/sidemenu/SideMenuItem.tsx b/public/app/core/components/sidemenu/SideMenuItem.tsx
index 19eb22651d8..c4d9fd28a6a 100644
--- a/public/app/core/components/sidemenu/SideMenuItem.tsx
+++ b/public/app/core/components/sidemenu/SideMenuItem.tsx
@@ -5,6 +5,7 @@ import { Link, styleMixins, useTheme2 } from '@grafana/ui';
 import SideMenuDropDown from './SideMenuDropDown';
 
 export interface Props {
+  isActive?: boolean;
   children: ReactNode;
   label: string;
   menuItems?: NavModelItem[];
@@ -16,6 +17,7 @@ export interface Props {
 }
 
 const SideMenuItem = ({
+  isActive = false,
   children,
   label,
   menuItems = [],
@@ -26,7 +28,7 @@ const SideMenuItem = ({
   url,
 }: Props) => {
   const theme = useTheme2();
-  const styles = getStyles(theme);
+  const styles = getStyles(theme, isActive);
   let element = (
     <button className={styles.element} onClick={onClick} aria-label={label}>
       <span className={styles.icon}>{children}</span>
@@ -71,7 +73,7 @@ const SideMenuItem = ({
 
 export default SideMenuItem;
 
-const getStyles = (theme: GrafanaTheme2) => ({
+const getStyles = (theme: GrafanaTheme2, isActive: Props['isActive']) => ({
   container: css`
     position: relative;
 
@@ -85,20 +87,11 @@ const getStyles = (theme: GrafanaTheme2) => ({
     }
 
     @media ${styleMixins.mediaUp(`${theme.breakpoints.values.md}px`)} {
-      // needs to be in here to work on safari...
-      &:not(:hover) {
-        border-left: 2px solid transparent;
-      }
+      color: ${isActive ? theme.colors.text.primary : theme.colors.text.secondary};
 
       &:hover {
         background-color: ${theme.colors.action.hover};
-        border-image: ${theme.colors.gradients.brandVertical};
-        border-image-slice: 1;
-        border-style: solid;
-        border-top: 0;
-        border-right: 0;
-        border-bottom: 0;
-        border-left-width: 2px;
+        color: ${theme.colors.text.primary};
 
         .dropdown-menu {
           animation: dropdown-anim 150ms ease-in-out 100ms forwards;
@@ -106,7 +99,7 @@ const getStyles = (theme: GrafanaTheme2) => ({
           display: flex;
           // important to overlap it otherwise it can be hidden
           // again by the mouse getting outside the hover space
-          left: ${theme.components.sidemenu.width - 3}px;
+          left: ${theme.components.sidemenu.width - 1}px;
           margin: 0;
           opacity: 0;
           top: 0;
@@ -121,12 +114,33 @@ const getStyles = (theme: GrafanaTheme2) => ({
   `,
   element: css`
     background-color: transparent;
-    border: 1px solid transparent;
-    color: ${theme.colors.text.secondary};
+    border: none;
+    color: inherit;
     display: block;
     line-height: 42px;
     text-align: center;
-    width: ${theme.components.sidemenu.width - 2}px;
+    width: ${theme.components.sidemenu.width - 1}px;
+
+    &::before {
+      display: ${isActive ? 'block' : 'none'};
+      content: ' ';
+      position: absolute;
+      left: 0;
+      top: 0;
+      bottom: 0;
+      width: 4px;
+      border-radius: 2px;
+      background-image: ${theme.colors.gradients.brandVertical};
+    }
+
+    &:focus-visible {
+      background-color: ${theme.colors.action.hover};
+      box-shadow: none;
+      color: ${theme.colors.text.primary};
+      outline: 2px solid ${theme.colors.primary.main};
+      outline-offset: -2px;
+      transition: none;
+    }
 
     .sidemenu-open--xs & {
       display: none;
diff --git a/public/app/core/components/sidemenu/TopSection.test.tsx b/public/app/core/components/sidemenu/TopSection.test.tsx
index cebadab7e7a..b285c1407c2 100644
--- a/public/app/core/components/sidemenu/TopSection.test.tsx
+++ b/public/app/core/components/sidemenu/TopSection.test.tsx
@@ -1,5 +1,6 @@
 import React from 'react';
 import { render, screen } from '@testing-library/react';
+import { BrowserRouter } from 'react-router-dom';
 import TopSection from './TopSection';
 
 jest.mock('../../config', () => ({
@@ -16,13 +17,21 @@ jest.mock('../../config', () => ({
 
 describe('Render', () => {
   it('should render search when empty', () => {
-    render(<TopSection />);
+    render(
+      <BrowserRouter>
+        <TopSection />
+      </BrowserRouter>
+    );
 
     expect(screen.getByText('Search dashboards')).toBeInTheDocument();
   });
 
   it('should render items and search item', () => {
-    render(<TopSection />);
+    render(
+      <BrowserRouter>
+        <TopSection />
+      </BrowserRouter>
+    );
 
     expect(screen.getByTestId('top-section-items').children.length).toBe(3);
   });
diff --git a/public/app/core/components/sidemenu/TopSection.tsx b/public/app/core/components/sidemenu/TopSection.tsx
index ade07e381f3..3d9ed42760e 100644
--- a/public/app/core/components/sidemenu/TopSection.tsx
+++ b/public/app/core/components/sidemenu/TopSection.tsx
@@ -1,17 +1,21 @@
 import React from 'react';
+import { useLocation } from 'react-router-dom';
 import { cloneDeep } from 'lodash';
 import { css } from '@emotion/css';
 import { GrafanaTheme2, NavModelItem } from '@grafana/data';
 import { locationService } from '@grafana/runtime';
 import { Icon, IconName, styleMixins, useTheme2 } from '@grafana/ui';
 import config from '../../config';
+import { isLinkActive, isSearchActive } from './utils';
 import SideMenuItem from './SideMenuItem';
 
 const TopSection = () => {
+  const location = useLocation();
   const theme = useTheme2();
   const styles = getStyles(theme);
   const navTree: NavModelItem[] = cloneDeep(config.bootData.navTree);
   const mainLinks = navTree.filter((item) => !item.hideFromMenu);
+  const activeItemId = mainLinks.find((item) => isLinkActive(location.pathname, item))?.id;
 
   const onOpenSearch = () => {
     locationService.partial({ search: 'open' });
@@ -19,13 +23,14 @@ const TopSection = () => {
 
   return (
     <div data-testid="top-section-items" className={styles.container}>
-      <SideMenuItem label="Search dashboards" onClick={onOpenSearch}>
+      <SideMenuItem isActive={isSearchActive(location)} label="Search dashboards" onClick={onOpenSearch}>
         <Icon name="search" size="xl" />
       </SideMenuItem>
       {mainLinks.map((link, index) => {
         return (
           <SideMenuItem
             key={`${link.id}-${index}`}
+            isActive={!isSearchActive(location) && activeItemId === link.id}
             label={link.text}
             menuItems={link.children}
             target={link.target}
diff --git a/public/app/core/components/sidemenu/utils.test.ts b/public/app/core/components/sidemenu/utils.test.ts
index 860ad65b0c1..b0ac98dc2f4 100644
--- a/public/app/core/components/sidemenu/utils.test.ts
+++ b/public/app/core/components/sidemenu/utils.test.ts
@@ -1,5 +1,6 @@
+import { NavModelItem } from '@grafana/data';
 import { updateConfig } from '../../config';
-import { getForcedLoginUrl } from './utils';
+import { getForcedLoginUrl, isLinkActive, isSearchActive } from './utils';
 
 describe('getForcedLoginUrl', () => {
   it.each`
@@ -23,3 +24,121 @@ describe('getForcedLoginUrl', () => {
     }
   );
 });
+
+describe('isLinkActive', () => {
+  it('returns true if the link url matches the pathname', () => {
+    const mockPathName = '/test';
+    const mockLink: NavModelItem = {
+      text: 'Test',
+      url: '/test',
+    };
+    expect(isLinkActive(mockPathName, mockLink)).toBe(true);
+  });
+
+  it('returns true if the pathname starts with the link url', () => {
+    const mockPathName = '/test/edit';
+    const mockLink: NavModelItem = {
+      text: 'Test',
+      url: '/test',
+    };
+    expect(isLinkActive(mockPathName, mockLink)).toBe(true);
+  });
+
+  it('returns true if a child link url matches the pathname', () => {
+    const mockPathName = '/testChild2';
+    const mockLink: NavModelItem = {
+      text: 'Test',
+      url: '/test',
+      children: [
+        {
+          text: 'TestChild',
+          url: '/testChild',
+        },
+        {
+          text: 'TestChild2',
+          url: '/testChild2',
+        },
+      ],
+    };
+    expect(isLinkActive(mockPathName, mockLink)).toBe(true);
+  });
+
+  it('returns true if the pathname starts with a child link url', () => {
+    const mockPathName = '/testChild2/edit';
+    const mockLink: NavModelItem = {
+      text: 'Test',
+      url: '/test',
+      children: [
+        {
+          text: 'TestChild',
+          url: '/testChild',
+        },
+        {
+          text: 'TestChild2',
+          url: '/testChild2',
+        },
+      ],
+    };
+    expect(isLinkActive(mockPathName, mockLink)).toBe(true);
+  });
+
+  it('returns false if none of the link urls match the pathname', () => {
+    const mockPathName = '/somethingWeird';
+    const mockLink: NavModelItem = {
+      text: 'Test',
+      url: '/test',
+      children: [
+        {
+          text: 'TestChild',
+          url: '/testChild',
+        },
+        {
+          text: 'TestChild2',
+          url: '/testChild2',
+        },
+      ],
+    };
+    expect(isLinkActive(mockPathName, mockLink)).toBe(false);
+  });
+
+  it('returns false for the base route if the pathname is not an exact match', () => {
+    const mockPathName = '/foo';
+    const mockLink: NavModelItem = {
+      text: 'Test',
+      url: '/',
+      children: [
+        {
+          text: 'TestChild',
+          url: '/',
+        },
+        {
+          text: 'TestChild2',
+          url: '/testChild2',
+        },
+      ],
+    };
+    expect(isLinkActive(mockPathName, mockLink)).toBe(false);
+  });
+});
+
+describe('isSearchActive', () => {
+  it('returns true if the search query parameter is "open"', () => {
+    const mockLocation = {
+      hash: '',
+      pathname: '/',
+      search: '?search=open',
+      state: '',
+    };
+    expect(isSearchActive(mockLocation)).toBe(true);
+  });
+
+  it('returns false if the search query parameter is missing', () => {
+    const mockLocation = {
+      hash: '',
+      pathname: '/',
+      search: '',
+      state: '',
+    };
+    expect(isSearchActive(mockLocation)).toBe(false);
+  });
+});
diff --git a/public/app/core/components/sidemenu/utils.ts b/public/app/core/components/sidemenu/utils.ts
index 1ef6a823e26..97f5b6fc026 100644
--- a/public/app/core/components/sidemenu/utils.ts
+++ b/public/app/core/components/sidemenu/utils.ts
@@ -1,4 +1,6 @@
+import { NavModelItem } from '@grafana/data';
 import { getConfig } from 'app/core/config';
+import { Location } from 'history';
 
 export const getForcedLoginUrl = (url: string) => {
   const queryParams = new URLSearchParams(url.split('?')[1]);
@@ -6,3 +8,37 @@ export const getForcedLoginUrl = (url: string) => {
 
   return `${getConfig().appSubUrl}${url.split('?')[0]}?${queryParams.toString()}`;
 };
+
+export const isLinkActive = (pathname: string, link: NavModelItem) => {
+  // strip out any query params
+  const linkPathname = link.url?.split('?')[0];
+  if (linkPathname) {
+    if (linkPathname === pathname) {
+      // exact match
+      return true;
+    } else if (linkPathname !== '/' && pathname.startsWith(linkPathname)) {
+      // partial match
+      return true;
+    } else if (linkPathname === '/alerting/list' && pathname.startsWith('/alerting/notification/')) {
+      // alert channel match
+      // TODO refactor routes such that we don't need this custom logic
+      return true;
+    } else if (linkPathname === '/' && pathname.startsWith('/d/')) {
+      // dashboard match
+      // TODO refactor routes such that we don't need this custom logic
+      return true;
+    }
+  }
+
+  // child match
+  if (link.children?.some((childLink) => isLinkActive(pathname, childLink))) {
+    return true;
+  }
+
+  return false;
+};
+
+export const isSearchActive = (location: Location<unknown>) => {
+  const query = new URLSearchParams(location.search);
+  return query.get('search') === 'open';
+};