[UnderlineNav2]: Introduce disclosure widget pattern (#2466)

* Disclosure pattern implementation * use token name for colours instead of accessing them from theme Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com> * code review feedback <3 * [UnderlineNav2]: Always show at least 2 items in the overflow menu (A11y remediations) (#2471) * Display at least two items in the menu * add changeset * [UnderlineNav2]: Add visually hidden heading where `aria-label` is present (#2470) * Add visually hidden heading * add changeset and a test * append 'navigation' to the aria-label string * use prop for 'as' * add changeset Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
primer · Oct 26, 2022 · 9ff856d · 9ff856d
1 parent 914dbf2
commit 9ff856d
Show file tree

Hide file tree

Showing 10 changed files with 167 additions and 75 deletions.
diff --git a/.changeset/curly-hornets-bathe.md b/.changeset/curly-hornets-bathe.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+UnderlineNav2: Introduce disclosure widget pattern
diff --git a/.changeset/moody-garlics-know.md b/.changeset/moody-garlics-know.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+UnderlineNav2: Always show at least two items in the overflow menu
diff --git a/.changeset/twelve-spoons-walk.md b/.changeset/twelve-spoons-walk.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+UnderlineNav2: Render a visually hidden heading for screen readers when aria-label is present
diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx
@@ -112,4 +112,10 @@ describe('UnderlineNav', () => {
     expect(loadingCounter?.className).toContain('LoadingCounter')
     expect(loadingCounter?.textContent).toBe('')
   })
+  it('renders a visually hidden h2 heading for screen readers when aria-label is present', () => {
+    const {container} = render(<ResponsiveUnderlineNav />)
+    const heading = container.getElementsByTagName('h2')[0]
+    expect(heading.className).toContain('VisuallyHidden')
+    expect(heading.textContent).toBe('Repository navigation')
+  })
 })
diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx
@@ -2,16 +2,22 @@ import React, {useRef, forwardRef, useCallback, useState, MutableRefObject, RefO
 import Box from '../Box'
 import sx, {merge, BetterSystemStyleObject, SxProp} from '../sx'
 import {UnderlineNavContext} from './UnderlineNavContext'
-import {ActionMenu} from '../ActionMenu'
-import {ActionList} from '../ActionList'
 import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver'
 import CounterLabel from '../CounterLabel'
 import {useTheme} from '../ThemeProvider'
 import {ChildWidthArray, ResponsiveProps} from './types'
-
-import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, menuItemStyles, GAP} from './styles'
+import VisuallyHidden from '../_VisuallyHidden'
+import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, menuStyles, menuItemStyles, GAP} from './styles'
 import styled from 'styled-components'
 import {LoadingCounter} from './LoadingCounter'
