Update examples/nextjs to use primer/react without styled-components #7094
Conversation
|
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the Next.js example from using the legacy @primer/styled-react package to the modern @primer/react package, removing the styled-components dependency entirely.
Key changes:
- Removed
@primer/styled-reactandstyled-componentsdependencies from package.json - Updated imports from
@primer/styled-reactto@primer/reactfor ThemeProvider and BaseStyles - Simplified the example by removing styled-components registry and custom styled components
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| package-lock.json | Removed @primer/styled-react and styled-components from the Next.js example workspace |
| examples/nextjs/package.json | Removed @primer/styled-react and styled-components dependencies |
| examples/nextjs/src/app/registry.tsx | Deleted the entire styled-components registry file (no longer needed) |
| examples/nextjs/src/app/layout.tsx | Updated to import from @primer/react and removed StyledComponentsRegistry wrapper |
| examples/nextjs/src/app/page.tsx | Simplified example to use only @primer/react Button, removing styled-components usage |
| examples/nextjs/src/app/global.css | Removed body styling that is now handled by BaseStyles component |
| @import '@primer/primitives/dist/css/primitives.css'; | ||
| @import '@primer/primitives/dist/css/functional/themes/light.css'; | ||
| @import '@primer/primitives/dist/css/functional/themes/dark.css'; |
There was a problem hiding this comment.
The removal of the body styles (color and background-color) appears intentional since these are now handled by the BaseStyles component in layout.tsx. However, there's no comment explaining this dependency. Consider adding a comment in this file explaining that body styles are managed by the BaseStyles component to prevent confusion if someone tries to add them back.
See below for a potential fix:
@ -1,3 +1,8 @@
/*
* Note: Body styles for `color` and `background-color` are intentionally omitted here.
* These are managed by the BaseStyles component in layout.tsx.
* Do not add body color/background-color styles in this file.
*/
| body { | ||
| color: var(--fgColor-default); | ||
| background-color: var(--bgColor-default); | ||
| } |
There was a problem hiding this comment.
I think we still need these on body since BaseStyles only adds them to the root of that component, right? Or is that not the case anymore 👀
There was a problem hiding this comment.
Pretty sure these styles never worked because the variables are not defined if data-color-mode is not present on <html> or <body> 🙃
BaseStyles only adds them to the root of that component
That is still correct
Eventually, it would make sense for us to offer a way to have ThemeProvider set data-color-mode on <html> instead on the div rendered by ThemeProvider or BaseStyles
There was a problem hiding this comment.
Fair 😂 lol, those attributes definitely need to be there 😰
Rollout strategy
Testing & Reviewing
Merge checklist