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

How to override theme colors now? #23112

Closed
martinbean opened this issue Jul 17, 2017 · 27 comments
Closed

How to override theme colors now? #23112

martinbean opened this issue Jul 17, 2017 · 27 comments

Comments

@martinbean
Copy link
Contributor

martinbean commented Jul 17, 2017

I’ve noticed in _variables.scss that the brand colours have been moved to a nested object.

Up to now, I‘ve been specifying variable overrides in a _custom.scss in my projects like this:

@import "custom";
@import "node_modules/bootstrap/scss/bootstrap";

I’d then override the variables I wanted in my _custom.scss file like this:

$brand-primary: #fc0;

$font-family-sans-serif: "Open Sans", sans-serif;

How do I override values on a per-variable basis now that they’re inside an object? For example, I have a project where I want the brand’s primary color to be green and not blue.

Sorry if this a n00b question, but I’m under the impression that object members like $theme-colors[primary] aren’t overridable in Sass without overriding the whole object, in which case these seriously reduces the configurability of Bootstrap in version 4.

If I’m doing something wrong, or I can override specific members of objects in Sass, then please let me know.

@bkdotcom
Copy link
Contributor

http://sass-lang.com/documentation/Sass/Script/Functions.html#map_merge-instance_method

@mdo
Copy link
Member

mdo commented Jul 17, 2017

While it does appear you cannot override part of the Sass maps, much like our variables, they do have the !default flag. See this example for how you can change the variable or Sass map. IMO this is a fine approach as it maps to variables—copy, paste, and change the value.

How do I override values on a per-variable basis now that they’re inside an object?

We also still have all the regular color variables, so you can customize the Sass map or you can change the base variables. See the variables file:

$blue:    #007bff !default;
$indigo:  #6610f2 !default;
$purple:  #6f42c1 !default;
$pink:    #e83e8c !default;
$red:     #dc3545 !default;
$orange:  #fd7e14 !default;
$yellow:  #ffc107 !default;
$green:   #28a745 !default;
$teal:    #20c997 !default;
$cyan:    #17a2b8 !default;

$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;

@mdo mdo closed this as completed Jul 17, 2017
@martinbean
Copy link
Contributor Author

martinbean commented Jul 18, 2017

@mdo I get that the theme colours have been mapped to “named” colour variables, but if I want my theme to be primarily green, I don’t want to be changing the value of $blue to be a green since $theme-colors[primary] is mapped to $blue.

I’m not liking the fact I have to copy-and-paste and override X, Y and Z just to change one setting. What happens if the keys in the map change? My theme’s going to break. Why do I have all these dependencies just to change one single colour in Bootstrap?

@dushkostanoeski
Copy link

@martinbean you don't need to change the value of $blue to green, you only have to change the primary color to $green in $theme-colors

@martinbean
Copy link
Contributor Author

martinbean commented Jul 18, 2017

@dushkostanoeski But I have to copy the whole of $theme-colors map into my _custom.scss file, and then also also the individual colour variables ($blue etc) because they’re referenced in the map.

All that, just to change one colour!

@houfio
Copy link

houfio commented Jul 18, 2017

You don't have to import the color variables if you import the _variables.scss file in your custom.scss file

@martinbean
Copy link
Contributor Author

@houfio That defeats the point of them having the !default flag if I just have to import them and override them any way?

@houfio
Copy link

houfio commented Jul 18, 2017

I guess so. Are you sure you have to copy them if you don't reference the file? It should work fine, because the bootstrap base file imports it too.

@martinbean
Copy link
Contributor Author

@houfio Did you take a look at my original post? I define custom variable values in a _custom.scss file that‘s included before Bootstrap. It’s in fact, what the comments say to do in the _variables.scss file: https://github.com/twbs/bootstrap/blob/v4-dev/scss/_variables.scss#L3-L4

However, with colours now being nested in a map, it’s created this awkward scenario where I need to copy whole blocks of variable definitions and hope Bootstrap doesn’t change the name or “shape” of the map (which is has been known to do in the past); or include its variables files before mine, making the point specifying values to override the defaults pointless.

@houfio
Copy link

houfio commented Jul 18, 2017

I know it it included before bootstrap, but because of the !default, it doesn't reassign the variable and uses the one in your _custom.scss file. Your _custom.scss file can't find the color variables, but iirc that shouldn't be a problem, since the map gets accessed in files where the colors are imported (unless I'm missing another problem)

