fix(NavList): merge sx and non-sx scenarios #6264
Conversation
🦋 Changeset detectedLatest commit: 864dc68 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
Pull Request Overview
This PR updates the NavList.Group component to consolidate the sx and no‑sx scenarios into a single render path and ensures that a divider is always rendered.
- Changed the component from an arrow function with conditional rendering to a standard function with a unified return
- Switched from using Box to BoxWithFallback when rendering ActionList.Group
- Updated the changeset to reflect the patch release
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/react/src/NavList/NavList.tsx | Unified NavList.Group rendering and ensured the divider is shown |
| .changeset/kind-pants-march.md | Documented the changeset for the patch release |
Comments suppressed due to low confidence (1)
packages/react/src/NavList/NavList.tsx:291
- [nitpick] Consider adding a comment to explain that <ActionList.Divider /> is intentionally rendered unconditionally to harmonize the visual output between the sx and no‑sx scenarios.
<ActionList.Divider />
github-actions
bot
added
the
integration-tests: recommended
|
👋 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! |
joshblack
changed the title
size-limit report 📦
|
| return ( | ||
| <> | ||
| <ActionList.Divider /> | ||
| <ActionList.Group {...props}> | ||
| <BoxWithFallback as={ActionList.Group} {...rest} sx={sx}> |
There was a problem hiding this comment.
I was thinking we probably don't need boxwithfallback here since ActionList.Group would have BoxWithFallback?
There was a problem hiding this comment.
Ooo great point, will update 👍
|
I think I need to coordinate this with: https://github.com/github/primer/issues/5407 in order to land in a release and not disrupt memex 👀 |
Closes https://github.com/github/primer/issues/5391
Update the
NavList.Groupcomponent to have the same result when bothsxand nosxprop is provided. This change has an unintended side-effect which is that in thesxcase no divider was being rendered 😅Changelog
New
Changed
sxand no-sxscenarios into single return forNavList.GroupRemoved
Rollout strategy