From 90d2d1f4da7c30046eebcf561fc879c1de71b635 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Tue, 9 Nov 2021 13:41:38 +0000 Subject: [PATCH] Navigation: Refactor mobile menu into it's own component (#41308) * Navigation: Start creating new NavBarMenu component * Navigation: Apply new NavBarMenu to NavBarNext * Navigation: Remove everything to do with .sidemenu-open--xs * Navigation: Ensure search is passed to NavBarMenu * Navigation: Standardise NavBarMenuItem * This extra check isn't needed anymore * Navigation: Refactor
  • out of NavBarMenu * Navigation: Combine NavBarMenuItem with DropdownChild * use spread syntax since performance shouldn't be a concern for such small arrays * Improve active item logic * Ensure unique keys * Remove this duplicate code * Add unit tests for getActiveItem * Add tests for NavBarMenu * Rename mobileMenuOpen -> menuOpen in NavBarNext (since it can be used for mobile menu or megamenu) * just use index to key the items * Use exact versions of @react-aria packages * Navigation: Make the dropdown header a NavBarMenuItem * Navigation: Stop using dropdown-menu for styles * Navigation: Hide divider in NavBarMenu + tweak color on section header --- package.json | 2 + .../core/components/NavBar/DropdownChild.tsx | 74 ----- public/app/core/components/NavBar/NavBar.tsx | 157 +++++----- .../components/NavBar/NavBarDropdown.test.tsx | 2 +- .../core/components/NavBar/NavBarDropdown.tsx | 84 ++---- .../app/core/components/NavBar/NavBarItem.tsx | 44 +-- .../components/NavBar/NavBarMenu.test.tsx | 31 ++ .../app/core/components/NavBar/NavBarMenu.tsx | 121 ++++++++ ...Child.test.tsx => NavBarMenuItem.test.tsx} | 14 +- .../core/components/NavBar/NavBarMenuItem.tsx | 117 ++++++++ .../app/core/components/NavBar/NavBarNext.tsx | 102 +++---- .../core/components/NavBar/NavBarSection.tsx | 6 - .../app/core/components/NavBar/utils.test.ts | 278 +++++++----------- public/app/core/components/NavBar/utils.ts | 80 +++-- public/app/routes/GrafanaCtrl.ts | 4 - public/app/types/events.ts | 1 - public/sass/components/_dropdown.scss | 9 +- yarn.lock | 19 +- 18 files changed, 611 insertions(+), 534 deletions(-) delete mode 100644 public/app/core/components/NavBar/DropdownChild.tsx create mode 100644 public/app/core/components/NavBar/NavBarMenu.test.tsx create mode 100644 public/app/core/components/NavBar/NavBarMenu.tsx rename public/app/core/components/NavBar/{DropdownChild.test.tsx => NavBarMenuItem.test.tsx} (77%) create mode 100644 public/app/core/components/NavBar/NavBarMenuItem.tsx diff --git a/package.json b/package.json index c14ba5ebdd5..5cd820d1c6a 100644 --- a/package.json +++ b/package.json @@ -243,6 +243,8 @@ "@opentelemetry/exporter-collector": "0.23.0", "@opentelemetry/semantic-conventions": "1.0.0", "@popperjs/core": "2.5.4", + "@react-aria/focus": "3.5.0", + "@react-aria/overlays": "3.7.2", "@reduxjs/toolkit": "1.6.1", "@sentry/browser": "5.25.0", "@sentry/types": "5.24.2", diff --git a/public/app/core/components/NavBar/DropdownChild.tsx b/public/app/core/components/NavBar/DropdownChild.tsx deleted file mode 100644 index abd9c95d6f2..00000000000 --- a/public/app/core/components/NavBar/DropdownChild.tsx +++ /dev/null @@ -1,74 +0,0 @@ -import React from 'react'; -import { css } from '@emotion/css'; -import { GrafanaTheme2 } from '@grafana/data'; -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, onClick, target, text, url }: Props) => { - const theme = useTheme2(); - const styles = getStyles(theme); - - const linkContent = ( -
    -
    - {icon && } - {text} -
    - {target === '_blank' && ( - - )} -
    - ); - - let element = ( - - ); - if (url) { - element = - !target && url.startsWith('/') ? ( - - {linkContent} - - ) : ( - - {linkContent} - - ); - } - - return isDivider ?
  • :
  • {element}
    • ; -}; - -export default DropdownChild; - -const getStyles = (theme: GrafanaTheme2) => ({ - element: css` - background-color: transparent; - border: none; - display: flex; - width: 100%; - `, - externalLinkIcon: css` - color: ${theme.colors.text.secondary}; - margin-left: ${theme.spacing(1)}; - `, - icon: css` - margin-right: ${theme.spacing(1)}; - `, - linkContent: css` - display: flex; - flex: 1; - flex-direction: row; - justify-content: space-between; - `, -}); diff --git a/public/app/core/components/NavBar/NavBar.tsx b/public/app/core/components/NavBar/NavBar.tsx index 4dbfd54cf54..c23ab43fa55 100644 --- a/public/app/core/components/NavBar/NavBar.tsx +++ b/public/app/core/components/NavBar/NavBar.tsx @@ -1,20 +1,31 @@ -import React, { FC, useCallback, useState } from 'react'; +import React, { FC, useState } from 'react'; import { useLocation } from 'react-router-dom'; import { css, cx } from '@emotion/css'; import { cloneDeep } from 'lodash'; import { GrafanaTheme2, NavModelItem, NavSection } from '@grafana/data'; import { Icon, IconName, useTheme2 } from '@grafana/ui'; import { locationService } from '@grafana/runtime'; -import appEvents from '../../app_events'; import { Branding } from 'app/core/components/Branding/Branding'; import config from 'app/core/config'; -import { CoreEvents, KioskMode } from 'app/types'; -import { enrichConfigItems, isLinkActive, isSearchActive } from './utils'; +import { KioskMode } from 'app/types'; +import { enrichConfigItems, getActiveItem, isMatchOrChildMatch, isSearchActive, SEARCH_ITEM_ID } from './utils'; import { OrgSwitcher } from '../OrgSwitcher'; import NavBarItem from './NavBarItem'; +import { NavBarSection } from './NavBarSection'; +import { NavBarMenu } from './NavBarMenu'; const homeUrl = config.appSubUrl || '/'; +const onOpenSearch = () => { + locationService.partial({ search: 'open' }); +}; + +const searchItem: NavModelItem = { + id: SEARCH_ITEM_ID, + onClick: onOpenSearch, + text: 'Search dashboards', +}; + export const NavBar: FC = React.memo(() => { const theme = useTheme2(); const styles = getStyles(theme); @@ -32,78 +43,79 @@ export const NavBar: FC = React.memo(() => { location, toggleSwitcherModal ); - const activeItemId = isSearchActive(location) - ? 'search' - : navTree.find((item) => isLinkActive(location.pathname, item))?.id; + const activeItem = isSearchActive(location) ? searchItem : getActiveItem(navTree, location.pathname); - const toggleNavBarSmallBreakpoint = useCallback(() => { - appEvents.emit(CoreEvents.toggleSidemenuMobile); - }, []); + const [mobileMenuOpen, setMobileMenuOpen] = useState(false); if (kiosk !== null) { return null; } - const onOpenSearch = () => { - locationService.partial({ search: 'open' }); - }; - return ( ); }); @@ -118,11 +130,6 @@ const getStyles = (theme: GrafanaTheme2) => ({ ${theme.breakpoints.up('md')} { display: block; } - - .sidemenu-open--xs & { - display: block; - margin-top: 0; - } `, sidemenu: css` display: flex; @@ -141,16 +148,6 @@ const getStyles = (theme: GrafanaTheme2) => ({ .sidemenu-hidden & { display: none; } - - .sidemenu-open--xs & { - background-color: ${theme.colors.background.primary}; - box-shadow: ${theme.shadows.z1}; - gap: ${theme.spacing(1)}; - height: auto; - margin-left: 0; - position: absolute; - width: 100%; - } `, grafanaLogo: css` display: none; @@ -165,14 +162,6 @@ const getStyles = (theme: GrafanaTheme2) => ({ justify-content: center; } `, - closeButton: css` - display: none; - - .sidemenu-open--xs & { - display: block; - font-size: ${theme.typography.fontSize}px; - } - `, mobileSidemenuLogo: css` align-items: center; cursor: pointer; @@ -187,9 +176,5 @@ const getStyles = (theme: GrafanaTheme2) => ({ `, spacer: css` flex: 1; - - .sidemenu-open--xs & { - display: none; - } `, }); diff --git a/public/app/core/components/NavBar/NavBarDropdown.test.tsx b/public/app/core/components/NavBar/NavBarDropdown.test.tsx index 8083daed70a..b220cb4362e 100644 --- a/public/app/core/components/NavBar/NavBarDropdown.test.tsx +++ b/public/app/core/components/NavBar/NavBarDropdown.test.tsx @@ -26,7 +26,7 @@ describe('NavBarDropdown', () => { it('attaches the header url to the header text if provided', () => { render( - + ); const link = screen.getByRole('link', { name: mockHeaderText }); diff --git a/public/app/core/components/NavBar/NavBarDropdown.tsx b/public/app/core/components/NavBar/NavBarDropdown.tsx index 3da5e6e063d..24f88c04e57 100644 --- a/public/app/core/components/NavBar/NavBarDropdown.tsx +++ b/public/app/core/components/NavBar/NavBarDropdown.tsx @@ -1,13 +1,14 @@ import React from 'react'; import { css } from '@emotion/css'; import { GrafanaTheme2, NavModelItem } from '@grafana/data'; -import { IconName, Link, useTheme2 } from '@grafana/ui'; -import DropdownChild from './DropdownChild'; +import { IconName, useTheme2 } from '@grafana/ui'; +import { NavBarMenuItem } from './NavBarMenuItem'; interface Props { headerTarget?: HTMLAnchorElement['target']; headerText: string; headerUrl?: string; + isVisible?: boolean; items?: NavModelItem[]; onHeaderClick?: () => void; reverseDirection?: boolean; @@ -18,6 +19,7 @@ const NavBarDropdown = ({ headerTarget, headerText, headerUrl, + isVisible, items = [], onHeaderClick, reverseDirection = false, @@ -25,32 +27,20 @@ const NavBarDropdown = ({ }: Props) => { const filteredItems = items.filter((item) => !item.hideFromMenu); const theme = useTheme2(); - const styles = getStyles(theme, reverseDirection, filteredItems); - - let header = ( - - ); - if (headerUrl) { - header = - !headerTarget && headerUrl.startsWith('/') ? ( - - {headerText} - - ) : ( - - {headerText} - - ); - } + const styles = getStyles(theme, reverseDirection, filteredItems, isVisible); return ( -