Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds NavList.GroupHeading component #5106

Merged
merged 10 commits into from
Oct 22, 2024
5 changes: 5 additions & 0 deletions .changeset/twelve-kings-confess.md
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
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,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 @@ -122,7 +122,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
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
28 changes: 28 additions & 0 deletions packages/react/src/NavList/NavList.stories.tsx
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
27 changes: 27 additions & 0 deletions packages/react/src/NavList/NavList.tsx
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 @@ -290,6 +291,31 @@ 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> = ({sx: sxProp = defaultSxProp, ...rest}) => {
return (
<ActionList.GroupHeading
as="h3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) optional suggestion:
from line 285:

TODO: API update to give flexibility to NavList.Group title's heading level

Maybe this is a good opportunity to do that here? (take as as a prop that defaults to h3 and can be overriden if necessary). We could then use this same component for the title prop instead of ActionList.GroupHeading as well 🤔

Wouldn't make the title's prop heading level itself configurable, but might offer an alternative manually through the new GroupHeading

sx={merge<SxProp['sx']>(
{
'> a {': {
color: 'var(--fgColor-default)',
textDecoration: 'inherit',
':hover': {textDecoration: 'underline'},
},
},
sxProp,
)}
Comment on lines +316 to +325
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still want to use sx here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to use regular CSS for component styling in PRC yet?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're adding new styles I think it's safe to go directly to CSS modules. The risk is entirely around the fact that there could be other styles in dotcom relying on the existing sx styling architecture.

{...rest}
/>
)
}

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

Expand All @@ -301,4 +327,5 @@ export const NavList = Object.assign(Root, {
TrailingAction,
Divider,
Group,
GroupHeading,
})
Loading