Skip to content

Remove sx rom the SideNav component #6735

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 8, 2025
Merged

Remove sx rom the SideNav component #6735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f09e90b
remove sx
liuliu-dev Sep 5, 2025
0c136d0
Merge branch 'main' into liuliu/remove-sx-from-sidenav
liuliu-dev Sep 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/rich-walls-fold.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': major
---

Update SideNav component to no longer support sx.
21 changes: 6 additions & 15 deletions packages/react/src/SideNav.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ import Link, {type LinkProps} from './Link'
import type React from 'react'
import {type PropsWithChildren} from 'react'
import {clsx} from 'clsx'
import type {SxProp} from './sx'
import classes from './SideNav.module.css'
import {BoxWithFallback} from './internal/components/BoxWithFallback'

type SideNavBaseProps = {
as?: React.ElementType
Expand All @@ -15,16 +13,15 @@ type SideNavBaseProps = {
className?: string
children?: React.ReactNode
'aria-label'?: string
} & SxProp
}

function SideNav({
as = 'nav',
as: Component = 'nav',
variant = 'normal',
className,
bordered,
children,
'aria-label': ariaLabel,
sx: sxProp,
}: SideNavBaseProps) {
const variantClassName = variant === 'lightweight' ? 'lightweight' : 'normal'
const newClassName = clsx(
Expand All @@ -39,9 +36,9 @@ function SideNav({
)

return (
<BoxWithFallback as={as} className={newClassName} aria-label={ariaLabel} sx={sxProp}>
<Component className={newClassName} aria-label={ariaLabel}>
{children}
</BoxWithFallback>
</Component>
)
}

Expand All @@ -58,15 +55,9 @@ const SideNavLink = ({selected, to, variant, className, children, ...rest}: Styl
// according to their docs, NavLink supports aria-current:
// https://reacttraining.com/react-router/web/api/NavLink/aria-current-string
return (
<BoxWithFallback
as={Link}
aria-current={isReactRouter || selected ? 'page' : undefined}
className={newClassName}
variant={variant}
{...rest}
>
<Link aria-current={isReactRouter || selected ? 'page' : undefined} className={newClassName} {...rest}>
Copy link
Preview

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

The variant prop is being removed from the Link component but it was previously passed to BoxWithFallback. This may cause a prop validation warning if Link doesn't accept a variant prop.

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The variant is already handled in newClassName and Link does not accept variant

{children}
</BoxWithFallback>
</Link>
)
}

Expand Down
Loading