Skip to content

Commit 46cdad4

Browse files
mperrottimperrotti
authored
Merge branch 'main' into mp/dont-pass-onRemove-through-token
2 parents 8bd19ce + 0530103 commit 46cdad4

File tree

6 files changed

+415
-61
lines changed

6 files changed

+415
-61
lines changed

‎.changeset/dry-feet-attack.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@primer/react": patch
3+
---
4+
5+
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‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,6 @@ If a `NavList.Item` contains a `NavList.SubNav`, the `NavList.Item` will render
174174

175175
### With React Router
176176

177-
<Note variant="danger">Not implemented yet</Note>
178-
179177
```jsx
180178
import {Link, useMatch, useResolvedPath} from 'react-router-dom'
181179
import {NavList} from '@primer/react'
@@ -203,8 +201,6 @@ function App() {
203201

204202
### With Next.js
205203

206-
<Note variant="danger">Not implemented yet</Note>
207-
208204
```jsx
209205
import {useRouter} from 'next/router'
210206
import Link from 'next/link'

‎src/NavList/NavList.stories.tsx‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export const Simple: Story = () => (
2626
</PageLayout>
2727
)
2828

29-
export const SubItems: Story = () => (
29+
export const WithSubItems: Story = () => (
3030
<PageLayout>
3131
<PageLayout.Pane position="start">
3232
<NavList>
@@ -47,4 +47,61 @@ export const SubItems: Story = () => (
4747
</PageLayout>
4848
)
4949

50+
type ReactRouterLikeLinkProps = {to: string; children: React.ReactNode}
51+
const ReactRouterLikeLink = React.forwardRef<HTMLAnchorElement, ReactRouterLikeLinkProps>(({to, ...props}, ref) => {
52+
// eslint-disable-next-line jsx-a11y/anchor-has-content
53+
return <a ref={ref} href={to} {...props} />
54+
})
55+
56+
export const WithReactRouterLink = () => (
57+
<PageLayout>
58+
<PageLayout.Pane position="start">
59+
<NavList>
60+
<NavList.Item as={ReactRouterLikeLink} to="#" aria-current="page">
61+
Item 1
62+
</NavList.Item>
63+
<NavList.Item as={ReactRouterLikeLink} to="#">
64+
Item 2
65+
</NavList.Item>
66+
<NavList.Item as={ReactRouterLikeLink} to="#">
67+
Item 3
68+
</NavList.Item>
69+
</NavList>
70+
</PageLayout.Pane>
71+
<PageLayout.Content></PageLayout.Content>
72+
</PageLayout>
73+
)
74+
75+
type NextJSLinkProps = {href: string; children: React.ReactNode}
76+
77+
const NextJSLikeLink = React.forwardRef<HTMLAnchorElement, NextJSLinkProps>(
78+
({href, children}, ref): React.ReactElement => {
79+
const child = React.Children.only(children)
80+
const childProps = {
81+
ref,
82+
href
83+
}
84+
return <>{React.isValidElement(child) ? React.cloneElement(child, childProps) : null}</>
85+
}
86+
)
87+
88+
export const WithNextJSLink = () => (
89+
<PageLayout>
90+
<PageLayout.Pane position="start">
91+
<NavList>
92+
<NextJSLikeLink href="#">
93+
<NavList.Item aria-current="page">Item 1</NavList.Item>
94+
</NextJSLikeLink>
95+
<NextJSLikeLink href="#">
96+
<NavList.Item>Item 2</NavList.Item>
97+
</NextJSLikeLink>
98+
<NextJSLikeLink href="#">
99+
<NavList.Item>Item 3</NavList.Item>
100+
</NextJSLikeLink>
101+
</NavList>
102+
</PageLayout.Pane>
103+
<PageLayout.Content></PageLayout.Content>
104+
</PageLayout>
105+
)
106+
50107
export default meta

‎src/NavList/NavList.test.tsx‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,26 @@ import React from 'react'
33
import {ThemeProvider, SSRProvider} from '..'
44
import {NavList} from './NavList'
55

6+
type ReactRouterLikeLinkProps = {to: string; children: React.ReactNode}
7+
8+
const ReactRouterLikeLink = React.forwardRef<HTMLAnchorElement, ReactRouterLikeLinkProps>(({to, ...props}, ref) => {
9+
// eslint-disable-next-line jsx-a11y/anchor-has-content
10+
return <a ref={ref} href={to} {...props} />
11+
})
12+
13+
type NextJSLinkProps = {href: string; children: React.ReactNode}
14+
15+
const NextJSLikeLink = React.forwardRef<HTMLAnchorElement, NextJSLinkProps>(
16+
({href, children}, ref): React.ReactElement => {
17+
const child = React.Children.only(children)
18+
const childProps = {
19+
ref,
20+
href
21+
}
22+
return <>{React.isValidElement(child) ? React.cloneElement(child, childProps) : null}</>
23+
}
24+
)
25+
626
describe('NavList', () => {
727
it('renders a simple list', () => {
828
const {container} = render(
@@ -60,6 +80,36 @@ describe('NavList.Item', () => {
6080
expect(homeLink).toHaveAttribute('aria-current', 'page')
6181
expect(aboutLink).not.toHaveAttribute('aria-current')
6282
})
83+
84+
it('is compatiable with React-Router-like link components', () => {
85+
const {getByRole} = render(
86+
<NavList>
87+
<NavList.Item as={ReactRouterLikeLink} to={'/'} aria-current="page">
88+
React Router link
89+
</NavList.Item>
90+
</NavList>
91+
)
92+
93+
const link = getByRole('link', {name: 'React Router link'})
94+
95+
expect(link).toHaveAttribute('aria-current', 'page')
96+
expect(link).toHaveAttribute('href', '/')
97+
})
98+
99+
it('is compatible with NextJS-like link components', () => {
100+
const {getByRole} = render(
101+
<NavList>
102+
<NextJSLikeLink href="/">
103+
<NavList.Item aria-current="page">NextJS link</NavList.Item>
104+
</NextJSLikeLink>
105+
</NavList>
106+
)
107+
108+
const link = getByRole('link', {name: 'NextJS link'})
109+
110+
expect(link).toHaveAttribute('href', '/')
111+
expect(link).toHaveAttribute('aria-current', 'page')
112+
})
63113
})
64114

65115
describe('NavList.Item with NavList.SubNav', () => {
@@ -227,4 +277,33 @@ describe('NavList.Item with NavList.SubNav', () => {
227277

228278
expect(consoleSpy).toHaveBeenCalled()
229279
})
280+
281+
it('is compatiable with React-Router-like link components', () => {
282+
function NavLink({href, children}: {href: string; children: React.ReactNode}) {
283+
// 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
284+
const isCurrent = children === 'Current'
285+
return (
286+
<NavList.Item as={ReactRouterLikeLink} to={href} aria-current={isCurrent ? 'page' : false}>
287+
{children}
288+
</NavList.Item>
289+
)
290+
}
291+
292+
const {queryByRole} = render(
293+
<NavList>
294+
<NavLink href="/">Item 1</NavLink>
295+
<NavList.Item>
296+
Item 2
297+
<NavList.SubNav>
298+
<NavLink href="/sub-item-1">Current</NavLink>
299+
<NavLink href="/sub-item-2">Sub item 2</NavLink>
300+
</NavList.SubNav>
301+
</NavList.Item>
302+
<NavLink href="/">Item 3</NavLink>
303+
</NavList>
304+
)
305+
306+
const currentLink = queryByRole('link', {name: 'Current'})
307+
expect(currentLink).toBeVisible()
308+
})
230309
})

‎src/NavList/NavList.tsx‎

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {ChevronDownIcon} from '@primer/octicons-react'
2+
import {ForwardRefComponent as PolymorphicForwardRefComponent} from '@radix-ui/react-polymorphic'
23
import {useSSRSafeId} from '@react-aria/ssr'
34
import React, {isValidElement} from 'react'
45
import styled from 'styled-components'
@@ -36,9 +37,8 @@ export type NavListItemProps = {
3637
'aria-current'?: 'page' | 'step' | 'location' | 'date' | 'time' | 'true' | 'false' | boolean
3738
} & SxProp
3839

39-
// TODO: as prop
4040
const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
41-
({href, 'aria-current': ariaCurrent, children, sx: sxProp = {}}, ref) => {
41+
({'aria-current': ariaCurrent, children, sx: sxProp = {}, ...props}, ref) => {
4242
const {depth} = React.useContext(SubNavContext)
4343

4444
// Get SubNav from children
@@ -51,13 +51,8 @@ const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
5151

5252
// Render ItemWithSubNav if SubNav is present
5353
if (subNav && isValidElement(subNav) && depth < 1) {
54-
// Search SubNav children for current Item
55-
const currentItem = React.Children.toArray(subNav.props.children).find(
56-
child => isValidElement(child) && child.props['aria-current']
57-
)
58-
5954
return (
60-
<ItemWithSubNav subNav={subNav} subNavContainsCurrentItem={Boolean(currentItem)} sx={sxProp}>
55+
<ItemWithSubNav subNav={subNav} sx={sxProp}>
6156
{childrenWithoutSubNav}
6257
</ItemWithSubNav>
6358
)
@@ -66,7 +61,6 @@ const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
6661
return (
6762
<ActionList.LinkItem
6863
ref={ref}
69-
href={href}
7064
aria-current={ariaCurrent}
7165
active={Boolean(ariaCurrent) && ariaCurrent !== 'false'}
7266
sx={merge<SxProp['sx']>(
@@ -77,12 +71,13 @@ const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
7771
},
7872
sxProp
7973
)}
74+
{...props}
8075
>
8176
{children}
8277
</ActionList.LinkItem>
8378
)
8479
}
85-
)
80+
) as PolymorphicForwardRefComponent<'a', NavListItemProps>
8681

8782
Item.displayName = 'NavList.Item'
8883

@@ -92,36 +87,48 @@ Item.displayName = 'NavList.Item'
9287
type ItemWithSubNavProps = {
9388
children: React.ReactNode
9489
subNav: React.ReactNode
95-
subNavContainsCurrentItem: boolean
9690
} & SxProp
9791

98-
const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: string}>({
92+
const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: string; isOpen: boolean}>({
9993
buttonId: '',
100-
subNavId: ''
94+
subNavId: '',
95+
isOpen: false
10196
})
10297

10398
// TODO: ref prop
10499
// TODO: Animate open/close transition
105-
function ItemWithSubNav({children, subNav, subNavContainsCurrentItem, sx: sxProp = {}}: ItemWithSubNavProps) {
100+
function ItemWithSubNav({children, subNav, sx: sxProp = {}}: ItemWithSubNavProps) {
106101
const buttonId = useSSRSafeId()
107102
const subNavId = useSSRSafeId()
108-
// SubNav starts open if current item is in it
109-
const [isOpen, setIsOpen] = React.useState(subNavContainsCurrentItem)
103+
const [isOpen, setIsOpen] = React.useState(false)
104+
const subNavRef = React.useRef<HTMLDivElement>(null)
105+
const [containsCurrentItem, setContainsCurrentItem] = React.useState(false)
106+
107+
React.useLayoutEffect(() => {
108+
if (subNavRef.current) {
109+
// Check if SubNav contains current item
110+
const currentItem = subNavRef.current.querySelector('[aria-current]')
111+
if (currentItem && currentItem.getAttribute('aria-current') !== 'false') {
112+
setContainsCurrentItem(true)
113+
setIsOpen(true)
114+
}
115+
}
116+
}, [subNav])
110117

111118
return (
112-
<ItemWithSubNavContext.Provider value={{buttonId, subNavId}}>
119+
<ItemWithSubNavContext.Provider value={{buttonId, subNavId, isOpen}}>
113120
<Box as="li" aria-labelledby={buttonId} sx={{listStyle: 'none'}}>
114121
<ActionList.Item
115122
as="button"
116123
id={buttonId}
117124
aria-expanded={isOpen}
118125
aria-controls={subNavId}
119126
// When the subNav is closed, how should we indicated that the subNav contains the current item?
120-
active={!isOpen && subNavContainsCurrentItem}
127+
active={!isOpen && containsCurrentItem}
121128
onClick={() => setIsOpen(open => !open)}
122129
sx={merge<SxProp['sx']>(
123130
{
124-
fontWeight: subNavContainsCurrentItem ? 'bold' : null // Parent item is bold if any of it's sub-items are current
131+
fontWeight: containsCurrentItem ? 'bold' : null // Parent item is bold if any of it's sub-items are current
125132
},
126133
sxProp
127134
)}
@@ -138,7 +145,7 @@ function ItemWithSubNav({children, subNav, subNavContainsCurrentItem, sx: sxProp
138145
</ActionList.TrailingVisual>
139146
</ActionList.Item>
140147

141-
{isOpen ? subNav : null}
148+
<div ref={subNavRef}>{subNav}</div>
142149
</Box>
143150
</ItemWithSubNavContext.Provider>
144151
)
@@ -156,7 +163,7 @@ const SubNavContext = React.createContext<{depth: number}>({depth: 0})
156163
// TODO: ref prop
157164
// NOTE: SubNav must be a direct child of an Item
158165
const SubNav = ({children, sx: sxProp = {}}: NavListSubNavProps) => {
159-
const {buttonId, subNavId} = React.useContext(ItemWithSubNavContext)
166+
const {buttonId, subNavId, isOpen} = React.useContext(ItemWithSubNavContext)
160167
const {depth} = React.useContext(SubNavContext)
161168

162169
if (!buttonId || !subNavId) {
@@ -179,7 +186,8 @@ const SubNav = ({children, sx: sxProp = {}}: NavListSubNavProps) => {
179186
sx={merge<SxProp['sx']>(
180187
{
181188
padding: 0,
182-
margin: 0
189+
margin: 0,
190+
display: isOpen ? 'block' : 'none'
183191
},
184192
sxProp
185193
)}

0 commit comments

Comments
 (0)