-
Notifications
You must be signed in to change notification settings - Fork 5
add colors css stylesheet for color variables #592
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
Conversation
🦋 Changeset detectedLatest commit: 77678a7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
cypress-design
|
||||||||||||||||||||||||||||||||||||||||
| Project |
cypress-design
|
| Branch Review |
mabel/create-css-colors-stylesheet
|
| Run status |
|
| Run duration | 02m 33s |
| Commit |
|
| Committer | mabela416 |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
236
|
| View all changes introduced in this branch ↗︎ | |
UI Coverage
7.14%
|
|
|---|---|
|
|
309
|
|
|
25
|
Accessibility
99.43%
|
|
|---|---|
|
|
0 critical
2 serious
0 moderate
1 minor
|
|
|
23
|
ryanjwilke
left a comment
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.
The code looks good to me. I would only make a recommendation simplify the final API for what is consumed in the repositories downstream.
For example, my proposal would be to change this:
@use "@cypress-design/css/dist/index.cjs.css" as *;
To something a bit simpler if it's technically possible:
// Option A
@use "@cypress-design/css" as *;
// Option B
@use "@cypress-design/css/colors" as *;
|
@mabela416 Any idea why Percy is missing so many screenshots? It seems a bit odd. I don't remember seeing this issue in other PRs but I could be wrong. I know we've struggled with flake in this repo in the past. |
I've seen this in other PRs. Not sure why because the other PRs I've seen this happen to are just for icons so we're not making big changes so I don't think the missing snapshots are related to my work |
I was able to get this name down to |
|
@mabela416 That seems like a better compromise for sure. The only part that's hard to know is what I'm importing from that file. Is it just colors? Is it the entire design system and other selectors? Again, your call, but I think it can be helpful to make sure the API as simple and understandable as possible for future engineers. |
Yeah this is limited to the |
…lors-stylesheet' into mabel/create-css-colors-stylesheet
I updated the ReadMe to add a note about this |
Currently we can only use the design system colors if we import them like this in our javascript files
but when we're working with css/scss stylesheets, we can't import the colors. In the dashboard, we have a separate design system where we defined the colors ourselves and that's what we've been using but for studio, we're not using this cypress services design system so I decided to add a stylesheet to the package
@cypress-design/cssso that we can import it in our css/scss files like thisLike this we'll also have a central place where all our color definitions exist.