@martinbean
Copy link
Contributor Author

martinbean commented Jul 18, 2017

@houfio My problem is, I just want to set the primary brand colour to be something other than blue. Before, I could have a _custom.scss file with this in:

$brand-primary: #006400;

That was it. Everything the “primary” colour (buttons etc) would then be a dark green when Bootstrap was compiled.

Now I can’t do that as that variable’s been removed and instead the theme colours folded into a map.

@houfio
Copy link

houfio commented Jul 18, 2017

If you don't need any of the other colors you can just do

$theme-colors: (
  primary: #006400
)

since all the calls to the map are loops.

@martinbean
Copy link
Contributor Author

martinbean commented Jul 18, 2017

@houfio I do want the other colours though! I just want to override one of the theme’s colours, and not have to copy a whole map to do so!

@houfio
Copy link

houfio commented Jul 18, 2017

I suppose this change is good if you want much customization, but not so much if you just want to change one color. But copying a map isn't that bad, is it?

You can also just import the variables file, which does defeat the point of the !default, but allows you to use the map_merge method.

@martinbean
Copy link
Contributor Author

@houfio My gripe with copying a whole map (and its dependent variables) is that, I’m then relying on Bootstrap not to change anything, which was the point of having a file of variable overrides in the first place. It just seems I have to go “too deep” into Bootstrap now to change one thing.

@fluke
Copy link

fluke commented Jul 20, 2017

This change makes overriding not work. Coming from a Rails example. I too want to change my theme to green. The following doesn't work. And it's not feasible for me to go and change the variables.scss

// Custom bootstrap variables must be set or imported *before* bootstrap.
$green: #89B630;

$theme-colors: (
  primary: $green
);

@import "bootstrap";

Currently have to change $blue to get the correct result.

@cr101
Copy link

cr101 commented Jul 20, 2017

@kartikluke Ideas in #22891 might help

@martinbean
Copy link
Contributor Author

@cr101 Doesn’t really help, as it seems to be people discussing the same issue as in here.

@mdo
Copy link
Member

mdo commented Jul 20, 2017

Bringing over the entire Sass map doesn't seem unreasonable to me... is the biggest concern that this feels heavy handed when compared to the old $brand-{color} variables?

@martinbean
Copy link
Contributor Author

martinbean commented Jul 20, 2017

is the biggest concern that this feels heavy handed when compared to the old $brand-{color} variables?

@mdo Exactly that. When overriding a $brand-{color} variable, I’m overriding what I want, and only what I want. To copy over a whole Sass map just to change one colour in that map just seems overly cumbersome, especially when I also have to re-define the individual colour variables referenced in that map, i.e. $blue et al. So changing the primary color goes from this:

$brand-primary: #006600;

To this:

$gray-100: #f8f9fa;
$gray-600: #868e96;
$gray-800: #343a40;

$red: #dc3545;
$yellow: #ffc107;
$green: #006600; // This is the only variable I wanted to change
$cyan: #17a2b8;

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

@bkdotcom
Copy link
Contributor

@martinbean

will this not work?

$theme-colors = map_merge($theme-colors, {success: $green});

@martinbean
Copy link
Contributor Author

@bkdotcom Not unless I import the whole of _variables.scss first 😩

@fluke
Copy link

fluke commented Jul 21, 2017

@martinbean @mdo The solution outlined here seems to be the best option. #22891 (comment)

// In a custom variables file...
$theme-colors: (
  primary: red
);


// In Bootstrap's _variables.scss file...
$theme-colors: () !default;
$theme-colors: map-merge((
  primary: $blue,
  secondary: $gray-600,
  success: $green,
  info: $cyan,
  warning: $yellow,
  danger: $red,
  light: $gray-100,
  dark: $gray-800
), $theme-colors);

@mdo
Copy link
Member

mdo commented Jul 25, 2017

@kartikluke Nice! That does work: https://codepen.io/anon/pen/XaJeVd. I'll see about a PR shortly.

@fluke
Copy link

fluke commented Jul 25, 2017

@hokiecoder figured it out. This should make it much easier to work with the new maps.

@mdo
Copy link
Member

mdo commented Aug 9, 2017

Please see #23260 for @hokiecoder's suggestion that @kartikluke pointed out here. Any feedback is most definitely welcomed!

@aManNamedJed
Copy link

I understand the concern. I honestly like this map feature.

@twbs twbs locked and limited conversation to collaborators Nov 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants