-
Notifications
You must be signed in to change notification settings - Fork 536
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
Bump @primer/primitives to v5 #1514
Changes from 10 commits
30bf353
319e9f2
39723b2
1183a44
a177e94
4b2307e
cdeb8f2
ca8f1f5
1c0fb35
30b4c44
341cc2e
83e187c
e1dd1d2
84a02a0
3d55c1b
a9245bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@primer/components": major | ||
--- | ||
|
||
Remove deprecated color variables by upgrading to @primer/primitives [v5](https://github.com/primer/primitives/pull/251) |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,7 @@ | |
"license": "MIT", | ||
"dependencies": { | ||
"@primer/octicons-react": "^13.0.0", | ||
"@primer/primitives": "4.8.1", | ||
"@primer/primitives": "5.1.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wohoo! |
||
"@radix-ui/react-polymorphic": "0.0.14", | ||
"@react-aria/ssr": "3.1.0", | ||
"@styled-system/css": "5.1.5", | ||
|
@@ -113,7 +113,7 @@ | |
"eslint-plugin-jsx-a11y": "6.4.1", | ||
"eslint-plugin-mdx": "1.15.1", | ||
"eslint-plugin-prettier": "3.4.0", | ||
"eslint-plugin-primer-react": "0.5.0", | ||
"eslint-plugin-primer-react": "0.6.1", | ||
"eslint-plugin-react": "7.24.0", | ||
"eslint-plugin-react-hooks": "4.2.0", | ||
"jest": "27.0.4", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,11 +57,13 @@ const StyledTokenButton = styled.span<TokenButtonProps & SxProp>` | |
|
||
&:hover, | ||
&:focus { | ||
background-color: ${get('colors.fade.fg10')}; | ||
// TODO: choose a better functional color variable for this | ||
background-color: ${get('colors.neutral.muted')}; | ||
} | ||
|
||
&:active { | ||
background-color: ${get('colors.fade.fg15')}; | ||
// TODO: choose a better functional color variable for this | ||
background-color: ${get('colors.neutral.subtle')}; | ||
Comment on lines
+60
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mperrotti Since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These work well for the regular tokens, but they're not ideal for IssueLabelTokens that have dark background colors. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm comfortable going forward with this as-is and improving in a follow-up There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. @auareyou do you have any ideas about how we can replace fade variables here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sweet. Let's move forward with this as-is for now 👍 |
||
} | ||
|
||
${variants} | ||
|
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.
I see! it was only on mdx files previously.