-
Notifications
You must be signed in to change notification settings - Fork 645
feat(TabNav): remove support for sx prop #6864
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
Conversation
🦋 Changeset detectedLatest commit: 6de983d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
…to remove_sx_support_tabnav
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/3359 |
|
🟢 ci completed with status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes support for the sx prop from the TabNav component in the main @primer/react package while maintaining backward compatibility by providing a styled-components wrapper in the @primer/styled-react package.
- Removes
SxPropfromTabNavPropsandTabNavLinkPropsin the main React package - Replaces
BoxWithFallbackusage with plain HTML elements to eliminatesxstyling - Creates a new styled-components wrapper in the styled-react package that preserves
sxfunctionality
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/TabNav/TabNav.tsx | Removes sx prop support and BoxWithFallback usage from main component |
| packages/styled-react/src/components/TabNav.tsx | Creates new styled-components wrapper that preserves sx functionality |
| packages/styled-react/src/deprecated.tsx | Updates exports to use local TabNav implementation |
| packages/styled-react/src/tests/primer-react-deprecated.browser.test.tsx | Updates test to include polymorphic rendering validation |
| .changeset/lucky-wasps-nail.md | Documents the breaking change |
| // @ts-ignore forwardedAs is valid here but I don't know how to fix the typescript error | ||
| const TabNavImpl = ({as, ...props}: TabNavProps) => <StyledTabNav forwardedAs={as} {...props} /> |
Copilot
AI
Sep 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using @ts-ignore, consider using a proper TypeScript assertion or interface extension to handle the forwardedAs prop. This improves type safety and code maintainability.
| // @ts-ignore forwardedAs is valid here but I don't know how to fix the typescript error | ||
| const TabNavLink = ({as, ...props}: TabNavLinkProps) => <StyledTabNavLink forwardedAs={as} {...props} /> |
Copilot
AI
Sep 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using @ts-ignore, consider using a proper TypeScript assertion or interface extension to handle the forwardedAs prop. This improves type safety and code maintainability.
francinelucca
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
|
👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks! |
Co-authored-by: Marie Lucca <[email protected]> Co-authored-by: Marie Lucca <[email protected]>
Closes #6779
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
Merge checklist