Dropdown: Fix keyboard accessibility (#84683)

* fix dropdown keyboard a11y

* remove unnecessary css

* restore tabIndex to keep linting happy

* use Box in Menu

* fix unit test

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

@@ -1,5 +1,6 @@
 import { css } from '@emotion/css';
 import {
+  FloatingFocusManager,
   autoUpdate,
   flip,
   offset as floatingUIOffset,
@@ -9,7 +10,6 @@ import {
   useFloating,
   useInteractions,
 } from '@floating-ui/react';
-import { FocusScope } from '@react-aria/focus';
 import React, { useEffect, useRef, useState } from 'react';
 import { CSSTransition } from 'react-transition-group';
 
@@ -83,7 +83,7 @@ export const Dropdown = React.memo(({ children, overlay, placement, offset, onVi
       })}
       {show && (
         <Portal>
-          <FocusScope autoFocus restoreFocus contain>
+          <FloatingFocusManager context={context}>
             {/*
               this is handling bubbled events from the inner overlay
               see https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/docs/rules/no-static-element-interactions.md#case-the-event-handler-is-only-being-used-to-capture-bubbled-events
@@ -100,7 +100,7 @@ export const Dropdown = React.memo(({ children, overlay, placement, offset, onVi
                 <div ref={transitionRef}>{ReactUtils.renderOrCallToRender(overlay, { ...getFloatingProps() })}</div>
               </CSSTransition>
             </div>
-          </FocusScope>
+          </FloatingFocusManager>
         </Portal>
       )}
     </>

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

@@ -4,6 +4,7 @@ import React, { useImperativeHandle, useRef } from 'react';
 import { GrafanaTheme2 } from '@grafana/data';
 
 import { useStyles2 } from '../../themes';
+import { Box } from '../Layout/Box/Box';
 
 import { MenuDivider } from './MenuDivider';
 import { MenuGroup } from './MenuGroup';
@@ -27,17 +28,22 @@ const MenuComp = React.forwardRef<HTMLDivElement, MenuProps>(
     const localRef = useRef<HTMLDivElement>(null);
     useImperativeHandle(forwardedRef, () => localRef.current!);
 
-    const [handleKeys] = useMenuFocus({ localRef, onOpen, onClose, onKeyDown });
+    const [handleKeys] = useMenuFocus({ isMenuOpen: true, localRef, onOpen, onClose, onKeyDown });
 
     return (
-      <div
+      <Box
         {...otherProps}
-        tabIndex={-1}
-        ref={localRef}
-        className={styles.wrapper}
-        role="menu"
         aria-label={ariaLabel}
+        backgroundColor="primary"
+        borderRadius="default"
+        boxShadow="z3"
+        display="inline-block"
         onKeyDown={handleKeys}
+        paddingX={0}
+        paddingY={0.5}
+        ref={localRef}
+        role="menu"
+        tabIndex={-1}
       >
         {header && (
           <div
@@ -50,7 +56,7 @@ const MenuComp = React.forwardRef<HTMLDivElement, MenuProps>(
           </div>
         )}
         {children}
-      </div>
+      </Box>
     );
   }
 );
@@ -66,17 +72,10 @@ export const Menu = Object.assign(MenuComp, {
 const getStyles = (theme: GrafanaTheme2) => {
   return {
     header: css({
-      padding: `${theme.spacing(0.5, 1, 1, 1)}`,
+      padding: theme.spacing(0.5, 1, 1, 1),
     }),
     headerBorder: css({
       borderBottom: `1px solid ${theme.colors.border.weak}`,
     }),
-    wrapper: css({
-      background: `${theme.colors.background.primary}`,
-      boxShadow: `${theme.shadows.z3}`,
-      display: `inline-block`,
-      borderRadius: `${theme.shape.radius.default}`,
-      padding: `${theme.spacing(0.5, 0)}`,
-    }),
   };
 };

packages/grafana-ui/src/components/Menu/MenuItem.tsx

@@ -79,7 +79,6 @@ export const MenuItem = React.memo(
     const styles = useStyles2(getStyles);
     const [isActive, setIsActive] = useState(active);
     const [isSubMenuOpen, setIsSubMenuOpen] = useState(false);
-    const [openedWithArrow, setOpenedWithArrow] = useState(false);
     const onMouseEnter = useCallback(() => {
       if (disabled) {
         return;
@@ -128,7 +127,6 @@ export const MenuItem = React.memo(
           event.stopPropagation();
           if (hasSubMenu) {
             setIsSubMenuOpen(true);
-            setOpenedWithArrow(true);
             setIsActive(true);
           }
           break;
@@ -178,8 +176,6 @@ export const MenuItem = React.memo(
               <SubMenu
                 items={childItems}
                 isOpen={isSubMenuOpen}
-                openedWithArrow={openedWithArrow}
-                setOpenedWithArrow={setOpenedWithArrow}
                 close={closeSubMenu}
                 customStyle={customSubMenuContainerStyles}
               />
@@ -219,7 +215,7 @@ const getStyles = (theme: GrafanaTheme2) => {
       width: '100%',
       position: 'relative',
 
-      '&:hover, &:focus, &:focus-visible': {
+      '&:hover, &:focus-visible': {
         background: theme.colors.action.hover,
         color: theme.colors.text.primary,
         textDecoration: 'none',

packages/grafana-ui/src/components/Menu/SubMenu.test.tsx

@@ -13,9 +13,7 @@ describe('SubMenu', () => {
       <MenuItem key="subitem2" label="subitem2" icon="apps" />,
     ];
 
-    render(
+    render(<SubMenu items={items} isOpen={true} close={jest.fn()} />);
-      <SubMenu items={items} isOpen={true} openedWithArrow={false} setOpenedWithArrow={jest.fn()} close={jest.fn()} />
-    );
 
     expect(screen.getByTestId(selectors.components.Menu.SubMenu.icon)).toBeInTheDocument();
 

packages/grafana-ui/src/components/Menu/SubMenu.tsx

@@ -17,10 +17,6 @@ export interface SubMenuProps {
   items?: Array<ReactElement<MenuItemProps>>;
   /** Open */
   isOpen: boolean;
-  /** Marks whether subMenu was opened with arrow */
-  openedWithArrow: boolean;
-  /** Changes value of openedWithArrow */
-  setOpenedWithArrow: (openedWithArrow: boolean) => void;
   /** Closes the subMenu */
   close: () => void;
   /** Custom style */
@@ -28,46 +24,42 @@ export interface SubMenuProps {
 }
 
 /** @internal */
-export const SubMenu = React.memo(
+export const SubMenu = React.memo(({ items, isOpen, close, customStyle }: SubMenuProps) => {
-  ({ items, isOpen, openedWithArrow, setOpenedWithArrow, close, customStyle }: SubMenuProps) => {
+  const styles = useStyles2(getStyles);
-    const styles = useStyles2(getStyles);
+  const localRef = useRef<HTMLDivElement>(null);
-    const localRef = useRef<HTMLDivElement>(null);
+  const [handleKeys] = useMenuFocus({
-    const [handleKeys] = useMenuFocus({
+    localRef,
-      localRef,
+    isMenuOpen: isOpen,
-      isMenuOpen: isOpen,
+    close,
-      openedWithArrow,
+  });
-      setOpenedWithArrow,
-      close,
-    });
 
-    const [pushLeft, setPushLeft] = useState(false);
+  const [pushLeft, setPushLeft] = useState(false);
-    useEffect(() => {
+  useEffect(() => {
-      if (isOpen && localRef.current) {
+    if (isOpen && localRef.current) {
-        setPushLeft(isElementOverflowing(localRef.current));
+      setPushLeft(isElementOverflowing(localRef.current));
-      }
+    }
-    }, [isOpen]);
+  }, [isOpen]);
 
-    return (
+  return (
-      <>
+    <>
-        <div className={styles.iconWrapper} aria-hidden data-testid={selectors.components.Menu.SubMenu.icon}>
+      <div className={styles.iconWrapper} aria-hidden data-testid={selectors.components.Menu.SubMenu.icon}>
-          <Icon name="angle-right" className={styles.icon} />
+        <Icon name="angle-right" className={styles.icon} />
-        </div>
+      </div>
-        {isOpen && (
+      {isOpen && (
-          <div
+        <div
-            ref={localRef}
+          ref={localRef}
-            className={cx(styles.subMenu, { [styles.pushLeft]: pushLeft })}
+          className={cx(styles.subMenu, { [styles.pushLeft]: pushLeft })}
-            data-testid={selectors.components.Menu.SubMenu.container}
+          data-testid={selectors.components.Menu.SubMenu.container}
-            style={customStyle}
+          style={customStyle}
-          >
+        >
-            <div tabIndex={-1} className={styles.itemsWrapper} role="menu" onKeyDown={handleKeys}>
+          <div tabIndex={-1} className={styles.itemsWrapper} role="menu" onKeyDown={handleKeys}>
-              {items}
+            {items}
-            </div>
           </div>
-        )}
+        </div>
-      </>
+      )}
-    );
+    </>
-  }
+  );
-);
+});
 
 SubMenu.displayName = 'SubMenu';
 

packages/grafana-ui/src/components/Menu/hooks.test.tsx

@@ -141,18 +141,15 @@ describe('useMenuFocus', () => {
     expect(onKeyDown).toHaveBeenCalledTimes(2);
   });
 
-  it('focuses on first item when menu was opened with arrow', () => {
+  it('focuses on first item', () => {
     const ref = createRef<HTMLDivElement>();
 
     render(getMenuElement(ref));
 
     const isMenuOpen = true;
-    const openedWithArrow = true;
+    renderHook(() => useMenuFocus({ localRef: ref, isMenuOpen }));
-    const setOpenedWithArrow = jest.fn();
-    renderHook(() => useMenuFocus({ localRef: ref, isMenuOpen, openedWithArrow, setOpenedWithArrow }));
 
     expect(screen.getByText('Item 1').tabIndex).toBe(0);
-    expect(setOpenedWithArrow).toHaveBeenCalledWith(false);
   });
 
   it('clicks focused item when Enter key is pressed', () => {

packages/grafana-ui/src/components/Menu/hooks.ts

@@ -8,8 +8,6 @@ const UNFOCUSED = -1;
 export interface UseMenuFocusProps {
   localRef: RefObject<HTMLDivElement>;
   isMenuOpen?: boolean;
-  openedWithArrow?: boolean;
-  setOpenedWithArrow?: (openedWithArrow: boolean) => void;
   close?: () => void;
   onOpen?: (focusOnItem: (itemId: number) => void) => void;
   onClose?: () => void;
@@ -23,8 +21,6 @@ export type UseMenuFocusReturn = [(event: React.KeyboardEvent) => void];
 export const useMenuFocus = ({
   localRef,
   isMenuOpen,
-  openedWithArrow,
-  setOpenedWithArrow,
   close,
   onOpen,
   onClose,
@@ -33,11 +29,10 @@ export const useMenuFocus = ({
   const [focusedItem, setFocusedItem] = useState(UNFOCUSED);
 
   useEffect(() => {
-    if (isMenuOpen && openedWithArrow) {
+    if (isMenuOpen) {
       setFocusedItem(0);
-      setOpenedWithArrow?.(false);
     }
-  }, [isMenuOpen, openedWithArrow, setOpenedWithArrow]);
+  }, [isMenuOpen]);
 
   useEffect(() => {
     const menuItems = localRef?.current?.querySelectorAll<HTMLElement | HTMLButtonElement | HTMLAnchorElement>(

public/app/features/alerting/unified/components/contact-points/ContactPoints.test.tsx

@@ -126,6 +126,8 @@ describe('contact points', () => {
         await userEvent.click(button);
         const deleteButton = await screen.queryByRole('menuitem', { name: 'delete' });
         expect(deleteButton).toBeDisabled();
+        // click outside the menu to close it otherwise we can't interact with the rest of the page
+        await userEvent.click(document.body);
       }
 
       // check buttons in Notification Templates