Sidemenu: Refactor BottomNavLinks to use SideMenuDropDown (#38598)

* Sidemenu: Attempt to refactor BottomNavLinks to use SideMenuDropDown * BottomNavLinks: Rewrite existing enzyme tests in RTL * BottomNavLinks: Use object spreading instead of slicing
2025-02-25 18:55:37 -06:00 · 2021-08-31 10:37:51 +01:00 · 2021-08-31 10:37:51 +01:00 · 7a242afcd6
commit 7a242afcd6
parent e47a60f511
10 changed files with 169 additions and 317 deletions
--- a/packages/grafana-data/src/types/navModel.ts
+++ b/packages/grafana-data/src/types/navModel.ts
@ -14,6 +14,7 @@ export interface NavModelItem {
  target?: string;
  parentItem?: NavModelItem;
  showOrgSwitcher?: boolean;
+  onClick?: () => void;
 }

 /**
--- a/pkg/api/index.go
+++ b/pkg/api/index.go
@ -35,7 +35,7 @@ func (hs *HTTPServer) getProfileNode(c *models.ReqContext) *dtos.NavLink {
 	if setting.AddChangePasswordLink() {
 		children = append(children, &dtos.NavLink{
 			Text: "Change password", Id: "change-password", Url: hs.Cfg.AppSubURL + "/profile/password",
-			Icon: "lock", HideFromMenu: true,
+			Icon: "lock",
 		})
 	}

--- a/public/app/core/components/sidemenu/BottomNavLinks.test.tsx
+++ b/public/app/core/components/sidemenu/BottomNavLinks.test.tsx
@ -1,101 +1,115 @@
 import React from 'react';
-import { shallow } from 'enzyme';
-import BottomNavLinks from './BottomNavLinks';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { BrowserRouter } from 'react-router-dom';
 import appEvents from '../../app_events';
 import { ShowModalReactEvent } from '../../../types/events';
 import { HelpModal } from '../help/HelpModal';
+import BottomNavLinks from './BottomNavLinks';

 jest.mock('../../app_events', () => ({
  publish: jest.fn(),
 }));

-const setup = (propOverrides?: object) => {
-  const props = Object.assign(
-    {
-      link: {
-        text: 'Hello',
-      },
-      user: {
-        id: 1,
-        isGrafanaAdmin: false,
-        isSignedIn: false,
-        orgCount: 2,
-        orgRole: '',
-        orgId: 1,
-        login: 'hello',
-        orgName: 'Grafana',
-        timezone: 'UTC',
-        helpFlags1: 1,
-        lightTheme: false,
-        hasEditPermissionInFolders: false,
-      },
-    },
-    propOverrides
-  );
-  return shallow(<BottomNavLinks {...props} />);
-};
+describe('BottomNavLinks', () => {
+  const mockUser = {
+    id: 1,
+    isGrafanaAdmin: false,
+    isSignedIn: false,
+    orgCount: 2,
+    orgRole: '',
+    orgId: 1,
+    login: 'hello',
+    orgName: 'mockOrganization',
+    timezone: 'UTC',
+    helpFlags1: 1,
+    lightTheme: false,
+    hasEditPermissionInFolders: false,
+  };

-describe('Render', () => {
-  it('should render component', () => {
-    const wrapper = setup();
+  it('renders the link text', () => {
+    const mockLink = {
+      text: 'Hello',
+    };

-    expect(wrapper).toMatchSnapshot();
+    render(
+      <BrowserRouter>
+        <BottomNavLinks link={mockLink} user={mockUser} />
+      </BrowserRouter>
+    );
+    const linkText = screen.getByText(mockLink.text);
+    expect(linkText).toBeInTheDocument();
  });

-  it('should render organization switcher', () => {
-    const wrapper = setup({
-      link: {
-        showOrgSwitcher: true,
-      },
-    });
+  it('attaches the link url to the text if provided', () => {
+    const mockLink = {
+      text: 'Hello',
+      url: '/route',
+    };

-    wrapper.find('.sidemenu-org-switcher a').simulate('click');
-    expect(wrapper).toMatchSnapshot();
+    render(
+      <BrowserRouter>
+        <BottomNavLinks link={mockLink} user={mockUser} />
+      </BrowserRouter>
+    );
+    const link = screen.getByRole('link', { name: mockLink.text });
+    expect(link).toBeInTheDocument();
+    expect(link).toHaveAttribute('href', mockLink.url);
  });

-  it('should render subtitle', () => {
-    const wrapper = setup({
-      link: {
-        subTitle: 'subtitle',
-      },
-    });
+  it('creates the correct children for the help link', () => {
+    const mockLink = {
+      id: 'help',
+      text: 'Hello',
+    };

-    expect(wrapper).toMatchSnapshot();
+    render(
+      <BrowserRouter>
+        <BottomNavLinks link={mockLink} user={mockUser} />
+      </BrowserRouter>
+    );
+    const documentation = screen.getByRole('link', { name: 'Documentation' });
+    const support = screen.getByRole('link', { name: 'Support' });
+    const community = screen.getByRole('link', { name: 'Community' });
+    const keyboardShortcuts = screen.getByText('Keyboard shortcuts');
+
+    expect(documentation).toBeInTheDocument();
+    expect(support).toBeInTheDocument();
+    expect(community).toBeInTheDocument();
+    expect(keyboardShortcuts).toBeInTheDocument();
  });

-  it('should render children', () => {
-    const wrapper = setup({
-      link: {
-        children: [
-          {
-            id: '1',
-          },
-          {
-            id: '2',
-          },
-          {
-            id: '3',
-          },
-          {
-            id: '4',
-            hideFromMenu: true,
-          },
-        ],
-      },
-    });
+  it('clicking the keyboard shortcuts button shows the modal', () => {
+    const mockLink = {
+      id: 'help',
+      text: 'Hello',
+    };

-    expect(wrapper).toMatchSnapshot();
-  });
-});
-
-describe('Functions', () => {
-  describe('item clicked', () => {
-    const wrapper = setup();
-    it('should emit show modal event if url matches shortcut', () => {
-      const instance = wrapper.instance() as BottomNavLinks;
-      instance.onOpenShortcuts();
-
-      expect(appEvents.publish).toHaveBeenCalledWith(new ShowModalReactEvent({ component: HelpModal }));
-    });
+    render(
+      <BrowserRouter>
+        <BottomNavLinks link={mockLink} user={mockUser} />
+      </BrowserRouter>
+    );
+    const keyboardShortcuts = screen.getByText('Keyboard shortcuts');
+    expect(keyboardShortcuts).toBeInTheDocument();
+    userEvent.click(keyboardShortcuts);
+    expect(appEvents.publish).toHaveBeenCalledWith(new ShowModalReactEvent({ component: HelpModal }));
+  });
+
+  it('shows the current organization and organization switcher if showOrgSwitcher is true', () => {
+    const mockLink = {
+      showOrgSwitcher: true,
+      text: 'Hello',
+    };
+
+    render(
+      <BrowserRouter>
+        <BottomNavLinks link={mockLink} user={mockUser} />
+      </BrowserRouter>
+    );
+    const currentOrg = screen.getByText(new RegExp(mockUser.orgName, 'i'));
+    const orgSwitcher = screen.getByText('Switch organization');
+    expect(currentOrg).toBeInTheDocument();
+    expect(orgSwitcher).toBeInTheDocument();
  });
 });
--- a/public/app/core/components/sidemenu/BottomNavLinks.tsx
+++ b/public/app/core/components/sidemenu/BottomNavLinks.tsx
@ -1,5 +1,4 @@
 import React, { PureComponent } from 'react';
-import { css } from '@emotion/css';
 import appEvents from '../../app_events';
 import { User } from '../../services/context_srv';
 import { NavModelItem } from '@grafana/data';
@ -8,6 +7,7 @@ import { OrgSwitcher } from '../OrgSwitcher';
 import { getFooterLinks } from '../Footer/Footer';
 import { ShowModalReactEvent } from '../../../types/events';
 import { HelpModal } from '../help/HelpModal';
+import SideMenuDropDown from './SideMenuDropDown';

 export interface Props {
  link: NavModelItem;
@ -36,14 +36,29 @@ export default class BottomNavLinks extends PureComponent<Props, State> {
  render() {
    const { link, user } = this.props;
    const { showSwitcherModal } = this.state;
-    const subMenuIconClassName = css`
-      margin-right: 8px;
-    `;

    let children = link.children || [];

    if (link.id === 'help') {
-      children = getFooterLinks();
+      children = [
+        ...getFooterLinks(),
+        {
+          text: 'Keyboard shortcuts',
+          icon: 'keyboard',
+          onClick: this.onOpenShortcuts,
+        },
+      ];
+    }
+
+    if (link.showOrgSwitcher) {
+      children = [
+        ...children,
+        {
+          text: 'Switch organization',
+          icon: 'arrow-random',
+          onClick: this.toggleSwitcherModal,
+        },
+      ];
    }

    return (
@ -54,52 +69,14 @@ export default class BottomNavLinks extends PureComponent<Props, State> {
            {link.img && <img src={link.img} alt="Profile picture" />}
          </span>
        </Link>
-        <ul className="dropdown-menu dropdown-menu--sidemenu" role="menu">
-          {link.subTitle && (
-            <li className="sidemenu-subtitle">
-              <span className="sidemenu-item-text">{link.subTitle}</span>
-            </li>
-          )}
-          {link.showOrgSwitcher && (
-            <li className="sidemenu-org-switcher">
-              <a onClick={this.toggleSwitcherModal}>
-                <div>
-                  <div className="sidemenu-org-switcher__org-current">Current Org.:</div>
-                  <div className="sidemenu-org-switcher__org-name">{user.orgName}</div>
-                </div>
-                <div className="sidemenu-org-switcher__switch">
-                  <Icon name="arrow-random" className={subMenuIconClassName} />
-                  Switch
-                </div>
-              </a>
-            </li>
-          )}
-
-          {showSwitcherModal && <OrgSwitcher onDismiss={this.toggleSwitcherModal} />}
-
-          {children.map((child, index) => {
-            return (
-              <li key={`${child.text}-${index}`}>
-                <a href={child.url} target={child.target} rel="noopener">
-                  {child.icon && <Icon name={child.icon as IconName} className={subMenuIconClassName} />}
-                  {child.text}
-                </a>
-              </li>
-            );
-          })}
-
-          {link.id === 'help' && (
-            <li key="keyboard-shortcuts">
-              <a onClick={() => this.onOpenShortcuts()}>
-                <Icon name="keyboard" className={subMenuIconClassName} /> Keyboard shortcuts
-              </a>
-            </li>
-          )}
-
-          <li className="side-menu-header">
-            <span className="sidemenu-item-text">{link.text}</span>
-          </li>
-        </ul>
+        <SideMenuDropDown
+          headerText={link.text}
+          headerUrl={link.url}
+          items={children}
+          reverseDirection
+          subtitleText={link.showOrgSwitcher ? `Current Org.: ${user.orgName}` : link.subTitle}
+        />
+        {showSwitcherModal && <OrgSwitcher onDismiss={this.toggleSwitcherModal} />}
      </div>
    );
  }
--- a/public/app/core/components/sidemenu/DropDownChild.test.tsx
+++ b/public/app/core/components/sidemenu/DropDownChild.test.tsx
@ -14,7 +14,7 @@ describe('DropDownChild', () => {
    expect(text).toBeInTheDocument();
  });

-  it('attaches the link to the text if provided', () => {
+  it('attaches the url to the text if provided', () => {
    render(
      <BrowserRouter>
        <DropDownChild text={mockText} url={mockUrl} />
@ -22,6 +22,7 @@ describe('DropDownChild', () => {
    );
    const link = screen.getByRole('link', { name: mockText });
    expect(link).toBeInTheDocument();
+    expect(link).toHaveAttribute('href', mockUrl);
  });

  it('displays an icon if a valid icon is provided', () => {
--- a/public/app/core/components/sidemenu/DropDownChild.tsx
+++ b/public/app/core/components/sidemenu/DropDownChild.tsx
@ -5,11 +5,13 @@ import { Icon, IconName, Link, useTheme2 } from '@grafana/ui';
 export interface Props {
  isDivider?: boolean;
  icon?: IconName;
+  onClick?: () => void;
+  target?: HTMLAnchorElement['target'];
  text: string;
  url?: string;
 }

-const DropDownChild = ({ isDivider = false, icon, text, url }: Props) => {
+const DropDownChild = ({ isDivider = false, icon, onClick, target, text, url }: Props) => {
  const theme = useTheme2();
  const iconClassName = css`
    margin-right: ${theme.spacing(1)};
@ -22,7 +24,18 @@ const DropDownChild = ({ isDivider = false, icon, text, url }: Props) => {
    </>
  );

-  const anchor = url ? <Link href={url}>{linkContent}</Link> : <a>{linkContent}</a>;
+  let anchor = <a onClick={onClick}>{linkContent}</a>;
+  if (url) {
+    anchor = url.startsWith('/') ? (
+      <Link onClick={onClick} href={url}>
+        {linkContent}
+      </Link>
+    ) : (
+      <a href={url} target={target} rel="noopener" onClick={onClick}>
+        {linkContent}
+      </a>
+    );
+  }

  return isDivider ? <li data-testid="dropdown-child-divider" className="divider" /> : <li>{anchor}</li>;
 };
--- a/public/app/core/components/sidemenu/SideMenuDropDown.test.tsx
+++ b/public/app/core/components/sidemenu/SideMenuDropDown.test.tsx
@ -23,7 +23,7 @@ describe('SideMenuDropDown', () => {
    expect(text).toBeInTheDocument();
  });

-  it('attaches the link to the header text if provided', () => {
+  it('attaches the header url to the header text if provided', () => {
    render(
      <BrowserRouter>
        <SideMenuDropDown headerText={mockHeaderText} headerUrl={mockHeaderUrl} />
@ -31,6 +31,7 @@ describe('SideMenuDropDown', () => {
    );
    const link = screen.getByRole('link', { name: mockHeaderText });
    expect(link).toBeInTheDocument();
+    expect(link).toHaveAttribute('href', mockHeaderUrl);
  });

  it('calls the onHeaderClick function when the header is clicked', () => {
--- a/public/app/core/components/sidemenu/SideMenuDropDown.tsx
+++ b/public/app/core/components/sidemenu/SideMenuDropDown.tsx
@ -2,15 +2,25 @@ import React from 'react';
 import DropDownChild from './DropDownChild';
 import { NavModelItem } from '@grafana/data';
 import { IconName, Link } from '@grafana/ui';
+import { css } from '@emotion/css';

 interface Props {
-  items?: NavModelItem[];
  headerText: string;
  headerUrl?: string;
+  items?: NavModelItem[];
  onHeaderClick?: () => void;
+  reverseDirection?: boolean;
+  subtitleText?: string;
 }

-const SideMenuDropDown = ({ items = [], headerText, headerUrl, onHeaderClick }: Props) => {
+const SideMenuDropDown = ({
+  headerText,
+  headerUrl,
+  items = [],
+  onHeaderClick,
+  reverseDirection = false,
+  subtitleText,
+}: Props) => {
  const headerContent = <span className="sidemenu-item-text">{headerText}</span>;
  const header = headerUrl ? (
    <Link href={headerUrl} onClick={onHeaderClick} className="side-menu-header-link">
@ -22,8 +32,12 @@ const SideMenuDropDown = ({ items = [], headerText, headerUrl, onHeaderClick }:
    </a>
  );

+  const menuClass = css`
+    flex-direction: ${reverseDirection ? 'column-reverse' : 'column'};
+  `;
+
  return (
-    <ul className="dropdown-menu dropdown-menu--sidemenu" role="menu">
+    <ul className={`${menuClass} dropdown-menu dropdown-menu--sidemenu`} role="menu">
      <li className="side-menu-header">{header}</li>
      {items
        .filter((item) => !item.hideFromMenu)
@ -32,10 +46,17 @@ const SideMenuDropDown = ({ items = [], headerText, headerUrl, onHeaderClick }:
            key={`${child.url}-${index}`}
            isDivider={child.divider}
            icon={child.icon as IconName}
+            onClick={child.onClick}
+            target={child.target}
            text={child.text}
            url={child.url}
          />
        ))}
+      {subtitleText && (
+        <li className="sidemenu-subtitle">
+          <span className="sidemenu-item-text">{subtitleText}</span>
+        </li>
+      )}
    </ul>
  );
 };
--- a/public/app/core/components/sidemenu/snapshots/BottomNavLinks.test.tsx.snap
+++ b/public/app/core/components/sidemenu/snapshots/BottomNavLinks.test.tsx.snap
@ -1,176 +0,0 @@
-// Jest Snapshot v1, https://goo.gl/fbAQLP
-
-exports[`Render should render children 1`] = `
-<div
-  className="sidemenu-item dropdown dropup"
->
-  <Link
-    className="sidemenu-link"
-  >
-    <span
-      className="icon-circle sidemenu-icon"
-    />
-  </Link>
-  <ul
-    className="dropdown-menu dropdown-menu--sidemenu"
-    role="menu"
-  >
-    <li
-      key="undefined-0"
-    >
-      <a
-        rel="noopener"
-      />
-    </li>
-    <li
-      key="undefined-1"
-    >
-      <a
-        rel="noopener"
-      />
-    </li>
-    <li
-      key="undefined-2"
-    >
-      <a
-        rel="noopener"
-      />
-    </li>
-    <li
-      key="undefined-3"
-    >
-      <a
-        rel="noopener"
-      />
-    </li>
-    <li
-      className="side-menu-header"
-    >
-      <span
-        className="sidemenu-item-text"
-      />
-    </li>
-  </ul>
-</div>
-`;
-
-exports[`Render should render component 1`] = `
-<div
-  className="sidemenu-item dropdown dropup"
->
-  <Link
-    className="sidemenu-link"
-  >
-    <span
-      className="icon-circle sidemenu-icon"
-    />
-  </Link>
-  <ul
-    className="dropdown-menu dropdown-menu--sidemenu"
-    role="menu"
-  >
-    <li
-      className="side-menu-header"
-    >
-      <span
-        className="sidemenu-item-text"
-      >
-        Hello
-      </span>
-    </li>
-  </ul>
-</div>
-`;
-
-exports[`Render should render organization switcher 1`] = `
-<div
-  className="sidemenu-item dropdown dropup"
->
-  <Link
-    className="sidemenu-link"
-  >
-    <span
-      className="icon-circle sidemenu-icon"
-    />
-  </Link>
-  <ul
-    className="dropdown-menu dropdown-menu--sidemenu"
-    role="menu"
-  >
-    <li
-      className="sidemenu-org-switcher"
-    >
-      <a
-        onClick={[Function]}
-      >
-        <div>
-          <div
-            className="sidemenu-org-switcher__org-current"
-          >
-            Current Org.:
-          </div>
-          <div
-            className="sidemenu-org-switcher__org-name"
-          >
-            Grafana
-          </div>
-        </div>
-        <div
-          className="sidemenu-org-switcher__switch"
-        >
-          <Icon
-            className="css-f8is2k"
-            name="arrow-random"
-          />
-          Switch
-        </div>
-      </a>
-    </li>
-    <OrgSwitcher
-      onDismiss={[Function]}
-    />
-    <li
-      className="side-menu-header"
-    >
-      <span
-        className="sidemenu-item-text"
-      />
-    </li>
-  </ul>
-</div>
-`;
-
-exports[`Render should render subtitle 1`] = `
-<div
-  className="sidemenu-item dropdown dropup"
->
-  <Link
-    className="sidemenu-link"
-  >
-    <span
-      className="icon-circle sidemenu-icon"
-    />
-  </Link>
-  <ul
-    className="dropdown-menu dropdown-menu--sidemenu"
-    role="menu"
-  >
-    <li
-      className="sidemenu-subtitle"
-    >
-      <span
-        className="sidemenu-item-text"
-      >
-        subtitle
-      </span>
-    </li>
-    <li
-      className="side-menu-header"
-    >
-      <span
-        className="sidemenu-item-text"
-      />
-    </li>
-  </ul>
-</div>
-`;
--- a/public/sass/components/_sidemenu.scss
+++ b/public/sass/components/_sidemenu.scss
@ -67,7 +67,7 @@ $mobile-menu-breakpoint: md;
      .dropdown-menu {
        border: none;
        margin: 0;
-        display: block;
+        display: flex;
        opacity: 0;
        top: 0px;
        // important to overlap it otherwise it can be hidden
@ -262,7 +262,7 @@ li.sidemenu-org-switcher {
    }

    .dropdown-menu--sidemenu {
-      display: block;
+      display: flex;
      position: unset;
      width: 100%;
      float: none;
@ -276,7 +276,7 @@ li.sidemenu-org-switcher {
    .sidemenu__bottom {
      .dropdown-menu--sidemenu {
        display: flex;
-        flex-direction: column-reverse;
+        flex-direction: column;
      }
    }
  }