[SG-36350] Accessibility: Global Menu: Focus isn't correctly captured when menus are opened

client/web/src/enterprise/insights/pages/dashboards/dashboard-page/DashboardsContentPage.test.tsx

@@ -165,15 +165,14 @@ const renderDashboardsContent = (
 
 const triggerDashboardMenuItem = async (screen: RenderWithBrandedContextResult & { user: UserEvent }, name: RegExp) => {
     const { user } = screen
-    const dashboardMenu = await waitFor(() => screen.getByRole('button', { name: /dashboard context menu/ }))
-    user.click(dashboardMenu)
 
-    const dashboardMenuItem = screen.getByRole('menuitem', { name })
+    const dashboardMenu = await screen.findByRole('button', { name: /dashboard context menu/ })
+    userEvent.click(dashboardMenu)
 
     // We're simulating keyboard navigation here to circumvent a bug in ReachUI
     // does not respond to programmatic click events on menu items
-    dashboardMenuItem.focus()
-    user.keyboard(' ')
+    screen.getByText(name).closest<HTMLButtonElement>('[role="menuitem"]')?.focus()
+    user.keyboard('{enter}')
 }
 
 beforeEach(() => {
@@ -218,7 +217,7 @@ describe('DashboardsContent', () => {
 
         const { history } = screen
 
-        await triggerDashboardMenuItem(screen, /configure dashboard/)
+        await triggerDashboardMenuItem(screen, /configure dashboard/i)
 
         expect(history.location.pathname).toEqual('/insights/dashboards/foo/edit')
     })
@@ -238,18 +237,17 @@ describe('DashboardsContent', () => {
     it('opens delete dashboard modal', async () => {
         const screen = renderDashboardsContent()
 
-        await triggerDashboardMenuItem(screen, /Delete/)
+        await triggerDashboardMenuItem(screen, /delete/i)
 
-        const addInsightHeader = await waitFor(() => screen.getByRole('heading', { name: /Delete/ }))
-        expect(addInsightHeader).toBeInTheDocument()
+        await waitFor(() => expect(screen.getByRole('heading', { name: /Delete/ })).toBeInTheDocument())
     })
 
     // copies dashboard url
     it('copies dashboard url', async () => {
         const screen = renderDashboardsContent()
 
-        await triggerDashboardMenuItem(screen, /Copy link/)
+        await triggerDashboardMenuItem(screen, /copy link/i)
 
-        sinon.assert.calledOnce(mockCopyURL)
+        await waitFor(() => sinon.assert.calledOnce(mockCopyURL))
     })
 })

client/web/src/nav/NavBar/NavDropdown.tsx

@@ -184,8 +184,8 @@ export const NavDropdown: React.FunctionComponent<React.PropsWithChildren<NavDro
                                 >
                                     {mobileHomeItem.content}
                                 </MenuLink>
-                                {items.map(item => (
-                                    <MenuLink as={Link} key={item.path} to={item.path}>
+                                {items.map((item, index) => (
+                                    <MenuLink as={Link} key={item.path} to={item.path} index={index}>
                                         {item.content}
                                     </MenuLink>
                                 ))}

client/wildcard/src/components/Menu/MenuItem.module.scss

@@ -38,9 +38,22 @@
             background-color: unset;
             color: unset;
         }
+
         &[data-selected]:not(:active) {
             background-color: var(--dropdown-link-hover-bg);
             color: unset;
         }
+
+        &[data-selected]:not(:hover) {
+            outline: none;
+            box-shadow: none;
+
+            > .dropdown-item-content {
+                width: 100%;
+                display: inline-block;
+                outline: 0;
+                box-shadow: var(--focus-box-shadow);
+            }
+        }
     }
 }

client/wildcard/src/components/Menu/MenuItem.tsx

@@ -23,7 +23,7 @@ export const MenuItem = React.forwardRef(({ children, className, disabled, ...pr
 
     return (
         <Component ref={reference} {...props} className={classNames(styles.dropdownItem, className)}>
-            {children}
+            <span className={styles.dropdownItemContent}>{children}</span>
         </Component>
     )
 }) as ForwardReferenceComponent<'div', MenuItemProps>

client/wildcard/src/components/Menu/MenuLink.tsx

@@ -19,8 +19,12 @@ export type MenuLinkProps = ReachMenuLinkProps
  *
  * @see — Docs https://reach.tech/menu-button#menulink
  */
-export const MenuLink = React.forwardRef(({ className, disabled, ...props }, reference) => {
+export const MenuLink = React.forwardRef(({ className, disabled, children, ...props }, reference) => {
     const Component = disabled ? MenuDisabledLink : ReachMenuLink
 
-    return <Component ref={reference} {...props} className={classNames(styles.dropdownItem, className)} />
+    return (
+        <Component ref={reference} {...props} className={classNames(styles.dropdownItem, className)}>
+            <span className={styles.dropdownItemContent}>{children}</span>
+        </Component>
+    )
 }) as ForwardReferenceComponent<'a', MenuLinkProps>

client/wildcard/src/components/Menu/MenuList.tsx

@@ -46,5 +46,6 @@ const Popover = React.forwardRef(({ popoverPosition, ...props }, reference) => (
         position={popoverPosition}
         focusLocked={false}
         className={classNames('py-1', props.className)}
+        keepInDOM={true}
     />
 )) as ForwardReferenceComponent<'div', PopoverProps>

client/wildcard/src/components/Popover/components/floating-panel/FloatingPanel.tsx

@@ -22,6 +22,8 @@ export interface FloatingPanelProps extends Omit<Tether, 'target' | 'element'>,
      * outside the dom tree.
      */
     rootRender?: HTMLElement | null
+
+    forceHidden?: boolean
 }
 
 /**
@@ -45,6 +47,7 @@ export const FloatingPanel = forwardRef((props, reference) => {
         targetPadding,
         constraint,
         rootRender,
+        forceHidden,
         ...otherProps
     } = props
 
@@ -72,6 +75,7 @@ export const FloatingPanel = forwardRef((props, reference) => {
             constrainToScrollParents,
             overflowToScrollParents,
             flipping,
+            forceHidden,
         })
 
         return unsubscribe
@@ -90,6 +94,7 @@ export const FloatingPanel = forwardRef((props, reference) => {
         constrainToScrollParents,
         overflowToScrollParents,
         flipping,
+        forceHidden,
     ])
 
     if (strategy === Strategy.Absolute) {

client/wildcard/src/components/Popover/components/popover-content/PopoverContent.tsx

@@ -17,6 +17,7 @@ export interface PopoverContentProps extends Omit<FloatingPanelProps, 'target' |
     isOpen?: boolean
     focusLocked?: boolean
     autoFocus?: boolean
+    keepInDOM?: boolean
 }
 
 export const PopoverContent = forwardRef((props, reference) => {
@@ -28,9 +29,11 @@ export const PopoverContent = forwardRef((props, reference) => {
         as: Component = 'div',
         role = 'dialog',
         'aria-modal': ariaModel = true,
+        keepInDOM = false,
+        // we should let FloatingPanel to control its `hidden` attribute
+        hidden,
         ...otherProps
     } = props
-
     const { isOpen: isOpenContext, targetElement, tailElement, anchor, setOpen } = useContext(PopoverContext)
     const { renderRoot } = useContext(PopoverRoot)
 
@@ -68,7 +71,7 @@ export const PopoverContent = forwardRef((props, reference) => {
         return () => setFocusLock(false)
     }, [autoFocus, focusLocked, tooltipElement])
 
-    if (!isOpenContext && !isOpen) {
+    if (!keepInDOM && !isOpenContext && !isOpen) {
         return null
     }
 
@@ -83,6 +86,7 @@ export const PopoverContent = forwardRef((props, reference) => {
             aria-modal={ariaModel}
             rootRender={renderRoot}
             className={classNames(styles.popover, otherProps.className)}
+            forceHidden={keepInDOM && !isOpenContext && !isOpen}
         >
             {focusLocked ? (
                 <FocusLock disabled={!focusLock} returnFocus={true}>

client/wildcard/src/components/Popover/tether/services/tether-render.ts

@@ -36,7 +36,7 @@ export function render(tether: Tether, eventTarget: HTMLElement | null, preserve
     const layout = getLayout(tether)
     const state = getState(layout)
 
-    if (state === null || !isVisible(tether.target)) {
+    if (state === null || !isVisible(tether.target) || tether.forceHidden) {
         setVisibility(tether.element, false)
         setVisibility((tether.marker as HTMLElement) ?? null, false)
 

client/wildcard/src/components/Popover/tether/services/types.ts

@@ -55,6 +55,8 @@ export interface Tether {
 
     overflowToScrollParents?: boolean
     constrainToScrollParents?: boolean
+
+    forceHidden?: boolean
 }
 
 export type MarkerElement = HTMLElement | SVGElement