Refactor tags to support custom colors #668
Conversation
| @@ -11,14 +11,16 @@ $tag-margin: ($pt-input-height - $tag-height) / 2 !default; | |||
| $tag-height-large: $pt-grid-size * 3 !default; | |||
| $tag-padding-large: ($tag-height-large - $pt-icon-size-large) / 2 !default; | |||
|
|
|||
| $tag-removable-selector: "&.pt-tag-removable"; | |||
There was a problem hiding this comment.
no need for variable here cuz it can't be customized anymore. just do the normal selector thing and write it out in css below.
| @@ -80,6 +82,8 @@ Styleguide components.tag.css | |||
| .pt-intent-success - Success intent | |||
| .pt-intent-warning - Warning intent | |||
| .pt-intent-danger - Danger intent | |||
| .pt-rose - Rose | |||
| .pt-cobalt - Cobalt | |||
There was a problem hiding this comment.
👎 these are not real modifiers; they cannot appear in the modifier table.
There was a problem hiding this comment.
Yup that was just for showcase in the PR
| background: none; | ||
| cursor: pointer; | ||
| padding: $tag-padding; | ||
| @mixin custom-tag($color, $dark-color, $background-color) { |
There was a problem hiding this comment.
call this mixin tag-colors() and use it to implement intents above.
There was a problem hiding this comment.
I tried to do that yesterday, but it's actually not that easy because this mixin implement its own pt-minimal and pt-dark which would conflict with other usages above
- they're all pt-tag-* now to indicate that they're somewhat public. - this is a Sass API break but those are unsupported so it's chill. - intent coloring has been pulled out to mixins--they don't come for free with the base mixin and that's ok.
| &:not([class*="pt-intent-"]) { | ||
| background: rgba($gray3, 0.2); | ||
| background-color: rgba($gray3, 0.2); |
There was a problem hiding this comment.
Out of scope for this PR, but whoa, didn't realize we were using a non-standard color for minimal-tag backgrounds. Reasoning for not just using $light-gray4?
There was a problem hiding this comment.
If you put a $light-gray4 tag on top of a $light-gray4 background, you end up with a problem 🎩 Transparency keeps this safe no matter what the background color
| color: map-get($pt-intent-text-colors, $intent); | ||
| } | ||
| @mixin pt-tag-minimal-intent($background-color, $text-color, $dark-text-color) { | ||
| background-color: rgba($background-color, 0.15); |
There was a problem hiding this comment.
What if I want an exact color for the background (e.g. some really light color from my style guide's color palette)?
There was a problem hiding this comment.
Fair point, but not a recommended practice for the same reasoning than above
Fixes #606
Changes proposed in this pull request:
pt-tag[-*]()now to indicate that they're sort-of public.-intentand-minimal-intentmixins for custom coloring, and use them to generate our own intent styles.