Implement left arrow to focus parent, fix list style on firefox (#43345)

* Implement left arrow to close menu for now, fix list style on firefox

* Implement onLeft

* Fix outline of first item in navbar

* Fix focus styles appearing when using mouse

* add unit test

public/app/core/components/NavBar/NavBarItem.test.tsx

@@ -136,12 +136,14 @@ describe('NavBarItem', () => {
         getTestContext({ link: { ...defaults.link, url: 'https://www.grafana.com' } });
 
         userEvent.tab();
+        expect(screen.getAllByRole('link')[0]).toHaveFocus();
         expect(screen.getAllByRole('menuitem')).toHaveLength(3);
         expect(screen.getAllByRole('menuitem')[0]).toHaveAttribute('tabIndex', '-1');
         expect(screen.getAllByRole('menuitem')[1]).toHaveAttribute('tabIndex', '-1');
         expect(screen.getAllByRole('menuitem')[2]).toHaveAttribute('tabIndex', '-1');
 
         userEvent.keyboard('{arrowright}');
+        expect(screen.getAllByRole('link')[0]).not.toHaveFocus();
         expect(screen.getAllByRole('menuitem')).toHaveLength(3);
         expect(screen.getAllByRole('menuitem')[0]).toHaveAttribute('tabIndex', '0');
         expect(screen.getAllByRole('menuitem')[1]).toHaveAttribute('tabIndex', '-1');
@@ -149,6 +151,27 @@ describe('NavBarItem', () => {
       });
     });
 
+    describe('and pressing arrow left on a menu item', () => {
+      it('then the nav bar item should receive focus', () => {
+        getTestContext({ link: { ...defaults.link, url: 'https://www.grafana.com' } });
+
+        userEvent.tab();
+        userEvent.keyboard('{arrowright}');
+        expect(screen.getAllByRole('link')[0]).not.toHaveFocus();
+        expect(screen.getAllByRole('menuitem')).toHaveLength(3);
+        expect(screen.getAllByRole('menuitem')[0]).toHaveAttribute('tabIndex', '0');
+        expect(screen.getAllByRole('menuitem')[1]).toHaveAttribute('tabIndex', '-1');
+        expect(screen.getAllByRole('menuitem')[2]).toHaveAttribute('tabIndex', '-1');
+
+        userEvent.keyboard('{arrowleft}');
+        expect(screen.getAllByRole('link')[0]).toHaveFocus();
+        expect(screen.getAllByRole('menuitem')).toHaveLength(3);
+        expect(screen.getAllByRole('menuitem')[0]).toHaveAttribute('tabIndex', '-1');
+        expect(screen.getAllByRole('menuitem')[1]).toHaveAttribute('tabIndex', '-1');
+        expect(screen.getAllByRole('menuitem')[2]).toHaveAttribute('tabIndex', '-1');
+      });
+    });
+
     describe('when appSubUrl is configured and user clicks on menuitem link', () => {
       it('then location service should be called with correct url', async () => {
         const { pushMock } = getTestContext(

public/app/core/components/NavBar/NavBarItemMenu.tsx

@@ -37,8 +37,9 @@ export function NavBarItemMenu(props: NavBarItemMenuProps): ReactElement | null
     if (menuHasFocus && !state.selectionManager.isFocused) {
       state.selectionManager.setFocusedKey(section?.key ?? '');
       state.selectionManager.setFocused(true);
-    } else if (!menuHasFocus && state.selectionManager.isFocused) {
+    } else if (!menuHasFocus) {
       state.selectionManager.setFocused(false);
+      state.selectionManager.setFocusedKey('');
       state.selectionManager.clearSelection();
     }
   }, [menuHasFocus, state.selectionManager, reverseMenuDirection, section?.key]);

public/app/core/components/NavBar/NavBarItemMenuItem.tsx

@@ -3,7 +3,7 @@ import { css } from '@emotion/css';
 import { useTheme2 } from '@grafana/ui';
 import { GrafanaTheme2, NavModelItem } from '@grafana/data';
 import { useMenuItem } from '@react-aria/menu';
-import { useFocus } from '@react-aria/interactions';
+import { useFocus, useKeyboard } from '@react-aria/interactions';
 import { TreeState } from '@react-stately/tree';
 import { mergeProps } from '@react-aria/utils';
 import { Node } from '@react-types/shared';
@@ -19,7 +19,7 @@ export interface NavBarItemMenuItemProps {
 }
 
 export function NavBarItemMenuItem({ className, item, state, onNavigate }: NavBarItemMenuItemProps): ReactElement {
-  const { onClose } = useNavBarItemMenuContext();
+  const { onClose, onLeft } = useNavBarItemMenuContext();
   const { key, rendered } = item;
   const ref = useRef<HTMLLIElement>(null);
   const isDisabled = state.disabledKeys.has(key);
@@ -47,8 +47,21 @@ export function NavBarItemMenuItem({ className, item, state, onNavigate }: NavBa
     ref
   );
 
+  const { keyboardProps } = useKeyboard({
+    onKeyDown: (e) => {
+      if (e.key === 'ArrowLeft') {
+        onLeft();
+      }
+      e.continuePropagation();
+    },
+  });
+
   return (
-    <li {...mergeProps(menuItemProps, focusProps)} ref={ref} className={classNames(styles.menuItem, className)}>
+    <li
+      {...mergeProps(menuItemProps, focusProps, keyboardProps)}
+      ref={ref}
+      className={classNames(styles.menuItem, className)}
+    >
       {rendered}
     </li>
   );

public/app/core/components/NavBar/NavBarItemMenuTrigger.tsx

@@ -5,7 +5,7 @@ import { GrafanaTheme2, NavModelItem } from '@grafana/data';
 import { MenuTriggerProps } from '@react-types/menu';
 import { useMenuTriggerState } from '@react-stately/menu';
 import { useMenuTrigger } from '@react-aria/menu';
-import { useFocusVisible, useFocusWithin, useHover, useKeyboard } from '@react-aria/interactions';
+import { useFocusWithin, useHover, useKeyboard } from '@react-aria/interactions';
 import { useButton } from '@react-aria/button';
 import { DismissButton, useOverlay } from '@react-aria/overlays';
 import { FocusScope } from '@react-aria/focus';
@@ -29,12 +29,9 @@ export function NavBarItemMenuTrigger(props: NavBarItemMenuTriggerProps): ReactE
   const state = useMenuTriggerState({ ...rest });
 
   // Get props for the menu trigger and menu elements
-  const ref = React.useRef(null);
+  const ref = React.useRef<HTMLButtonElement>(null);
   const { menuTriggerProps, menuProps } = useMenuTrigger({}, state, ref);
 
-  // style to the focused menu item
-  let { isFocusVisible } = useFocusVisible({ isTextInput: false });
-
   const { hoverProps } = useHover({
     onHoverChange: (isHovering) => {
       if (isHovering) {
@@ -47,7 +44,7 @@ export function NavBarItemMenuTrigger(props: NavBarItemMenuTriggerProps): ReactE
 
   const { focusWithinProps } = useFocusWithin({
     onFocusWithinChange: (isFocused) => {
-      if (isFocused && isFocusVisible) {
+      if (isFocused) {
         state.open();
       }
       if (!isFocused) {
@@ -132,7 +129,6 @@ export function NavBarItemMenuTrigger(props: NavBarItemMenuTriggerProps): ReactE
   const { overlayProps } = useOverlay(
     {
       onClose: () => state.close(),
-      shouldCloseOnBlur: true,
       isOpen: state.isOpen,
       isDismissable: true,
     },
@@ -143,7 +139,17 @@ export function NavBarItemMenuTrigger(props: NavBarItemMenuTriggerProps): ReactE
     <div className={cx(styles.element, 'dropdown')} {...focusWithinProps} {...hoverProps}>
       {element}
       {state.isOpen && (
-        <NavBarItemMenuContext.Provider value={{ menuProps, menuHasFocus, onClose: () => state.close() }}>
+        <NavBarItemMenuContext.Provider
+          value={{
+            menuProps,
+            menuHasFocus,
+            onClose: () => state.close(),
+            onLeft: () => {
+              setMenuHasFocus(false);
+              ref.current?.focus();
+            },
+          }}
+        >
           <FocusScope restoreFocus>
             <div {...overlayProps} ref={overlayRef}>
               <DismissButton onDismiss={() => state.close()} />

public/app/core/components/NavBar/NavBarItemWithoutMenu.tsx

@@ -100,7 +100,7 @@ export function getNavBarItemWithoutMenuStyles(theme: GrafanaTheme2, isActive?:
         box-shadow: none;
         color: ${theme.colors.text.primary};
         outline: 2px solid ${theme.colors.primary.main};
-        outline-offset: 2px;
+        outline-offset: -2px;
         transition: none;
       }
     `,

public/app/core/components/NavBar/NavBarSection.tsx

@@ -24,6 +24,7 @@ export function NavBarSection({ children, className }: Props) {
 const getStyles = (theme: GrafanaTheme2, newNavigationEnabled: boolean) => ({
   container: css`
     display: none;
+    list-style: none;
 
     ${theme.breakpoints.up('md')} {
       background-color: ${newNavigationEnabled ? theme.colors.background.primary : 'inherit'};

public/app/core/components/NavBar/context.tsx

@@ -3,12 +3,14 @@ import { createContext, HTMLAttributes, useContext } from 'react';
 export interface NavBarItemMenuContextProps {
   menuHasFocus: boolean;
   onClose: () => void;
+  onLeft: () => void;
   menuProps?: HTMLAttributes<HTMLElement>;
 }
 
 export const NavBarItemMenuContext = createContext<NavBarItemMenuContextProps>({
   menuHasFocus: false,
   onClose: () => undefined,
+  onLeft: () => undefined,
 });
 
 export function useNavBarItemMenuContext(): NavBarItemMenuContextProps {