Skip to content

Commit eb93bf5

Browse files
committed
Add support for flat nav list with no sections
1 parent 6ade809 commit eb93bf5

5 files changed

Lines changed: 101 additions & 23 deletions

File tree

packages/react/src/NavList/NavList.features.stories.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,14 @@ export const MultipleExpandedSections: Story = {
109109
render: () => <ExpandedDocsNavigation />,
110110
}
111111

112+
export const FlatList: Story = {
113+
render: () => (
114+
<NavList aria-label="Article navigation">
115+
{renderArticleItems(['Overview', 'Quickstart', 'Install GitHub Copilot'], 'Overview')}
116+
</NavList>
117+
),
118+
}
119+
112120
export const FiveLevels: Story = {
113121
render: () => (
114122
<NavList aria-label="Five-level navigation">

packages/react/src/NavList/NavList.module.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
position: relative;
2424
}
2525

26-
.NavList__list > .NavList__item + .NavList__item {
26+
.NavList__list:not(.NavList__list--flat) > .NavList__item + .NavList__item {
2727
background-image: repeating-linear-gradient(
2828
to right,
2929
var(--brand-color-border-muted) 0 var(--base-size-2),

packages/react/src/NavList/NavList.module.css.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ declare const styles: {
1515
readonly "NavList__link": string;
1616
readonly "NavList__link--disabled": string;
1717
readonly "NavList__list": string;
18+
readonly "NavList__list--flat": string;
1819
readonly "NavList__subNav": string;
1920
readonly "NavList__toggleIcon": string;
2021
readonly "NavList__trailingVisual": string;

packages/react/src/NavList/NavList.test.tsx

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,19 @@ describe('NavList', () => {
5151
expect(currentLink).toHaveAttribute('aria-current', 'page')
5252
})
5353

54+
it('renders a flat root list as leaf items', () => {
55+
const {getByRole} = render(<NavListFixture />)
56+
57+
const list = getByRole('list')
58+
const link = getByRole('link', {name: 'Overview'})
59+
const item = link.closest('li')
60+
61+
expect(list).toHaveClass('NavList__list--flat')
62+
expect(item).toHaveClass('NavList__item--leaf')
63+
expect(item).toHaveClass('NavList__item--level-2')
64+
expect(item).not.toHaveClass('NavList__item--level-1')
65+
})
66+
5467
it('supports className and ref passthrough on the root', () => {
5568
const expectedClass = 'test-class'
5669

@@ -94,6 +107,45 @@ describe('NavList', () => {
94107
expect(getByRole('link', {name: 'Custom link'})).toHaveAttribute('href', '/custom')
95108
})
96109

110+
it('passes keyboard handlers to rendered item controls', async () => {
111+
const user = userEvent.setup()
112+
const linkKeyDownTargets: HTMLElement[] = []
113+
const toggleKeyDownTargets: HTMLElement[] = []
114+
const onLinkKeyDown = jest.fn((event: React.KeyboardEvent<HTMLElement>) => {
115+
linkKeyDownTargets.push(event.currentTarget)
116+
})
117+
const onToggleKeyDown = jest.fn((event: React.KeyboardEvent<HTMLElement>) => {
118+
toggleKeyDownTargets.push(event.currentTarget)
119+
})
120+
121+
const {getByRole} = render(
122+
<NavList>
123+
<NavList.Item href="/overview" onKeyDown={onLinkKeyDown}>
124+
Overview
125+
</NavList.Item>
126+
<NavList.Item onKeyDown={onToggleKeyDown}>
127+
Docs
128+
<NavList.SubNav>
129+
<NavList.Item href="/docs/actions">Actions</NavList.Item>
130+
</NavList.SubNav>
131+
</NavList.Item>
132+
</NavList>,
133+
)
134+
135+
const link = getByRole('link', {name: 'Overview'})
136+
const toggle = getByRole('button', {name: 'Docs expand'})
137+
138+
link.focus()
139+
await user.keyboard('{ArrowDown}')
140+
toggle.focus()
141+
await user.keyboard('{ArrowDown}')
142+
143+
expect(onLinkKeyDown).toHaveBeenCalledTimes(1)
144+
expect(linkKeyDownTargets[0]).toBe(link)
145+
expect(onToggleKeyDown).toHaveBeenCalledTimes(1)
146+
expect(toggleKeyDownTargets[0]).toBe(toggle)
147+
})
148+
97149
it('renders visual slots', () => {
98150
const {getByRole} = render(
99151
<NavList>

packages/react/src/NavList/NavList.tsx

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ const testIds = {
3939
},
4040
}
4141

42-
type NavListRootProps = {
42+
export type NavListRootProps = {
4343
/**
4444
* Accessible label for the navigation landmark. Defaults to "Navigation".
4545
*/
@@ -100,24 +100,32 @@ const NavListRoot = forwardRef<HTMLElement, NavListRootProps>(
100100
...rest
101101
},
102102
ref,
103-
) => (
104-
<NavListContext.Provider value={{internalAccessibleLabels}}>
105-
<nav
106-
ref={ref}
107-
className={clsx(styles.NavList, className)}
108-
aria-label={ariaLabelledBy ? undefined : ariaLabel ?? internalAccessibleLabels.defaultNavigationLabel}
109-
aria-labelledby={ariaLabelledBy}
110-
data-testid={testId || testIds.root}
111-
{...rest}
112-
>
113-
<NavListLevelContext.Provider value={1}>
114-
<ul className={styles.NavList__list} data-testid={testIds.list}>
115-
{children}
116-
</ul>
117-
</NavListLevelContext.Provider>
118-
</nav>
119-
</NavListContext.Provider>
120-
),
103+
) => {
104+
const hasTopLevelSubNav = Children.toArray(children).some(childHasDirectSubNav)
105+
const rootLevel = hasTopLevelSubNav ? 1 : 2
106+
107+
return (
108+
<NavListContext.Provider value={{internalAccessibleLabels}}>
109+
<nav
110+
ref={ref}
111+
className={clsx(styles.NavList, className)}
112+
aria-label={ariaLabelledBy ? undefined : ariaLabel ?? internalAccessibleLabels.defaultNavigationLabel}
113+
aria-labelledby={ariaLabelledBy}
114+
data-testid={testId || testIds.root}
115+
{...rest}
116+
>
117+
<NavListLevelContext.Provider value={rootLevel}>
118+
<ul
119+
className={clsx(styles.NavList__list, !hasTopLevelSubNav && styles['NavList__list--flat'])}
120+
data-testid={testIds.list}
121+
>
122+
{children}
123+
</ul>
124+
</NavListLevelContext.Provider>
125+
</nav>
126+
</NavListContext.Provider>
127+
)
128+
},
121129
)
122130

123131
type Visual = ReactElement | React.ElementType
@@ -186,6 +194,18 @@ function getTextContent(node: ReactNode): string {
186194
.join('')
187195
}
188196

197+
function childHasDirectSubNav(node: ReactNode): boolean {
198+
if (!isValidElement(node)) return false
199+
200+
if (node.type === React.Fragment) {
201+
return Children.toArray((node as ElementWithChildren).props.children).some(childHasDirectSubNav)
202+
}
203+
204+
return Children.toArray((node as ElementWithChildren).props.children).some(
205+
child => isValidElement<NavListSubNavProps>(child) && child.type === NavListSubNav,
206+
)
207+
}
208+
189209
function renderVisual(visual: Visual | undefined, className: string) {
190210
if (!visual) return null
191211

@@ -216,7 +236,6 @@ const NavListItem = forwardRef(
216236
leadingVisual,
217237
onClick,
218238
onExpandedChange,
219-
onKeyDown,
220239
trailingVisual,
221240
'aria-current': ariaCurrent,
222241
'data-testid': testId,
@@ -332,11 +351,9 @@ const NavListItem = forwardRef(
332351
)
333352

334353
return (
335-
// eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
336354
<li
337355
className={clsx(styles.NavList__item, levelClassNames[level], isLeafItem && styles['NavList__item--leaf'])}
338356
data-testid={testId || testIds.item}
339-
onKeyDown={onKeyDown}
340357
>
341358
<div className={styles.NavList__itemContent}>
342359
{hasSubNav && canExpand ? (

0 commit comments

Comments
 (0)