-
Notifications
You must be signed in to change notification settings - Fork 72
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
Design system: Modular CSS support, Color palette, Ant theme update #5453
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
df4aeec
to
ad77763
Compare
fides Run #10771
Run Properties:
|
Project |
fides
|
Branch Review |
refs/pull/5453/merge
|
Run status |
Passed #10771
|
Run duration | 00m 37s |
Commit |
c31ddf2839 ℹ️: Merge aa56277b1dd08ccd33176c7a7d28f591a1a67bc8 into e5f240d537c41c8ddb429f737928...
|
Committer | Jason Gill |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
View all changes introduced in this branch ↗︎ |
ad77763
to
646a181
Compare
e951f11
to
a405e1b
Compare
a405e1b
to
aa56277
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5453 +/- ##
==========================================
+ Coverage 77.00% 85.47% +8.46%
==========================================
Files 384 384
Lines 24116 24116
Branches 2624 2624
==========================================
+ Hits 18571 20612 +2041
+ Misses 4971 2950 -2021
+ Partials 574 554 -20 ☔ View full report in Codecov by Sentry. |
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.
Great work! This is organized very neatly. Having a single source of truth for colors is very important. The flexibility for having the palettes be used for ant overrides, but also exported as variables and TS too is going to be very helpful. Left a tiny question about base styles. Thanks for also updating the PC. Approved.
colorError: "#d9534f", | ||
colorLink: "#2272ce", | ||
colorBgBase: "#ffffff", | ||
colorPrimary: palette.FIDESUI_MINOS, |
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.
Very clean using the palette for overides.
* Adds the color variables from the palette to the root element | ||
*/ | ||
:root { | ||
@each $color, $value in $colors { |
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.
Very helpful to have every color as a css variable.
@@ -0,0 +1,85 @@ | |||
$colors: ( |
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.
With all the different libraries, utilities, etc having a single source for colors is going to be much simpler. This is great, and the added TS export of colors is just chef kiss.
2aeddcb
to
aa49267
Compare
aa49267
to
44a657f
Compare
- name: Build | ||
run: npm run build | ||
|
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.
cy:start
already runs a build, so this was slowing down everything considerably building everything twice!
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.
Nice optimization
clients/admin-ui/turbo.json
Outdated
@@ -1,6 +1,6 @@ | |||
{ | |||
"extends": ["//"], | |||
"pipeline": { | |||
"tasks": { |
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.
new version update made this change automatically
b8eb274
to
ba49a77
Compare
clients/package.json
Outdated
} | ||
"turbo": "^2.2.3" | ||
}, | ||
"packageManager": "[email protected]" |
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.
turbo upgrade made all of these additions automatically
ba49a77
to
f7970bc
Compare
clients/turbo.json
Outdated
@@ -1,6 +1,6 @@ | |||
{ | |||
"$schema": "https://turbo.build/schema.v1.json", | |||
"pipeline": { | |||
"tasks": { |
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.
turbo upgrade made this change automatically
f7970bc
to
ea92c00
Compare
38b6148
to
4d16645
Compare
f7daffc
to
c01a4dd
Compare
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.
Looks great! Thanks for doing this, this'll make things a lot easier moving forward.
c01a4dd
to
ecacfd1
Compare
ecacfd1
to
aa56277
Compare
fides Run #10772
Run Properties:
|
Project |
fides
|
Branch Review |
main
|
Run status |
Passed #10772
|
Run duration | 00m 37s |
Commit |
e3d2fe8c03: Design system: Modular CSS support, Color palette, Ant theme update (#5453)
|
Committer | Jason Gill |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
View all changes introduced in this branch ↗︎ |
Closes HJ-146
Description Of Changes
Adds Modular CSS support via SASS integration with NextJS. Creates new CSS module containing and exporting our custom color palette to match Figma work. Utilizes new palette to feed values in the Ant theme.
Code Changes
sass
npm package, which NextJS will automatically recognize and support for CSS modulespalette.module.scss
with values input directly from the Figma paletteSteps to Confirm
Pre-Merge Checklist
CHANGELOG.md