UnderlineNav2: expose aria-current in the API and enable to use it fo…

…r select status (#2618) * UnderlineNav2: expose aria-current as an API and enable to use it for select status * remove selected * Update UnderlineNav docs (#2608) * update docs and code comments * add changeset * remove invalid comment * take other aria-current options into account when checking selected state * Apply suggestions from code review Co-authored-by: Cole Bemis <colebemis@github.com> * linting fix and extend interaction test for a case that failed to catch Co-authored-by: Cole Bemis <colebemis@github.com>
primer · Nov 29, 2022 · 0468315 · 0468315
1 parent 5ec5772
commit 0468315
Show file tree

Hide file tree

Showing 11 changed files with 110 additions and 50 deletions.
diff --git a/.changeset/light-lemons-shave.md b/.changeset/light-lemons-shave.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+UnderlineNav2: expose aria-current in the API to enable it to be used as the selected status of UnderlineNav.Item
diff --git a/docs/content/drafts/UnderlineNav2.mdx b/docs/content/drafts/UnderlineNav2.mdx
@@ -4,7 +4,7 @@ componentId: underline_nav_2
 status: Draft
 description: Use an underlined nav to allow tab like navigation with overflow behaviour in your UI.
 source: https://github.com/primer/react/tree/main/src/UnderlineNav2
-storybook: '/react/storybook/?path=/story/components-underlinenav'
+storybook: '/react/storybook/?path=/story/drafts-components-underlinenav--playground'
 ---
 
 ```js
@@ -17,7 +17,7 @@ import {UnderlineNav} from '@primer/react/drafts'
 
 ```jsx live drafts
 <UnderlineNav aria-label="Repository">
-  <UnderlineNav.Item selected>Code</UnderlineNav.Item>
+  <UnderlineNav.Item aria-current="page">Code</UnderlineNav.Item>
   <UnderlineNav.Item>Issues</UnderlineNav.Item>
   <UnderlineNav.Item>Pull Requests</UnderlineNav.Item>
 </UnderlineNav>
@@ -27,7 +27,7 @@ import {UnderlineNav} from '@primer/react/drafts'
 
 ```jsx live drafts
 <UnderlineNav aria-label="Repository">
-  <UnderlineNav.Item selected icon={CodeIcon}>
+  <UnderlineNav.Item aria-current="page" icon={CodeIcon}>
     Code
   </UnderlineNav.Item>
   <UnderlineNav.Item icon={IssueOpenedIcon} counter={30}>
@@ -71,7 +71,7 @@ const Navigation = () => {
           <UnderlineNav.Item
             key={item.navigation}
             icon={item.icon}
-            selected={index === selectedIndex}
+            aria-current={index === selectedIndex ? 'page' : undefined}
             onSelect={e => {
               setSelectedIndex(index)
               e.preventDefault()
@@ -92,7 +92,7 @@ render(<Navigation />)
 
 ```jsx live drafts
 <UnderlineNav aria-label="Repository" loadingCounters={true}>
-  <UnderlineNav.Item counter={4} selected>
+  <UnderlineNav.Item counter={4} aria-current="page">
     Code
   </UnderlineNav.Item>
   <UnderlineNav.Item counter={44}>Issues</UnderlineNav.Item>
@@ -108,7 +108,7 @@ import {UnderlineNav} from '@primer/react/drafts'
 const Navigation = () => {
   return (
     <UnderlineNav aria-label="Repository">
-      <UnderlineNav.Item as={Link} to="code" counter={4} selected>
+      <UnderlineNav.Item as={Link} to="code" counter={4} aria-current="page">
         Code
       </UnderlineNav.Item>
       <UnderlineNav.Item counter={44} as={Link} to="issues">
@@ -127,7 +127,11 @@ const Navigation = () => {
 ### UnderlineNav
 
 <PropsTable>
-  <PropsTableRow name="children" required type="UnderlineNav.Item[]" />
+  <PropsTableRow
+    name="afterSelect"
+    type="(event) => void"
+    description="The handler that gets called when a nav link child is selected"
+  />
   <PropsTableRow
     name="aria-label"
     type="string"
@@ -136,42 +140,59 @@ const Navigation = () => {
         'Scroll ${aria-label} left/right')
      "
   />
+  <PropsTableRow name="children" required type="UnderlineNav.Item[]" />
   <PropsTableRow
     name="loadingCounters"
     type="boolean"
     defaultValue="false"
     description="Whether the navigation items are in loading state. Component waits for all the counters to finish loading to prevent multiple layout shifts."
   />
-  <PropsTableRow
-    name="afterSelect"
-    type="(event) => void"
-    description="The handler that gets called when a nav link child is selected"
-  />
   <PropsTableSxRow />
 </PropsTable>
 
 ### UnderlineNav.Item
 
 <PropsTable>
+  <PropsTableRow
+    name="aria-current"
+    type={`| 'page'
+| 'step'
+| 'location'
+| 'date'
+| 'time'
+| true
+| false`}
+    defaultValue="false"
+    description={
+      <>
+        Set <InlineCode>aria-current</InlineCode> to <InlineCode>"page"</InlineCode> to indicate that the item
+        represents the current page. Set <InlineCode>aria-current</InlineCode> to <InlineCode>"location"</InlineCode> to
+        indicate that the item represents the current location on a page. For more information about{' '}
+        <InlineCode>aria-current</InlineCode>, see{' '}
+        <Link href="https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-current">MDN</Link>.
+      </>
+    }
+  />
+
+  <PropsTableAsRow defaultElementType="a" />
+  <PropsTableRow name="counter" type="number" description="The number to display in the counter label." />
   <PropsTableRow
     name="href"
     type="string"
     description="The URL that the item navigates to. 'href' is passed to the underlying '<a>' element. If 'as' is specified, the component may need different props and 'href' is ignored. (Required prop for the react router is 'to' for example)"
   />
   <PropsTableRow name="icon" type="Component" description="The leading icon comes before item label" />
-  <PropsTableRow name="selected" type="boolean" description="Whether the link is selected" />
   <PropsTableRow
     name="onSelect"
     type="(event) => void"
     description="The handler that gets called when a nav link is selected. For example, a manual route binding can be done here(I.e. 'navigate(href)' for the react router.)"
   />
-  <PropsTableRow
-    name="as"
-    type="string | React.ElementType"
-    defaultValue="a"
-    description="The underlying element to render — either a HTML element name or a React component."
+  <PropsTableBasePropRows
+    passthroughPropsLink={
+      <Link href="https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#Attributes">MDN</Link>
+    }
+    elementType="a"
   />
-  <PropsTableSxRow />
 </PropsTable>
 
 ## Status
@@ -184,9 +205,9 @@ const Navigation = () => {
     adaptsToScreenSizes: true,
     fullTestCoverage: false,
     usedInProduction: false,
-    usageExamplesDocumented: false,
-    hasStorybookStories: false,
-    designReviewed: false,
+    usageExamplesDocumented: true,
+    hasStorybookStories: true,
+    designReviewed: true,
     a11yReviewed: false,
     stableApi: false,
     addressedApiFeedback: false,

diff --git a/src/UnderlineNav2/UnderlineNav.Item.stories.tsx b/src/UnderlineNav2/UnderlineNav.Item.stories.tsx
@@ -39,13 +39,9 @@ export default {
   }
 } as Meta<typeof UnderlineNavItem>
 
-// UnderlineNav.Item controls only work on the "Docs" tab. Because UnderlineNav children don't get re-rendered when they are changed.
-// This is an intentional behaviour of UnderlineNav for keeping a selected menu item visible. I will update here once I find a better solution.
-// In the meantime, you can use the "Docs" tab to see the controls.
-
 export const Playground: Story = args => {
   return (
-    <UnderlineNavItem selected {...args}>
+    <UnderlineNavItem aria-current="page" {...args}>
       {args.children}
     </UnderlineNavItem>
   )

diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx
@@ -60,7 +60,7 @@ const ResponsiveUnderlineNav = ({
           <UnderlineNav.Item
             key={item.navigation}
             icon={item.icon}
-            selected={item.navigation === selectedItemText}
+            aria-current={item.navigation === selectedItemText ? 'page' : undefined}
             counter={item.counter}
           >
             {item.navigation}
@@ -119,7 +119,7 @@ describe('UnderlineNav', () => {
       <UnderlineNav aria-label="Test Navigation">
         <UnderlineNav.Item onSelect={onSelect}>Item 1</UnderlineNav.Item>
         <UnderlineNav.Item onSelect={onSelect}>Item 2</UnderlineNav.Item>
-        <UnderlineNav.Item selected onSelect={onSelect}>
+        <UnderlineNav.Item aria-current="page" onSelect={onSelect}>
           Item 3
         </UnderlineNav.Item>
       </UnderlineNav>

diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx
@@ -1,4 +1,4 @@
-import React, {useRef, forwardRef, useCallback, useState, MutableRefObject, RefObject} from 'react'
+import React, {useRef, forwardRef, useCallback, useState, MutableRefObject, RefObject, useEffect} from 'react'
 import Box from '../Box'
 import sx, {merge, BetterSystemStyleObject, SxProp} from '../sx'
 import {UnderlineNavContext} from './UnderlineNavContext'
@@ -20,11 +20,12 @@ import {useSSRSafeId} from '@react-aria/ssr'
 export type UnderlineNavProps = {
   'aria-label'?: React.AriaAttributes['aria-label']
   as?: React.ElementType
-  align?: 'right'
   sx?: SxProp
+  // cariant and align are currently not in used. Keeping here until some design explorations are finalized.
   variant?: 'default' | 'small'
+  align?: 'right'
   /**
-   * loading state for all counters (to prevent multiple layout shifts)
+   * loading state for all counters. It displays loading animation for individual counters (UnderlineNav.Item) until all are resolved. It is needed to prevent multiple layout shift.
    */
   loadingCounters?: boolean
   afterSelect?: (event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => void
@@ -89,8 +90,10 @@ const overflowEffect = (
       if (index < numberOfListItems) {
         items.push(child)
       } else {
+        const ariaCurrent = child.props['aria-current']
+        const isCurrent = Boolean(ariaCurrent) && ariaCurrent !== 'false'
         // We need to make sure to keep the selected item always visible.
-        if (child.props.selected) {
+        if (isCurrent) {
           // If selected item couldn't make in 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.
@@ -218,6 +221,29 @@ export const UnderlineNav = forwardRef(
       actions: []
     })
 
+    /*
+     * This is needed to make sure responsiveProps.items and ResponsiveProps.actions are updated when children are changed
+     * Particually when an item is selected. It adds 'aria-current="page"' attribute to the child and we need to make sure
+     * responsiveProps.items and ResponsiveProps.actions are updated with that attribute
+     */
+    useEffect(() => {
+      const childArray = getValidChildren(children)
+
+      const updatedItems = responsiveProps.items.map(item => {
+        return childArray.find(child => child.key === item.key) || item
+      })
+
+      const updatedActions = responsiveProps.actions.map(action => {
+        return childArray.find(child => child.key === action.key) || action
+      })
+
+      setResponsiveProps({
+        items: updatedItems,
+        actions: updatedActions
+      })
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [children])
+
     const updateListAndMenu = useCallback((props: ResponsiveProps, displayIcons: boolean) => {
       setResponsiveProps(props)
       setIconsVisible(displayIcons)
@@ -332,7 +358,7 @@ export const UnderlineNav = forwardRef(
                   {actions.map((action, index) => {
                     const {children: actionElementChildren, ...actionElementProps} = action.props
                     return (
-                      <Box key={index} as="li">
+                      <Box key={actionElementChildren} as="li">
                         <ActionList.Item
                           {...actionElementProps}
                           as={action.props.as || 'a'}

diff --git a/src/UnderlineNav2/UnderlineNav2.stories.tsx b/src/UnderlineNav2/UnderlineNav2.stories.tsx
@@ -40,7 +40,7 @@ export const Playground: Story = args => {
   return (
     <UnderlineNav {...args}>
       {children.map((child: string, index: number) => (
-        <UnderlineNavItem key={index} href="#" selected={index === 0}>
+        <UnderlineNavItem key={index} href="#" aria-current={index === 0 ? 'page' : undefined}>
           {child}
         </UnderlineNavItem>
       ))}

diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx
@@ -24,21 +24,24 @@ type LinkProps = {
 
 export type UnderlineNavItemProps = {
   /**
-   * Primary content for an NavLink
+   * Primary content for an UnderlineNav
    */
   children?: React.ReactNode
   /**
    * Callback that will trigger both on click selection and keyboard selection.
    */
   onSelect?: (event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => void
   /**
-   * Is the `Link` is currently selected?
+   * Is `UnderlineNav.Item` current page?
    */
-  selected?: boolean
+  'aria-current'?: 'page' | 'step' | 'location' | 'date' | 'time' | 'true' | 'false' | boolean
   /**
    *  Icon before the text
    */
   icon?: React.FunctionComponent<IconProps>
+  /**
+   * Renders `UnderlineNav.Item` as given component
+   **/
   as?: React.ElementType
   /**
    * Counter
@@ -56,7 +59,7 @@ export const UnderlineNavItem = forwardRef(
       children,
       counter,
       onSelect,
-      selected: preSelected = false,
+      'aria-current': ariaCurrent,
       icon: Icon,
       ...props
     },
@@ -100,7 +103,10 @@ export const UnderlineNavItem = forwardRef(
 
         setChildrenWidth({text, width: domRect.width})
         setNoIconChildrenWidth({text, width: domRect.width - iconWidthWithMargin})
-        preSelected && selectedLink === undefined && setSelectedLink(ref as RefObject<HTMLElement>)
+
+        if (selectedLink === undefined && Boolean(ariaCurrent) && ariaCurrent !== 'false') {
+          setSelectedLink(ref as RefObject<HTMLElement>)
+        }
 
         // Only runs when a menu item is selected (swapping the menu item with the list item to keep it visible)
         if (selectedLinkText === text) {
@@ -111,7 +117,7 @@ export const UnderlineNavItem = forwardRef(
       }
     }, [
       ref,
-      preSelected,
+      ariaCurrent,
       selectedLink,
       selectedLinkText,
       setSelectedLinkText,
@@ -150,7 +156,7 @@ export const UnderlineNavItem = forwardRef(
           href={href}
           onKeyPress={keyPressHandler}
           onClick={clickHandler}
-          {...(selectedLink === ref ? {'aria-current': 'page'} : {})}
+          aria-current={ariaCurrent}
           sx={merge(getLinkStyles(theme, {variant}, selectedLink, ref), sxProp as SxProp)}
           {...props}
           ref={ref}

diff --git a/src/UnderlineNav2/__snapshots__/UnderlineNav.test.tsx.snap b/src/UnderlineNav2/__snapshots__/UnderlineNav.test.tsx.snap
@@ -206,6 +206,7 @@ exports[`UnderlineNav renders consistently 1`] = `
         className="c3"
       >
         <a
+          aria-current="page"
           className="c4"
           href="#"
           onClick={[Function]}

diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx
@@ -47,7 +47,7 @@ export const PullRequestPage = () => {
         </Box>
       </Box>
       <UnderlineNav aria-label="Pull Request">
-        <UnderlineNav.Item icon={CommentDiscussionIcon} counter="0" selected>
+        <UnderlineNav.Item icon={CommentDiscussionIcon} counter="0" aria-current="page">
           Conversation
         </UnderlineNav.Item>
         <UnderlineNav.Item counter={3} icon={CommitIcon}>
@@ -85,7 +85,7 @@ export const ReposPage = () => {
         <UnderlineNav.Item
           key={item.navigation}
           icon={item.icon}
-          selected={index === selectedIndex}
+          aria-current={index === selectedIndex ? 'page' : undefined}
           onSelect={event => {
             event.preventDefault()
             setSelectedIndex(index)
@@ -149,7 +149,7 @@ export const ProfilePage = () => {
             <UnderlineNav.Item
               key={item.navigation}
               icon={item.icon}
-              selected={index === selectedIndex}
+              aria-current={index === selectedIndex ? 'page' : undefined}
               onSelect={event => {
                 event.preventDefault()
                 setSelectedIndex(index)