Provide variables for theme-colors

[kriim]

Pull request #22836 introduced the two color maps colors and theme-colors:

$colors: (
  blue: $blue,
  indigo: $indigo,
  purple: $purple,
  pink: $pink,
  red: $red,
  orange: $orange,
  yellow: $yellow,
  green: $green,
  teal: $teal,
  cyan: $cyan,
  white: $white,
  gray: $gray-600,
  gray-dark: $gray-800
) !default;

$theme-colors: (
  primary: $blue,
  secondary: $gray-600,
  success: $green,
  info: $cyan,
  warning: $yellow,
  danger: $red,
  light: $gray-100,
  dark: $gray-800
) !default;

While updating a test project to v4-beta I noticed that it isn't possible anymore to override the theme-colors properly. To override theme-colors.primary one had to change the variable blue.

Would it be possible to introduce variables for the theme-colors which are then used in the color map? This way one could override single elements of the theme colors without doing strange things like $blue: red; or having to redefine the whole theme-colors map (while making sure that all other used variables are already defined, e.g. $green).

My suggestion is to do something like this:

$primary: $blue !default;
$secondary: $gray-600 !default,
$success: $green !default,
$info: $cyan !default,
$warning: $yellow !default,
$danger: $red !default,
$light: $gray-100 !default,
$dark: $gray-800 !default

$theme-colors: (
  primary: $primary,
  secondary: $secondary,
  success: $success,
  info: $info,
  warning: $warning,
  danger: $danger,
  light: $light,
  dark: $dark
) !default;

[andresgalante]

Hi @kriim, I am trying to understand the use case:
If you want to change primary to red, instead of dong $blue: red you'd do:

$theme-colors: (
  primary: red,
  ....
}

What am I missing?

[kriim]

Ah, sorry, I should have mentioned this. I did this without touching the actual Bootstrap Sass files.

I included Bootstrap using npm as I don't want to maintain a copy of the bootstrap files if possible.

Assume a src/css/custom-bootstrap.scss like this:

$primary: red;
@import "../../node_modules/bootstrap/scss/bootstrap.scss";

For the sake of completeness, the following worked fine with v4-alpha.6:

$brand-primary: red;
@import "../../node_modules/bootstrap/scss/bootstrap.scss";

[andresgalante]

Thanks for the clarification, 👍
I see what you mean. I think this is a common use case.
Why don't you send a PR and see if it has legs? or I can open it if yo want

[andresgalante]

@kriim This is what you have in mind, right?
https://github.com/andresgalante/bootstrap/blob/color-variable/scss/_variables.scss#L64-L83

[kriim]

@andresgalante Yes, that is exactly what I had in mind 👍.

Thanks for the quick resolution :). If I had seen your reply earlier, I also would have been happy to send a PR.

[andresgalante]

Sorry, this is your idea, if you want close my PR and send it yourself.

[kriim]

I'm totally fine with that ;). That was sort of an excuse that I haven't done it myself.

[kriim]

After looking at your changes I saw that #23260 already introduced a way to customize the theme colors, the following works also fine (will get into beta 2):

$theme-colors: (
    primary: red
);

But there was some discussion on #23260 if the variables should exist anyways.

@mdo commented:

You don't have to pull from the map at all times—we have all the variables there still. We use the Sass map to reduce overhead and iterate over all our variables to generate every component's themed modifier classes

So I guess we can include the theme variables ...

[andresgalante]

👍 make sense, I'll close the PR

[kriim]

By 'I guess we can include the theme variables' I actually meant that we could merge your PR :).

As I understood the comments on #23260 the maps are only for looping over the theme colors to generate button variants etc.

The variables may still be useful for people who are defining their own Sass styles based on Bootstrap:

.my-class {
  color: $primary;
}

(It's easier than getting the values from a map)

[andresgalante]

right, I'll leave it open and let mdo decide.

[kriim]

Yes 👍 , sorry for the confusion.

[andresgalante]

@kriim I just had to set a $blue: red type os situation. You are right, this is something that needs fix. I hope #23918 it gets merge

[mdo]

I understand the appeal of separate variables, but it definitely diminishes the usefulness of the maps. Say you want to change a color—that's straightforward enough. But if you want to add a new one, you have to declare the variable and add it to the map. Not a ton of work, but it takes away some of the ease of use here for what amounts to a syntax change for snagging a color.

[jcoyne]

How about separating the variables from the colors file?
Then you could customize the primary color by doing:

@include 'bootstrap/colors'

$theme-colors: () !default;
$theme-colors: map-merge((
  "primary":    $my-custom-color,
  "secondary":  $gray-600,
  "success":    $color3,
  "info":       $cyan,
  "warning":    $yellow,
  "danger":     $red,
  "light":      $gray-100,
  "dark":       $gray-800
), $theme-colors);

@include 'bootstrap/variables'

Presently we can't $theme-colors prior to bootstrap/variables, because the colors used to define it are in bootstrap/variables

EDIT: Apparently 0bc39aa fixes this issue, but it didn't ship in Beta 1.