Skip to content

Commit

Permalink
Adds NavList.GroupHeading component (#5106)
Browse files Browse the repository at this point in the history 
* adds NavList.GroupHeading component

* Create twelve-kings-confess.md

* re-styles NavList.GroupHeading links to be fgColor-default

* guards against 'title' AND 'NavList.GroupHeading' being used together, adds tests, updates docs

* rm title prop from NavList.GroupHeading story
  • Loading branch information
@mperrotti
mperrotti authored Oct 22, 2024
1 parent 016f760 commit 851c857
Show file tree
Hide file tree
Showing 8 changed files with 147 additions and 27 deletions.
5 changes: 5 additions & 0 deletions .changeset/twelve-kings-confess.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

Adds NavList.GroupHeading component that can be used instead of the ActionList.Group 'title' prop if you need to render something besides a string
4 changes: 2 additions & 2 deletions packages/react/src/ActionList/Group.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export const Group: React.FC<React.PropsWithChildren<ActionListGroupProps>> = ({
)
}

export type GroupHeadingProps = Pick<ActionListGroupProps, 'variant' | 'auxiliaryText'> &
export type ActionListGroupHeadingProps = Pick<ActionListGroupProps, 'variant' | 'auxiliaryText'> &
Omit<ActionListHeadingProps, 'as'> &
SxProp &
React.HTMLAttributes<HTMLElement> & {
Expand All @@ -123,7 +123,7 @@ export type GroupHeadingProps = Pick<ActionListGroupProps, 'variant' | 'auxiliar
* hidden from the accessibility tree due to the limitation of listbox children. https://w3c.github.io/aria/#listbox
* groups under menu or listbox are labelled by `aria-label`
*/
export const GroupHeading: React.FC<React.PropsWithChildren<GroupHeadingProps>> = ({
export const GroupHeading: React.FC<React.PropsWithChildren<ActionListGroupHeadingProps>> = ({
as,
variant,
// We are not recommending this prop to be used, it should only be used internally for incremental rollout.
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/ActionList/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {LeadingVisual, TrailingVisual} from './Visuals'
import {Heading} from './Heading'

export type {ActionListProps} from './shared'
export type {ActionListGroupProps} from './Group'
export type {ActionListGroupProps, ActionListGroupHeadingProps} from './Group'
export type {ActionListItemProps} from './shared'
export type {ActionListLinkItemProps} from './LinkItem'
export type {ActionListDividerProps} from './Divider'
Expand Down
18 changes: 18 additions & 0 deletions packages/react/src/NavList/NavList.docs.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,24 @@
},
{
"name": "NavList.Group",
"props": [
{
"name": "title",
"type": "string",
"description": "The text that gets rendered as the group's heading. Alternatively, you can pass the `NavList.GroupHeading` component as a child of `NavList.Group`.\n If both `title` and `NavList.GroupHeading` are passed, `NavList.GroupHeading` will be rendered as the heading."
},
{
"name": "sx",
"type": "SystemStyleObject"
},
{
"name": "ref",
"type": "React.RefObject<HTMLElement>"
}
]
},
{
"name": "NavList.GroupHeading",
"props": [
{
"name": "sx",
Expand Down
28 changes: 28 additions & 0 deletions packages/react/src/NavList/NavList.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,34 @@ export const WithGroup: StoryFn = () => (
</PageLayout>
)

export const WithGroupHeadingLinks: StoryFn = () => (
<PageLayout>
<PageLayout.Pane position="start">
<NavList>
<NavList.Group>
<NavList.GroupHeading>
<a href="#group-1">Group 1</a>
</NavList.GroupHeading>
<NavList.Item aria-current="true" href="#">
Item 1A
</NavList.Item>
<NavList.Item href="#">Item 1B</NavList.Item>
<NavList.Item href="#">Item 1C</NavList.Item>
</NavList.Group>
<NavList.Group>
<NavList.GroupHeading>
<a href="#group-2">Group 2</a>
</NavList.GroupHeading>
<NavList.Item href="#">Item 2A</NavList.Item>
<NavList.Item href="#">Item 2B</NavList.Item>
<NavList.Item href="#">Item 2C</NavList.Item>
</NavList.Group>
</NavList>
</PageLayout.Pane>
<PageLayout.Content></PageLayout.Content>
</PageLayout>
)

export const WithTrailingAction = () => {
return (
<PageLayout>
Expand Down
23 changes: 23 additions & 0 deletions packages/react/src/NavList/NavList.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,29 @@ describe('NavList', () => {
expect(container).toMatchSnapshot()
})

it('only shows NavList.GroupHeading when NavList.Group `title` prop is passed AND NavList.GroupHeading is a child', () => {
const {getByText} = render(
<ThemeProvider>
<NavList>
<NavList.Group title="Overview">
<NavList.GroupHeading>Group heading</NavList.GroupHeading>
<NavList.Item href="/getting-started" aria-current="page">
Getting started
</NavList.Item>
</NavList.Group>
<NavList.Group title="Components">
<NavList.Item href="/Avatar">Avatar</NavList.Item>
</NavList.Group>
</NavList>
</ThemeProvider>,
)
const groupHeading = getByText('Group heading')
const groupTitle = getByText('Overview')

expect(groupHeading).toBeVisible()
expect(groupTitle).not.toBeVisible()
})

it('supports TrailingAction', async () => {
const {getByRole} = render(
<NavList>
Expand Down
44 changes: 42 additions & 2 deletions packages/react/src/NavList/NavList.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
ActionListDividerProps,
ActionListLeadingVisualProps,
ActionListTrailingVisualProps,
ActionListGroupHeadingProps,
} from '../ActionList'
import {ActionList} from '../ActionList'
import {ActionListContainerContext} from '../ActionList/ActionListContainerContext'
Expand Down Expand Up @@ -279,9 +280,21 @@ const Group: React.FC<NavListGroupProps> = ({title, children, sx: sxProp = defau
<>
{/* Hide divider if the group is the first item in the list */}
<ActionList.Divider sx={{'&:first-child': {display: 'none'}}} />
<ActionList.Group {...props} sx={sxProp}>
<ActionList.Group
{...props}
// If somebody tries to pass the `title` prop AND a `NavList.GroupHeading` as a child, hide the `ActionList.GroupHeading`
sx={merge<SxProp['sx']>(sxProp, {
':has([data-component="NavList.GroupHeading"]):has([data-component="ActionList.GroupHeading"])': {
'[data-component="ActionList.GroupHeading"]': {display: 'none'},
},
})}
>
{/* Setting up the default value for the heading level. TODO: API update to give flexibility to NavList.Group title's heading level */}
{title ? <ActionList.GroupHeading as="h3">{title}</ActionList.GroupHeading> : null}
{title ? (
<ActionList.GroupHeading as="h3" data-component="ActionList.GroupHeading">
{title}
</ActionList.GroupHeading>
) : null}
{children}
</ActionList.Group>
</>
Expand All @@ -290,6 +303,32 @@ const Group: React.FC<NavListGroupProps> = ({title, children, sx: sxProp = defau

Group.displayName = 'NavList.Group'

export type NavListGroupHeadingProps = ActionListGroupHeadingProps

/**
* This is an alternative to the `title` prop on `NavList.Group`.
* It was primarily added to allow links in group headings.
*/
const GroupHeading: React.FC<NavListGroupHeadingProps> = ({as = 'h3', sx: sxProp = defaultSxProp, ...rest}) => {
return (
<ActionList.GroupHeading
as={as}
sx={merge<SxProp['sx']>(
{
'> a {': {
color: 'var(--fgColor-default)',
textDecoration: 'inherit',
':hover': {textDecoration: 'underline'},
},
},
sxProp,
)}
data-component="NavList.GroupHeading"
{...rest}
/>
)
}

// ----------------------------------------------------------------------------
// Export

Expand All @@ -301,4 +340,5 @@ export const NavList = Object.assign(Root, {
TrailingAction,
Divider,
Group,
GroupHeading,
})
50 changes: 28 additions & 22 deletions packages/react/src/NavList/__snapshots__/NavList.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,10 @@ exports[`NavList renders with groups 1`] = `
margin-top: 8px;
}
.c2:has([data-component="NavList.GroupHeading"]):has([data-component="ActionList.GroupHeading"]) [data-component="ActionList.GroupHeading"] {
display: none;
}
.c3 {
padding-top: 6px;
padding-bottom: 6px;
Expand Down Expand Up @@ -872,6 +876,7 @@ exports[`NavList renders with groups 1`] = `
>
<h3
class="ActionListGroupHeading Heading"
data-component="ActionList.GroupHeading"
id=":r7:"
>
Overview
Expand Down Expand Up @@ -920,6 +925,7 @@ exports[`NavList renders with groups 1`] = `
>
<h3
class="ActionListGroupHeading Heading"
data-component="ActionList.GroupHeading"
id=":r9:"
>
Components
Expand Down Expand Up @@ -1389,15 +1395,15 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
class="c0"
>
<li
aria-labelledby=":r26:"
aria-labelledby=":r2c:"
class="c1 c2"
>
<button
aria-controls=":r27:"
aria-controls=":r2d:"
aria-expanded="true"
aria-labelledby=":r26:--label :r26:--trailing-visual "
aria-labelledby=":r2c:--label :r2c:--trailing-visual "
class="c3 c4"
id=":r26:"
id=":r2c:"
tabindex="0"
>
<div
Expand All @@ -1409,13 +1415,13 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
>
<span
class="c1 c7"
id=":r26:--label"
id=":r2c:--label"
>
Item
</span>
<span
class="c1 c8"
id=":r26:--trailing-visual"
id=":r2c:--trailing-visual"
>
<svg
aria-hidden="true"
Expand All @@ -1437,19 +1443,19 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
</button>
<div>
<ul
aria-labelledby=":r26:"
aria-labelledby=":r2c:"
class="c1 c10"
id=":r27:"
id=":r2d:"
>
<li
class="c3 c11"
>
<a
aria-current="page"
aria-labelledby=":r29:--label "
aria-labelledby=":r2f:--label "
class="c12 c13"
href="#"
id=":r29:"
id=":r2f:"
tabindex="0"
>
<div
Expand All @@ -1458,7 +1464,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
>
<span
class="c1 c14"
id=":r29:--label"
id=":r2f:--label"
>
Sub Item
</span>
Expand Down Expand Up @@ -1923,15 +1929,15 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
class="c0"
>
<li
aria-labelledby=":r20:"
aria-labelledby=":r26:"
class="c1 c2"
>
<button
aria-controls=":r21:"
aria-controls=":r27:"
aria-expanded="false"
aria-labelledby=":r20:--label :r20:--trailing-visual "
aria-labelledby=":r26:--label :r26:--trailing-visual "
class="c3 c4"
id=":r20:"
id=":r26:"
tabindex="0"
>
<div
Expand All @@ -1943,13 +1949,13 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
>
<span
class="c1 c7"
id=":r20:--label"
id=":r26:--label"
>
Item
</span>
<span
class="c1 c8"
id=":r20:--trailing-visual"
id=":r26:--trailing-visual"
>
<svg
aria-hidden="true"
Expand All @@ -1971,19 +1977,19 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
</button>
<div>
<ul
aria-labelledby=":r20:"
aria-labelledby=":r26:"
class="c1 c10"
id=":r21:"
id=":r27:"
>
<li
class="c3 c11"
>
<a
aria-current="page"
aria-labelledby=":r23:--label "
aria-labelledby=":r29:--label "
class="c12 c13"
href="#"
id=":r23:"
id=":r29:"
tabindex="0"
>
<div
Expand All @@ -1992,7 +1998,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
>
<span
class="c1 c7"
id=":r23:--label"
id=":r29:--label"
>
Sub Item
</span>
Expand Down

0 comments on commit 851c857

Please sign in to comment.