+import {Button} from '../Button'
+import {useFocusZone} from '../hooks/useFocusZone'
+import {FocusKeys} from '@primer/behaviors'
+import {TriangleDownIcon} from '@primer/octicons-react'
+import {useOnEscapePress} from '../hooks/useOnEscapePress'
+import {useOnOutsideClick} from '../hooks/useOnOutsideClick'
+import {ActionList} from '../ActionList'
+import {useSSRSafeId} from '@react-aria/ssr'
 
 export type UnderlineNavProps = {
   'aria-label'?: React.AriaAttributes['aria-label']
@@ -63,23 +69,34 @@ const overflowEffect = (
   const items: Array<React.ReactElement> = []
   const actions: Array<React.ReactElement> = []
 
-  // For fine pointer devices, first we check if we can fit all the items with icons
+  // First, we check if we can fit all the items with their icons
   if (childArray.length <= numberOfItemsPossible) {
     items.push(...childArray)
   } else if (childArray.length <= numberOfItemsWithoutIconPossible) {
-    // if we can't fit all the items with icons, we check if we can fit all the items without icons
+    // if we can't fit all the items with their icons, we check if we can fit all the items without their icons
     iconsVisible = false
     items.push(...childArray)
   } else {
-    // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu
+    // if we can't fit all the items without their icons, we keep the icons hidden and show the ones that doesn't fit into the list in the overflow menu
     iconsVisible = false
+
+    /* Below is an accessibiility requirement. Never show only one item in the overflow menu.
+     * If there is only one item left to display in the overflow menu according to the calculation,
+     * we need to pull another item from the list into the overflow menu.
+     */
+    const numberOfItemsInMenu = childArray.length - numberOfItemsPossibleWithMoreMenu
+    const numberOfListItems =
+      numberOfItemsInMenu === 1 ? numberOfItemsPossibleWithMoreMenu - 1 : numberOfItemsPossibleWithMoreMenu
+
     for (const [index, child] of childArray.entries()) {
-      if (index < numberOfItemsPossibleWithMoreMenu) {
+      if (index < numberOfListItems) {
         items.push(child)
-        // keeping selected item always visible.
+        // We need to make sure to keep the selected item always visible.
       } else if (child.props.selected) {
-        // If selected item's index couldn't make the list, we swap it with the last item in the list.
-        const propsectiveAction = items.splice(numberOfItemsPossibleWithMoreMenu - 1, 1, child)[0]
+        // If selected item can't make it to the list, we swap it with the last item in the list.
+        const indexToReplaceAt = numberOfListItems - 1 // because we are replacing the last item in the list
+        // splice method modifies the array by removing 1 item here at the given index and replace it with the "child" element then returns the removed item.
+        const propsectiveAction = items.splice(indexToReplaceAt, 1, child)[0]
         actions.push(propsectiveAction)
       } else {
         actions.push(child)
@@ -129,6 +146,9 @@ export const UnderlineNav = forwardRef(
     const navRef = (forwardedRef ?? backupRef) as MutableRefObject<HTMLElement>
     const listRef = useRef<HTMLUListElement>(null)
     const moreMenuRef = useRef<HTMLLIElement>(null)
+    const moreMenuBtnRef = useRef<HTMLButtonElement>(null)
+    const containerRef = React.useRef<HTMLUListElement>(null)
+    const disclosureWidgetId = useSSRSafeId()
 
     const {theme} = useTheme()
 
@@ -187,13 +207,12 @@ export const UnderlineNav = forwardRef(
       React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement> | null
     >(null)
 
-    const [asNavItem, setAsNavItem] = useState('a')
-
     const [iconsVisible, setIconsVisible] = useState<boolean>(true)
 
     const afterSelectHandler = (event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => {
       if (!event.defaultPrevented) {
         if (typeof afterSelect === 'function') afterSelect(event)
+        closeOverlay()
       }
     }
 
@@ -235,6 +254,39 @@ export const UnderlineNav = forwardRef(
       // eslint-disable-next-line no-console
       console.warn('Use the `aria-label` prop to provide an accessible label for assistive technology')
     }
+    const [isWidgetOpen, setIsWidgetOpen] = useState(false)
+
+    const closeOverlay = React.useCallback(() => {
+      setIsWidgetOpen(false)
+    }, [setIsWidgetOpen])
+
+    const focusOnMoreMenuBtn = React.useCallback(() => {
+      moreMenuBtnRef.current?.focus()
+    }, [])
+
+    useFocusZone({
+      containerRef: backupRef,
+      bindKeys: FocusKeys.ArrowVertical | FocusKeys.ArrowHorizontal | FocusKeys.HomeAndEnd | FocusKeys.Tab
+    })
+
+    useOnEscapePress(
+      (event: KeyboardEvent) => {
+        if (isWidgetOpen) {
+          event.preventDefault()
+          closeOverlay()
+          focusOnMoreMenuBtn()
+        }
+      },
+      [isWidgetOpen]
+    )
+
+    useOnOutsideClick({onClickOutside: closeOverlay, containerRef, ignoreClickRefs: [moreMenuBtnRef]})
+    const onAnchorClick = useCallback((event: React.MouseEvent<HTMLButtonElement>) => {
+      if (event.defaultPrevented || event.button !== 0) {
+        return
+      }
+      setIsWidgetOpen(isWidgetOpen => !isWidgetOpen)
+    }, [])
 
     return (
       <UnderlineNavContext.Provider
@@ -246,14 +298,14 @@ export const UnderlineNav = forwardRef(
           setSelectedLink,
           selectedLinkText,
           setSelectedLinkText,
-          setAsNavItem,
           selectEvent,
           afterSelect: afterSelectHandler,
           variant,
           loadingCounters,
           iconsVisible
         }}
       >
+        {ariaLabel && <VisuallyHidden as="h2">{`${ariaLabel} navigation`}</VisuallyHidden>}
         <Box
           as={as}
           sx={merge<BetterSystemStyleObject>(getNavStyles(theme, {align}), sxProp)}
@@ -265,46 +317,54 @@ export const UnderlineNav = forwardRef(
             {actions.length > 0 && (
               <MoreMenuListItem ref={moreMenuRef}>
                 <Box sx={getDividerStyle(theme)}></Box>
-                <ActionMenu>
-                  <ActionMenu.Button sx={moreBtnStyles}>More</ActionMenu.Button>
-                  <ActionMenu.Overlay align="end">
-                    <ActionList selectionVariant="single">
-                      {actions.map((action, index) => {
-                        const {children: actionElementChildren, ...actionElementProps} = action.props
-                        return (
-                          <Box key={index} as="li">
-                            <ActionList.Item
-                              sx={menuItemStyles}
-                              as={asNavItem}
-                              {...actionElementProps}
-                              onSelect={(
-                                event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>
-                              ) => {
-                                swapMenuItemWithListItem(action, index, event, updateListAndMenu)
-                                setSelectEvent(event)
-                              }}
-                            >
-                              <Box
-                                as="span"
-                                sx={{display: 'flex', alignItems: 'center', justifyContent: 'space-between'}}
-                              >
-                                {actionElementChildren}
-
-                                {loadingCounters ? (
-                                  <LoadingCounter />
-                                ) : (
-                                  actionElementProps.counter !== undefined && (
-                                    <CounterLabel>{actionElementProps.counter}</CounterLabel>
-                                  )
-                                )}
-                              </Box>
-                            </ActionList.Item>
+                <Button
+                  ref={moreMenuBtnRef}
+                  sx={moreBtnStyles}
+                  aria-controls={disclosureWidgetId}
+                  aria-expanded={isWidgetOpen}
+                  onClick={onAnchorClick}
+                  trailingIcon={TriangleDownIcon}
+                >
+                  More
+                </Button>
+                <ActionList
+                  selectionVariant="single"
+                  ref={containerRef}
+                  id={disclosureWidgetId}
+                  sx={menuStyles}
+                  style={{display: isWidgetOpen ? 'block' : 'none'}}
+                >
+                  {actions.map((action, index) => {
+                    const {children: actionElementChildren, ...actionElementProps} = action.props
+                    return (
+                      <Box key={index} as="li">
+                        <ActionList.Item
+                          {...actionElementProps}
+                          as={action.props.as || 'a'}
+                          sx={menuItemStyles}
+                          onSelect={(event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => {
+                            swapMenuItemWithListItem(action, index, event, updateListAndMenu)
+                            setSelectEvent(event)
+                            closeOverlay()
+                            focusOnMoreMenuBtn()
+                          }}
+                        >
+                          <Box as="span" sx={{display: 'flex', alignItems: 'center', justifyContent: 'space-between'}}>
+                            {actionElementChildren}
+
+                            {loadingCounters ? (
+                              <LoadingCounter />
+                            ) : (
+                              actionElementProps.counter !== undefined && (
+                                <CounterLabel>{actionElementProps.counter}</CounterLabel>
+                              )
+                            )}
                           </Box>
-                        )
-                      })}
-                    </ActionList>
-                  </ActionMenu.Overlay>
-                </ActionMenu>
+                        </ActionList.Item>
+                      </Box>
+                    )
+                  })}
+                </ActionList>
               </MoreMenuListItem>
             )}
           </NavigationList>

diff --git a/src/UnderlineNav2/UnderlineNavContext.tsx b/src/UnderlineNav2/UnderlineNavContext.tsx
@@ -10,7 +10,6 @@ export const UnderlineNavContext = createContext<{
   selectedLinkText: string
   setSelectedLinkText: React.Dispatch<React.SetStateAction<string>>
   selectEvent: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement> | null
-  setAsNavItem: React.Dispatch<React.SetStateAction<string>>
   afterSelect?: (event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => void
   variant: 'default' | 'small'
   loadingCounters: boolean
@@ -24,7 +23,6 @@ export const UnderlineNavContext = createContext<{
   selectedLinkText: '',
   setSelectedLinkText: () => null,
   selectEvent: null,
-  setAsNavItem: () => null,
   variant: 'default',
   loadingCounters: false,
   iconsVisible: true

diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx
@@ -72,7 +72,6 @@ export const UnderlineNavItem = forwardRef(
       selectedLinkText,
       setSelectedLinkText,
       selectEvent,
-      setAsNavItem,
       afterSelect,
       variant,
       loadingCounters,
@@ -107,7 +106,6 @@ export const UnderlineNavItem = forwardRef(
         if (typeof onSelect === 'function' && selectEvent !== null) onSelect(selectEvent)
         setSelectedLinkText('')
       }
-      setAsNavItem(Component)
     }, [
       ref,
       preSelected,
@@ -118,9 +116,7 @@ export const UnderlineNavItem = forwardRef(
       setChildrenWidth,
       setNoIconChildrenWidth,
       onSelect,
-      selectEvent,
-      setAsNavItem,
-      Component
+      selectEvent
     ])
 
     const keyPressHandler = React.useCallback(

diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx
@@ -72,16 +72,16 @@ export const withCounterLabels = () => {
   )
 }
 
-const items: {navigation: string; icon: React.FC<IconProps>; counter?: number | string}[] = [
-  {navigation: 'Code', icon: CodeIcon},
-  {navigation: 'Issues', icon: IssueOpenedIcon, counter: '12K'},
-  {navigation: 'Pull Requests', icon: GitPullRequestIcon, counter: 13},
-  {navigation: 'Discussions', icon: CommentDiscussionIcon, counter: 5},
-  {navigation: 'Actions', icon: PlayIcon, counter: 4},
-  {navigation: 'Projects', icon: ProjectIcon, counter: 9},
-  {navigation: 'Insights', icon: GraphIcon, counter: '0'},
-  {navigation: 'Settings', icon: GearIcon, counter: 10},
-  {navigation: 'Security', icon: ShieldLockIcon}
+const items: {navigation: string; icon: React.FC<IconProps>; counter?: number | string; href?: string}[] = [
+  {navigation: 'Code', icon: CodeIcon, href: '#code'},
+  {navigation: 'Issues', icon: IssueOpenedIcon, counter: '12K', href: '#issues'},
+  {navigation: 'Pull Requests', icon: GitPullRequestIcon, counter: 13, href: '#pull-requests'},
+  {navigation: 'Discussions', icon: CommentDiscussionIcon, counter: 5, href: '#discussions'},
+  {navigation: 'Actions', icon: PlayIcon, counter: 4, href: '#actions'},
+  {navigation: 'Projects', icon: ProjectIcon, counter: 9, href: '#projects'},
+  {navigation: 'Insights', icon: GraphIcon, counter: '0', href: '#insights'},
+  {navigation: 'Settings', icon: GearIcon, counter: 10, href: '#settings'},
+  {navigation: 'Security', icon: ShieldLockIcon, href: '#security'}
 ]
 
 export const InternalResponsiveNav = () => {
@@ -96,6 +96,7 @@ export const InternalResponsiveNav = () => {
           selected={index === selectedIndex}
           onSelect={() => setSelectedIndex(index)}
           counter={item.counter}
+          href={item.href}
         >
           {item.navigation}
         </UnderlineNav.Item>

diff --git a/src/UnderlineNav2/styles.ts b/src/UnderlineNav2/styles.ts
@@ -39,8 +39,7 @@ export const getNavStyles = (theme?: Theme, props?: Partial<Pick<UnderlineNavPro
   borderBottom: '1px solid',
   borderBottomColor: `${theme?.colors.border.muted}`,
   align: 'row',
-  alignItems: 'center',
-  position: 'relative'
+  alignItems: 'center'
 })
 
 export const ulStyles = {
@@ -52,7 +51,8 @@ export const ulStyles = {
   margin: 0,
   marginBottom: '-1px',
   alignItems: 'center',
-  gap: `${GAP}px`
+  gap: `${GAP}px`,
+  position: 'relative'
 }
 
 export const getDividerStyle = (theme?: Theme) => ({
@@ -71,7 +71,10 @@ export const moreBtnStyles = {
   fontWeight: 'normal',
   boxShadow: 'none',
   paddingY: 1,
-  paddingX: 2
+  paddingX: 2,
+  '& > span[data-component="trailingIcon"]': {
+    marginLeft: 0
+  }
 }
 
 export const getLinkStyles = (
@@ -142,3 +145,16 @@ export const menuItemStyles = {
   // To reset the style when the menu items are rendered as react router links
   textDecoration: 'none'
 }
+
+export const menuStyles = {
+  position: 'absolute',
+  top: '90%',
+  right: '0',
+  boxShadow: '0 1px 3px rgba(0, 0, 0, 0.12), 0 1px 2px rgba(0, 0, 0, 0.24)',
+  borderRadius: '12px',
+  backgroundColor: 'canvas.overlay',
+  listStyle: 'none',
+  // Values are from ActionMenu
+  minWidth: '192px',
+  maxWidth: '640px'
+}