-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
More color utilities! #760
Conversation
Since these styles are very repetitive, it might not be that big of a difference after gzipping? 🤔 |
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'm on the fence with the fg-
vs color-
prefix.
fg-
is short and would make it similar tobg-
.color-
describes better what it does -> changing thecolor
property.
Hmm.. I'm slightly leaning towards color-
.
You're right! Before: 7.48 kB vs. after: 8.47 kB for git co master
npm run dist
npx gzip-size-cli dist/utilities.css
# 7.48 kB
git co more-color-utilities
npm run dist
npx gzip-size-cli dist/utilities.css
# 8.47 kB So yeah, maybe not as big of an issue as I thought! 🤗 |
Yeah, I went back and forth on this. The use case that has me leaning toward <%= octicon("heart", class: "text-red") %> ...thanks to Of course, on the other hand, |
On a second thought, because <%= octicon("heart", class: "color-red-8") %>
<%= octicon("heart", class: "fg-red-8") %>
<%= octicon("heart", class: "c-red-8") %> Not sure how much trouble it would be to also change it for Components? <StyledOcticon icon={Heart} color="red.8" />
<StyledOcticon icon={Heart} fg="red.8" />
<StyledOcticon icon={Heart} c="red.8" /> |
ea6e0c2
to
2db3b0b
Compare
Just copying my Slack comments over here! I could see the benefit of either |
2db3b0b
to
e3780e3
Compare
7: $purple-700, | ||
8: $purple-800, | ||
9: $purple-900, | ||
) !default; |
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.
Note: I did this by hand, but we're going to eventually automate the generation of this file from primer-colors
, as in #761.
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.
On Slack the consensus was to go with color-
prefix. Therefore ready to 🚢 👍
This PR adds 140 new utility classes — a foreground and background utility for each shade (10) of every hue (of which there are 7).
There is some redundancy with existing utilities, and GitHub's postcss config combines selectors with the same style (for instance,
.text-blue
and.fg-blue-5
are combined), but the size difference isnotactually pretty insignificant: less than 1K gzipped!I am swapping out the
text-
prefix used in our "old" color utilities forfg-
color-
because we often use utilities to give color to octicons rather than text specifically.