Uh oh!
There was an error while loading. Please reload this page.
Remove support for sx and Box usage from the UnderlineNav component - #6668
Conversation
🦋 Changeset detectedLatest commit: d59c187 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! |
size-limit report 📦
|
There was a problem hiding this comment.
Pull Request Overview
This PR removes the deprecated sx prop and Box component usage from the UnderlineNav component as part of a major release. The changes replace styled-system functionality with CSS modules and native HTML elements for better performance and maintainability.
Key Changes
- Removed
sxprop support from bothUnderlineNavandUnderlineNavItemcomponents - Replaced
Boxcomponents with native HTML elements (li,div,span) - Migrated inline styles to CSS modules for better maintainability
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
packages/react/src/UnderlineNav/styles.ts | Changed marginRight from theme unit to explicit pixel value |
packages/react/src/UnderlineNav/UnderlineNavItem.tsx | Removed sx prop, Box import, replaced Box with li element and CSS module |
packages/react/src/UnderlineNav/UnderlineNavItem.module.css | Added CSS module for UnderlineNavItem styling |
packages/react/src/UnderlineNav/UnderlineNav.tsx | Removed sx prop, Box imports, replaced Box elements with native HTML |
packages/react/src/UnderlineNav/UnderlineNav.module.css | Added CSS module for menu item content styling |
packages/react/src/UnderlineNav/UnderlineNav.docs.json | Removed sx prop from documentation |
.changeset/hot-mirrors-feel.md | Added changeset for major version bump |
👋 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! |
👋 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! |
joshblack
left a comment
There was a problem hiding this comment.
Looks great! just left one question
| ${sx}; | ||
| ` | ||
| // Needed for typing the ref with HTMLULListElement | ||
| export const NavigationList = styled.ul`` |
There was a problem hiding this comment.
Super random, do we need this export? I couldn't find where it is used 🤔
👋 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! |
👋 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! |
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/3004 |
…er/react into liuliu/migrate-underlinenav
👋 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! |
🟢 ci completed with status |
👋 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! |
👋 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! |
👋 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! |
Closes#5770
sxprops withUnderlineNav. Tracking here: https://github.com/github/primer/issues/5827Changelog
Removed
Removed
sxprops andBoxfromUnderlineNav.Rollout strategy