Repository navigation
chore: upgrade @primer/react to v38 - #505
Conversation
|
|
||
| const DashLayout = ({ children }: { children: React.ReactNode }) => { | ||
| return <Box>{children}</Box> | ||
| return <>{children}</> |
There was a problem hiding this comment.
Should this swap to div like other Box uses did?
| @@ -0,0 +1,31 @@ | |||
| .fieldMarginBottom { | |||
There was a problem hiding this comment.
We may want to add a CSS linter to this repo now, if there is not already one.
| </Box> | ||
| <Box sx={{ textAlign: 'center' }}> | ||
| <Text sx={{ fontSize: '1', color: 'fg.muted' }}> | ||
| <div className={sharedStyles.authCard}> |
There was a problem hiding this comment.
These components are also much more readable this way
| import { Avatar, Box, Link, Octicon, Stack } from '@primer/react' | ||
| import { Blankslate, DataTable, Table } from '@primer/react/drafts' | ||
| import { Avatar, Link, Stack } from '@primer/react' | ||
| import { Blankslate, DataTable, Table } from '@primer/react/experimental' |
There was a problem hiding this comment.
How much do we want to depend on experimental components? I guess if we're already using the equivalent currently then it makes sense
| </Stack> | ||
| ) | ||
| }, | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any |
There was a problem hiding this comment.
Nice, thanks for cleaning these up along the way!
|
Forgot to ask if this should be released. I guess we can release after the other dependency updates land? |
Not sure! What do you think? I'm good with releasing each one since they work independently but releasing together also makes sense. @riley-kohler -- do you have a preference? |
|
My preference is generally to always release if possible (continuous deployment mentality). It really sucks when an open source project has changes that you want for whatever reason that are merged but not published. It's a hassle to release if a change was merged without the release tag but we should make sure the next PR releases. |
3 participants
Pull Request
@primer/react36.27.0 → 38.39.0 and@primer/octicons-reactto 19.35.0styled-components, which drops support for thesxprop andstyled-systemprops — migrates every affected component offsxto CSS Modules.module.cssfiles added alongside each migrated component, plus a sharedsrc/app/styles/shared.module.cssfor common auth-card stylingAddresses the
@primer/reactv38 step of the dependency upgrade plan in #412.Working UI post-changes
Readiness Checklist
Author/Contributor
npm run formatand fix any formatting issues that have been introducednpm run lintand fix any linting issues that have been introducednpm run testand run tests