NavList.Item: Add as prop (#2076)

* Add as prop to NavList.Item

* Add test case for react router link with subnav

* Use DOM APIs to determine if subnav contains current item

* Update snapshots

* Remove "not implemented" warnings

* Create dry-feet-attack.md

* Fix merge issues

* Update snapshot

.changeset/dry-feet-attack.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Draft `NavList.Item` now accepts an `as` prop, allowing it to be rendered as a Next.js or React Router link

docs/content/NavList.mdx

@@ -174,8 +174,6 @@ If a `NavList.Item` contains a `NavList.SubNav`, the `NavList.Item` will render
 
 ### With React Router
 
-<Note variant="danger">Not implemented yet</Note>
-
 ```jsx
 import {Link, useMatch, useResolvedPath} from 'react-router-dom'
 import {NavList} from '@primer/react'
@@ -203,8 +201,6 @@ function App() {
 
 ### With Next.js
 
-<Note variant="danger">Not implemented yet</Note>
-
 ```jsx
 import {useRouter} from 'next/router'
 import Link from 'next/link'

src/NavList/NavList.stories.tsx

@@ -26,7 +26,7 @@ export const Simple: Story = () => (
   </PageLayout>
 )
 
-export const SubItems: Story = () => (
+export const WithSubItems: Story = () => (
   <PageLayout>
     <PageLayout.Pane position="start">
       <NavList>
@@ -47,4 +47,61 @@ export const SubItems: Story = () => (
   </PageLayout>
 )
 
+type ReactRouterLikeLinkProps = {to: string; children: React.ReactNode}
+const ReactRouterLikeLink = React.forwardRef<HTMLAnchorElement, ReactRouterLikeLinkProps>(({to, ...props}, ref) => {
+  // eslint-disable-next-line jsx-a11y/anchor-has-content
+  return <a ref={ref} href={to} {...props} />
+})
+
+export const WithReactRouterLink = () => (
+  <PageLayout>
+    <PageLayout.Pane position="start">
+      <NavList>
+        <NavList.Item as={ReactRouterLikeLink} to="#" aria-current="page">
+          Item 1
+        </NavList.Item>
+        <NavList.Item as={ReactRouterLikeLink} to="#">
+          Item 2
+        </NavList.Item>
+        <NavList.Item as={ReactRouterLikeLink} to="#">
+          Item 3
+        </NavList.Item>
+      </NavList>
+    </PageLayout.Pane>
+    <PageLayout.Content></PageLayout.Content>
+  </PageLayout>
+)
+
+type NextJSLinkProps = {href: string; children: React.ReactNode}
+
+const NextJSLikeLink = React.forwardRef<HTMLAnchorElement, NextJSLinkProps>(
+  ({href, children}, ref): React.ReactElement => {
+    const child = React.Children.only(children)
+    const childProps = {
+      ref,
+      href
+    }
+    return <>{React.isValidElement(child) ? React.cloneElement(child, childProps) : null}</>
+  }
+)
+
+export const WithNextJSLink = () => (
+  <PageLayout>
+    <PageLayout.Pane position="start">
+      <NavList>
+        <NextJSLikeLink href="#">
+          <NavList.Item aria-current="page">Item 1</NavList.Item>
+        </NextJSLikeLink>
+        <NextJSLikeLink href="#">
+          <NavList.Item>Item 2</NavList.Item>
+        </NextJSLikeLink>
+        <NextJSLikeLink href="#">
+          <NavList.Item>Item 3</NavList.Item>
+        </NextJSLikeLink>
+      </NavList>
+    </PageLayout.Pane>
+    <PageLayout.Content></PageLayout.Content>
+  </PageLayout>
+)
+
 export default meta

src/NavList/NavList.test.tsx

@@ -3,6 +3,26 @@ import React from 'react'
 import {ThemeProvider, SSRProvider} from '..'
 import {NavList} from './NavList'
 
+type ReactRouterLikeLinkProps = {to: string; children: React.ReactNode}
+
+const ReactRouterLikeLink = React.forwardRef<HTMLAnchorElement, ReactRouterLikeLinkProps>(({to, ...props}, ref) => {
+  // eslint-disable-next-line jsx-a11y/anchor-has-content
+  return <a ref={ref} href={to} {...props} />
+})
+
+type NextJSLinkProps = {href: string; children: React.ReactNode}
+
+const NextJSLikeLink = React.forwardRef<HTMLAnchorElement, NextJSLinkProps>(
+  ({href, children}, ref): React.ReactElement => {
+    const child = React.Children.only(children)
+    const childProps = {
+      ref,
+      href
+    }
+    return <>{React.isValidElement(child) ? React.cloneElement(child, childProps) : null}</>
+  }
+)
+
 describe('NavList', () => {
   it('renders a simple list', () => {
     const {container} = render(
@@ -60,6 +80,36 @@ describe('NavList.Item', () => {
     expect(homeLink).toHaveAttribute('aria-current', 'page')
     expect(aboutLink).not.toHaveAttribute('aria-current')
   })
+
+  it('is compatiable with React-Router-like link components', () => {
+    const {getByRole} = render(
+      <NavList>
+        <NavList.Item as={ReactRouterLikeLink} to={'/'} aria-current="page">
+          React Router link
+        </NavList.Item>
+      </NavList>
+    )
+
+    const link = getByRole('link', {name: 'React Router link'})
+
+    expect(link).toHaveAttribute('aria-current', 'page')
+    expect(link).toHaveAttribute('href', '/')
+  })
+
+  it('is compatible with NextJS-like link components', () => {
+    const {getByRole} = render(
+      <NavList>
+        <NextJSLikeLink href="/">
+          <NavList.Item aria-current="page">NextJS link</NavList.Item>
+        </NextJSLikeLink>
+      </NavList>
+    )
+
+    const link = getByRole('link', {name: 'NextJS link'})
+
+    expect(link).toHaveAttribute('href', '/')
+    expect(link).toHaveAttribute('aria-current', 'page')
+  })
 })
 
 describe('NavList.Item with NavList.SubNav', () => {
@@ -227,4 +277,33 @@ describe('NavList.Item with NavList.SubNav', () => {
 
     expect(consoleSpy).toHaveBeenCalled()
   })
+
+  it('is compatiable with React-Router-like link components', () => {
+    function NavLink({href, children}: {href: string; children: React.ReactNode}) {
+      // In a real app, you'd check if the href matches the url of the current page. For testing purposes, we'll use the text of the link to determine if it's current
+      const isCurrent = children === 'Current'
+      return (
+        <NavList.Item as={ReactRouterLikeLink} to={href} aria-current={isCurrent ? 'page' : false}>
+          {children}
+        </NavList.Item>
+      )
+    }
+
+    const {queryByRole} = render(
+      <NavList>
+        <NavLink href="/">Item 1</NavLink>
+        <NavList.Item>
+          Item 2
+          <NavList.SubNav>
+            <NavLink href="/sub-item-1">Current</NavLink>
+            <NavLink href="/sub-item-2">Sub item 2</NavLink>
+          </NavList.SubNav>
+        </NavList.Item>
+        <NavLink href="/">Item 3</NavLink>
+      </NavList>
+    )
+
+    const currentLink = queryByRole('link', {name: 'Current'})
+    expect(currentLink).toBeVisible()
+  })
 })

src/NavList/NavList.tsx

@@ -1,4 +1,5 @@
 import {ChevronDownIcon} from '@primer/octicons-react'
+import {ForwardRefComponent as PolymorphicForwardRefComponent} from '@radix-ui/react-polymorphic'
 import {useSSRSafeId} from '@react-aria/ssr'
 import React, {isValidElement} from 'react'
 import styled from 'styled-components'
@@ -36,9 +37,8 @@ export type NavListItemProps = {
   'aria-current'?: 'page' | 'step' | 'location' | 'date' | 'time' | 'true' | 'false' | boolean
 } & SxProp
 
-// TODO: as prop
 const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
-  ({href, 'aria-current': ariaCurrent, children, sx: sxProp = {}}, ref) => {
+  ({'aria-current': ariaCurrent, children, sx: sxProp = {}, ...props}, ref) => {
     const {depth} = React.useContext(SubNavContext)
 
     // Get SubNav from children
@@ -51,13 +51,8 @@ const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
 
     // Render ItemWithSubNav if SubNav is present
     if (subNav && isValidElement(subNav) && depth < 1) {
-      // Search SubNav children for current Item
-      const currentItem = React.Children.toArray(subNav.props.children).find(
-        child => isValidElement(child) && child.props['aria-current']
-      )
-
       return (
-        <ItemWithSubNav subNav={subNav} subNavContainsCurrentItem={Boolean(currentItem)} sx={sxProp}>
+        <ItemWithSubNav subNav={subNav} sx={sxProp}>
           {childrenWithoutSubNav}
         </ItemWithSubNav>
       )
@@ -66,7 +61,6 @@ const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
     return (
       <ActionList.LinkItem
         ref={ref}
-        href={href}
         aria-current={ariaCurrent}
         active={Boolean(ariaCurrent) && ariaCurrent !== 'false'}
         sx={merge<SxProp['sx']>(
@@ -77,12 +71,13 @@ const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
           },
           sxProp
         )}
+        {...props}
       >
         {children}
       </ActionList.LinkItem>
     )
   }
-)
+) as PolymorphicForwardRefComponent<'a', NavListItemProps>
 
 Item.displayName = 'NavList.Item'
 
@@ -92,36 +87,48 @@ Item.displayName = 'NavList.Item'
 type ItemWithSubNavProps = {
   children: React.ReactNode
   subNav: React.ReactNode
-  subNavContainsCurrentItem: boolean
 } & SxProp
 
-const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: string}>({
+const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: string; isOpen: boolean}>({
   buttonId: '',
-  subNavId: ''
+  subNavId: '',
+  isOpen: false
 })
 
 // TODO: ref prop
 // TODO: Animate open/close transition
-function ItemWithSubNav({children, subNav, subNavContainsCurrentItem, sx: sxProp = {}}: ItemWithSubNavProps) {
+function ItemWithSubNav({children, subNav, sx: sxProp = {}}: ItemWithSubNavProps) {
   const buttonId = useSSRSafeId()
   const subNavId = useSSRSafeId()
-  // SubNav starts open if current item is in it
-  const [isOpen, setIsOpen] = React.useState(subNavContainsCurrentItem)
+  const [isOpen, setIsOpen] = React.useState(false)
+  const subNavRef = React.useRef<HTMLDivElement>(null)
+  const [containsCurrentItem, setContainsCurrentItem] = React.useState(false)
+
+  React.useLayoutEffect(() => {
+    if (subNavRef.current) {
+      // Check if SubNav contains current item
+      const currentItem = subNavRef.current.querySelector('[aria-current]')
+      if (currentItem && currentItem.getAttribute('aria-current') !== 'false') {
+        setContainsCurrentItem(true)
+        setIsOpen(true)
+      }
+    }
+  }, [subNav])
 
   return (
-    <ItemWithSubNavContext.Provider value={{buttonId, subNavId}}>
+    <ItemWithSubNavContext.Provider value={{buttonId, subNavId, isOpen}}>
       <Box as="li" aria-labelledby={buttonId} sx={{listStyle: 'none'}}>
         <ActionList.Item
           as="button"
           id={buttonId}
           aria-expanded={isOpen}
           aria-controls={subNavId}
           // When the subNav is closed, how should we indicated that the subNav contains the current item?
-          active={!isOpen && subNavContainsCurrentItem}
+          active={!isOpen && containsCurrentItem}
           onClick={() => setIsOpen(open => !open)}
           sx={merge<SxProp['sx']>(
             {
-              fontWeight: subNavContainsCurrentItem ? 'bold' : null // Parent item is bold if any of it's sub-items are current
+              fontWeight: containsCurrentItem ? 'bold' : null // Parent item is bold if any of it's sub-items are current
             },
             sxProp
           )}
@@ -138,7 +145,7 @@ function ItemWithSubNav({children, subNav, subNavContainsCurrentItem, sx: sxProp
           </ActionList.TrailingVisual>
         </ActionList.Item>
 
-        {isOpen ? subNav : null}
+        <div ref={subNavRef}>{subNav}</div>
       </Box>
     </ItemWithSubNavContext.Provider>
   )
@@ -156,7 +163,7 @@ const SubNavContext = React.createContext<{depth: number}>({depth: 0})
 // TODO: ref prop
 // NOTE: SubNav must be a direct child of an Item
 const SubNav = ({children, sx: sxProp = {}}: NavListSubNavProps) => {
-  const {buttonId, subNavId} = React.useContext(ItemWithSubNavContext)
+  const {buttonId, subNavId, isOpen} = React.useContext(ItemWithSubNavContext)
   const {depth} = React.useContext(SubNavContext)
 
   if (!buttonId || !subNavId) {
@@ -179,7 +186,8 @@ const SubNav = ({children, sx: sxProp = {}}: NavListSubNavProps) => {
         sx={merge<SxProp['sx']>(
           {
             padding: 0,
-            margin: 0
+            margin: 0,
+            display: isOpen ? 'block' : 'none'
           },
           sxProp
         )}