Implement functional color variables part 3 [CounterLabel, Dialog, Dropdown, DropdownStyles, FilterList]#1099
Conversation
🦋 Changeset detectedLatest commit: 29e2b5e 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 |
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/primer/primer-components/5g11aGBic5rYbvQTN7GesQ83zaat |
VanAnderson
force-pushed
the
VanAnderson/implement-functional-color-vars-3
branch
from
0403e05 to
53c97b9
Compare
|
|
||
| const DialogBase = styled.div<StyledDialogBaseProps>` | ||
| box-shadow: 0px 4px 32px rgba(0, 0, 0, 0.35); | ||
| box-shadow: ${get('shadows.shadow.large')}; |
There was a problem hiding this comment.
couldn't find a primer/css file for dialog, so I'm guessing at some of these. Dark mode doesn't seem quite right:
There was a problem hiding this comment.
Here's the Primer CSS equivalent for Dialog: https://primer-css-p02l95sun-primer.vercel.app/css/components/box-overlay
Looks like the code for that actually lives in github/github: https://github.com/github/github/blob/master/app/assets/stylesheets/components/box-overlay.scss
VanAnderson
force-pushed
the
VanAnderson/implement-functional-color-vars-3
branch
from
c05fa44 to
f8dce3a
Compare
| text-overflow: ellipsis; | ||
| white-space: nowrap; | ||
| cursor: pointer; | ||
| border-radius: ${get('radii.1')}; |
There was a problem hiding this comment.
| border-radius: ${get('radii.1')}; | |
| border-radius: ${get('radii.2')}; |
colebemis
left a comment
colebemis
left a comment
There was a problem hiding this comment.
Looking good 😄 Just left a few suggestions
| import {render as HTMLRender, cleanup} from '@testing-library/react' | ||
| import {axe, toHaveNoViolations} from 'jest-axe' | ||
| import 'babel-polyfill' | ||
| import {default as primitives} from '@primer/primitives' |
There was a problem hiding this comment.
Instead of importing from primitives we can import the theme:
- import {default as primitives} from '@primer/primitives'
+ import theme from '../theme'That might make values easier to reference in the tests
|
|
||
| const DialogBase = styled.div<StyledDialogBaseProps>` | ||
| box-shadow: 0px 4px 32px rgba(0, 0, 0, 0.35); | ||
| box-shadow: ${get('shadows.shadow.large')}; |
There was a problem hiding this comment.
Here's the Primer CSS equivalent for Dialog: https://primer-css-p02l95sun-primer.vercel.app/css/components/box-overlay
Looks like the code for that actually lives in github/github: https://github.com/github/github/blob/master/app/assets/stylesheets/components/box-overlay.scss
3 participants
This PR updates the following components to use functional color variables in preparation for color mode support:
Part of https://github.com/github/design-systems/issues/1219