Skip to content
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

Refactor tags to support custom colors #668

Merged
merged 10 commits into from
Feb 24, 2017
126 changes: 59 additions & 67 deletions packages/core/src/components/tag/_common.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// styles for a default .pt-tag
@mixin tag($removable-selector: "&.pt-tag-removable") {
@mixin tag() {
display: inline-block;
position: relative;
border: none;
border-radius: $pt-border-radius;
box-shadow: none;
background: $gray1;
background-color: $gray1;
min-width: $tag-height;
padding: $tag-padding ($tag-padding * 3);
line-height: $pt-icon-size-standard;
Expand All @@ -30,26 +32,51 @@ $tag-padding-large: ($tag-height-large - $pt-icon-size-large) / 2 !default;
}

.pt-dark & {
&:not([class*="pt-intent-"]) {
background: $gray5;
color: $pt-text-color;
}
}

#{$removable-selector} {
padding-right: $tag-height;
background-color: $gray5;
color: $pt-text-color;
}

@each $intent, $color in $pt-intent-colors {
&.pt-intent-#{$intent} {
background: $color;
background-color: $color;
color: $white;
}
}

#{$tag-removable-selector} {
padding-right: $tag-height;

.pt-tag-remove {
@include pt-icon();

position: absolute;
top: 0;
right: 0;
opacity: 0.5;
border: none;
background: none;
cursor: pointer;
padding: $tag-padding;
color: inherit;

&:hover {
opacity: 0.8;
background: none;
text-decoration: none;
}

&:active {
opacity: 1;
}

&::before {
content: $pt-icon-small-cross;
}
}
}
}

// updated properties for a .pt-tag.pt-large
@mixin tag-large($removable-selector: "&.pt-tag-removable") {
@mixin tag-large() {
min-width: $tag-height-large;
padding: $tag-padding-large ($tag-padding-large * 2);
line-height: $pt-icon-size-large;
Expand All @@ -59,84 +86,49 @@ $tag-padding-large: ($tag-height-large - $pt-icon-size-large) / 2 !default;
border-radius: $tag-height-large / 2;
}

#{$removable-selector} {
#{$tag-removable-selector} {
padding-right: $tag-height-large;

.pt-tag-remove {
@include pt-icon-sized($pt-icon-size-large);

padding: $tag-padding-large;
}
}
}

@mixin tag-minimal() {
&:not([class*="pt-intent-"]) {
background: rgba($gray3, 0.2);
background-color: rgba($gray3, 0.2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: $pt-text-color;

.pt-dark & {
background: rgba($gray3, 0.2);
color: $pt-dark-text-color;
}
}

@each $intent, $color in $pt-intent-colors {
&.pt-intent-#{$intent} {
background: rgba($color, 0.15);
background-color: rgba($color, 0.2);
color: map-get($pt-intent-text-colors, $intent);
}

.pt-dark &.pt-intent-#{$intent} {
background: rgba($color, 0.2);
color: map-get($pt-dark-intent-text-colors, $intent);
.pt-dark & {
color: map-get($pt-dark-intent-text-colors, $intent);
}
}
}
}

// styles for default .pt-tag-remove
@mixin tag-remove() {
@include pt-icon();

position: absolute;
top: 0;
right: 0;
opacity: 0.5;

border: none;
background: none;
cursor: pointer;
padding: $tag-padding;
@mixin custom-tag($color, $dark-color, $background-color) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call this mixin tag-colors() and use it to implement intents above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

background: $background-color;
color: $white;

&:hover {
opacity: 0.8;
background: none;
text-decoration: none;
}

&::before {
content: $pt-icon-small-cross;
}

// standard (non-minimal) tags have a unique text color in dark theme.
// intent tags don't change in the dark theme.
.pt-dark .pt-tag:not(.pt-minimal):not([class*="pt-intent-"]) & {
color: $pt-text-color;
}
}

// updated properties for .pt-tag.pt-large .pt-tag-remove
@mixin tag-remove-large() {
@include pt-icon-sized($pt-icon-size-large);
margin-left: $pt-grid-size / 2;
padding: $tag-padding-large;
}

@mixin tag-remove-minimal() {
color: inherit;
&.pt-minimal {
background-color: rgba($background-color, 0.2);
color: $color;

@each $intent, $color in $pt-intent-colors {
.pt-intent-#{$intent} & {
color: $color;
}

.pt-dark .pt-intent-#{$intent} & {
color: $color;
.pt-dark & {
color: $dark-color;
}
}
}
25 changes: 12 additions & 13 deletions packages/core/src/components/tag/_tag.scss
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ Markup:
.pt-intent-success - Success intent
.pt-intent-warning - Warning intent
.pt-intent-danger - Danger intent
.pt-rose - Rose
.pt-cobalt - Cobalt

Styleguide components.tag.css
*/
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👎 these are not real modifiers; they cannot appear in the modifier table.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup that was just for showcase in the PR


Styleguide components.tag.css.minimal
*/
Expand All @@ -89,19 +93,6 @@ Styleguide components.tag.css
}
}

.pt-tag-remove {
@include tag-remove();

.pt-large & {
@include tag-remove-large();
}

.pt-minimal & {
@include tag-remove-minimal();
}
}


/*
JavaScript API

Expand Down Expand Up @@ -129,3 +120,11 @@ Blueprint class name.

Styleguide components.tag.js
*/

.pt-tag.pt-rose {
@include custom-tag($rose2, $rose4, $rose3);
}

.pt-tag.pt-cobalt {
@include custom-tag($cobalt2, $cobalt4, $cobalt3);
}