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.

Sign up for GitHub

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

Switch to custom properties for colorizing buttons #30500

Closed
wants to merge 1 commit into from
Closed

Switch to custom properties for colorizing buttons #30500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 82 additions & 21 deletions scss/_buttons.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,48 @@

.btn {
display: inline-block;
padding: $btn-padding-y $btn-padding-x;
@include font-size($btn-font-size);
font-family: $btn-font-family;
font-weight: $btn-font-weight;
line-height: $btn-line-height;
color: $body-color;
color: var(--btn-accent-contrast-color);
text-align: center;
text-decoration: if($link-decoration == none, null, none);
white-space: $btn-white-space;
vertical-align: middle;
cursor: if($enable-pointer-cursor-for-buttons, pointer, null);
user-select: none;
background-color: transparent;
border: $btn-border-width solid transparent;
@include button-size($btn-padding-y, $btn-padding-x, $btn-font-size, $btn-border-radius);
background-color: var(--btn-accent-color);
border: $btn-border-width solid $btn-border-color;

@include transition($btn-transition);
@include gradient-bg();
@include border-radius($btn-border-radius, 0); // Manually declare to provide an override to the browser default
@include box-shadow($btn-box-shadow);

&:hover {
color: $body-color;
color: var(--btn-accent-contrast-color);
text-decoration: if($link-hover-decoration == underline, none, null);
}

&:focus,
&.focus {
outline: 0;
box-shadow: $btn-focus-box-shadow;

// Avoid using mixin so we can pass custom focus shadow properly
@if $enable-shadows {
box-shadow: $btn-box-shadow, $btn-focus-box-shadow;
} @else {
box-shadow: $btn-focus-box-shadow;
}
}

&:active,
&.active {
&.active,
.show > &.dropdown-toggle {
// Remove CSS gradients if they're enabled
background-image: if($enable-gradients, none, null);
@include box-shadow($btn-active-box-shadow);

&:focus {
Expand All @@ -43,38 +57,82 @@
&.disabled,
fieldset:disabled & { // stylelint-disable-line selector-no-qualifying-type
pointer-events: none;
// Remove CSS gradients if they're enabled
background-image: if($enable-gradients, none, null);
opacity: $btn-disabled-opacity;
@include box-shadow(none);
}
}

.btn-outline {
color: var(--btn-accent-color);
background-color: transparent;
border-color: currentColor;

&:hover {
background-color: var(--btn-accent-color);
Copy link
Contributor

Choose a reason for hiding this comment

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

If we put here --btn-accent-color for hover background-color and for color library user can't make these values different without styles overwrite or setting own selectors with variables values.

For example I want to set color as red, but background-color as green
I will need to write:

.btn-outline {
  --btn-accent-color: red;
  &:hover {
    --btn-accent-color: green;
  }
}

I think better do config just with set of variables without writing new nested selectors. What do you think?
And, btw, isn't there color will be the same as background color or not?

Copy link
Contributor

@ydmitry ydmitry Sep 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

If we use approach similar to #31708 with placing in SCSS styles only SCSS variables, but in SCSS variables set CSS variables, we could do:

// Styles
.btn-outline {
  color: $btn-color;
  &:hover {
     background-color: $btn-hover-bg-color;
  }
}

// Default variables, will do the same as in PR
$btn-color: var(--btn-accent-color) !default;
$btn-hover-bg-color: var(--btn-accent-color) !default;

// In application can changed to e.g.:
$btn-color: var(--btn-color);
$btn-hover-bg-color: var(--btn-hover-bg-color);

// In application custom styles - setting variant without knowing selectors structure
.btn-custom {
  --btn-color: red;
  --btn-hover-color-bg: green;
}

border-color: var(--btn-accent-color);
}

&:focus,
&.focus {
color: var(--btn-accent-contrast-color);
background-color: var(--btn-accent-color);
border-color: var(--btn-accent-color);
}

&.active,
&:active {
color: var(--btn-accent-contrast-color);
background-color: var(--btn-accent-color);
border-color: var(--btn-accent-color);
}
Comment on lines +77 to +89
Copy link
Member

Choose a reason for hiding this comment

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

Those are the same, can't we have a single selector stack?

}
Comment on lines +67 to +90
Copy link

Choose a reason for hiding this comment

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

If I am not mistaken, the approach would be something like,

.btn {
  --btn-color: default here;
  --btn-background-color: default here;
  --btn-border-color: currentColor;

  color: var(--btn-color);
  background-color: var(--btn-background-color);
  border-color: var(--btn-border-color);
}

.btn-outline {
  --btn-color: change me;
  --btn-background-color: change me;
  --btn-border-color: change me;

  &:hover {
     --btn-color: change me;
     --btn-background-color: change me;
     --btn-border-color: change me;
  }

  &:focus,
  &.focus {
      --btn-color: default here;
      --btn-background-color: default here;
      --btn-border-color: currentColor;
  }

  &.active,
  &:active {
      --btn-color: change me;
      --btn-background-color: change me;
      --btn-border-color: change me;
  }
}

the idea is that you focus more on swapping the values than overriding styles.

At least that is how I understand it.


//
// Alternate buttons
//

@each $color, $value in $theme-colors {
.btn-#{$color} {
@include button-variant($value, $value);
}
}
$contrast-color: color-contrast($value);

@each $color, $value in $theme-colors {
.btn-outline-#{$color} {
@include button-outline-variant($value);
--btn-accent-color: #{$value};
Copy link

Choose a reason for hiding this comment

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

I'm a bit confused by the fact that the variables are set here. The nice thing about CSS variables are that they cascade through the DOM hierarchy, so that devs can set these e.g. at the :root or some other ascendant of a button, and every button (and other element that was configured in the ancestor) below that ancestor will use the same theme - without having to use any descendant selectors for .btn etc.

But if the variable is set here on the .btn element, then it seems that wouldn't work, because it would always override what was set in an ancestor. So to make it work, define a theme for all descendant Bootstrap components at an ancestor element, it seems we'd have to use specific descendant selectors like .btn again to override the variables. That kind of defeats the purpose of the variables mostly.

How I imagine it, components like .btn would only read variables, and they are set e.g. on :root or any ancestor by devs (and Bootstrap can provide defaults on those and integrate them there with the Sass variables).

Copy link
Contributor

@ydmitry ydmitry Sep 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I have the same impression, looks like CSS custom properties here used to refactor code in a cool way, not to increase customization capabilities and flexibility...

So, it's improves only internal Bootstrap development experience, decreases bundles size, provides some additional usages of modifiers, but not improves customization.

--btn-accent-contrast-color: #{$contrast-color};

&:hover {
$accent-hover: darken($value, 5%);
$accent-hover-contrast-color: color-contrast($accent-hover);

--btn-accent-color: #{$accent-hover};
@if ($contrast-color != $accent-hover-contrast-color) {
--btn-accent-contrast-color: #{$accent-hover-contrast-color};
}
}

&.active,
&:active {
$accent-active: darken($value, 15%);
$accent-active-contrast-color: color-contrast($accent-active);

--btn-accent-color: #{$accent-active};
@if ($contrast-color != $accent-active-contrast-color) {
--btn-accent-contrast-color: #{$accent-active-contrast-color};
}
}
}
}


//
// Link buttons
//

// Make a button look and behave like a link
.btn-link {
font-weight: $font-weight-normal;
font-weight: $font-weight-base;
color: $btn-link-color;
text-decoration: $link-decoration;
border-color: if($btn-border-color != transparent, transparent, null);

&:hover {
color: $btn-link-hover-color;
Expand All @@ -84,27 +142,30 @@
&:focus,
&.focus {
text-decoration: $link-hover-decoration;
box-shadow: 0 0 0 $btn-focus-offset $body-bg, 0 0 0 $btn-focus-width;
}

&:disabled,
&.disabled {
color: $btn-link-disabled-color;
}

// No need for an active state here
}


//
// Button Sizes
//

.btn-lg {
@include button-size($btn-padding-y-lg, $btn-padding-x-lg, $btn-font-size-lg, $btn-border-radius-lg);
.btn-sm {
padding: $btn-padding-y-sm $btn-padding-x-sm;
@include font-size($btn-font-size-sm);
@include border-radius($btn-border-radius-sm);
}

.btn-sm {
@include button-size($btn-padding-y-sm, $btn-padding-x-sm, $btn-font-size-sm, $btn-border-radius-sm);
.btn-lg {
padding: $btn-padding-y-lg $btn-padding-x-lg;
@include font-size($btn-font-size-lg);
@include border-radius($btn-border-radius-lg);
}


Expand Down
1 change: 0 additions & 1 deletion scss/_mixins.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

// Components
@import "mixins/alert";
@import "mixins/buttons";
@import "mixins/caret";
@import "mixins/pagination";
@import "mixins/lists";
Expand Down
4 changes: 3 additions & 1 deletion scss/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,12 +534,14 @@ $btn-padding-y-lg: $input-btn-padding-y-lg !default;
$btn-padding-x-lg: $input-btn-padding-x-lg !default;
$btn-font-size-lg: $input-btn-font-size-lg !default;

$btn-border-color: rgba($black, .01) !default;
$btn-border-width: $input-btn-border-width !default;

$btn-font-weight: $font-weight-normal !default;
$btn-box-shadow: inset 0 1px 0 rgba($white, .15), 0 1px 1px rgba($black, .075) !default;
$btn-focus-width: $input-btn-focus-width !default;
$btn-focus-box-shadow: $input-btn-focus-box-shadow !default;
$btn-focus-offset: $input-btn-focus-width / 2 !default;
$btn-focus-box-shadow: 0 0 0 $btn-focus-offset, 0 0 0 $input-btn-focus-width var(--btn-accent-color) !default;
$btn-disabled-opacity: .65 !default;
$btn-active-box-shadow: inset 0 3px 5px rgba($black, .125) !default;

Expand Down
121 changes: 0 additions & 121 deletions scss/mixins/_buttons.scss
View file
Open in desktop

This file was deleted.

10 changes: 5 additions & 5 deletions scss/mixins/_gradients.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// Gradients

@mixin gradient-bg($color, $foreground: null) {
@mixin gradient-bg($color: null, $foreground: null) {
background-color: $color;

@if $enable-gradients {
@if $foreground {
background-image: $foreground, linear-gradient(180deg, mix($body-bg, $color, 15%), $color);
background-image: $foreground, linear-gradient(180deg, rgba($color, .15), transparent);
} @else {
background-image: linear-gradient(180deg, mix($body-bg, $color, 15%), $color);
background-image: linear-gradient(180deg, rgba($body-bg, .15), transparent);
}
} @else {
background-color: $color;
}
}

Expand Down
4 changes: 2 additions & 2 deletions site/content/docs/4.3/components/buttons.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ When using button classes on `<a>` elements that are used to trigger in-page fun

## Outline buttons

In need of a button, but not the hefty background colors they bring? Replace the default modifier classes with the `.btn-outline-*` ones to remove all background images and colors on any button.
In need of a button, but not the hefty background colors they bring? Replace the default modifier classes with the `.btn-outline btn-*` ones to remove all background images and colors on any button.

{{< example >}}
{{< buttons.inline >}}
{{- range (index $.Site.Data "theme-colors") }}
<button type="button" class="btn btn-outline-{{ .name }}">{{ .name | title }}</button>
<button type="button" class="btn btn-outline btn-{{ .name }}">{{ .name | title }}</button>
{{- end -}}
{{< /buttons.inline >}}
{{< /example >}}
Expand Down
Loading