-
-
Notifications
You must be signed in to change notification settings - Fork 79k
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
Updated .bg-* and .text-* utilities break when CSS variables are not enabled in v5.1.0 #34756
Comments
|
The whole bg-, text- update in 5.1.0 has seemingly complicated the entire process of adding a new theme-color. The only way I could find to get the bg- and text- classes regenerated for the new "custom" theme color is....
I discovered this on an SO question and here's the working code demo Whereas in 5.0.2 one could simply follow the example in the docs...
|
In addition to that, the suggested way of extending color utility classes generates HEX color values instead of RGBA and still keeps useless |
Thanks! |
I consider this either a regression or a lack in documentation.
|
Oh, and btw the same is true if you need to add spacing classes:
|
arrrggghhhh add +1 to frustration of note. this change has had me tearing my hair out. i sort of got it working with _variables.scss (my custom variables)
styles.scss (main style)
at the devs, its changes like this that might actually drive people away tho. would suggest you consider "ability and ease of extending bootstrap" as a core just like getting the code working right. i know people might disagree with this, but this change for instance has some what had me questioning if bootstrap and me remain in a loving relationship or not). 12k lines of css :( last i looked it was around 8 or so |
Sorry we've been slow to investigate and identify a path forward. We'll need a little more time to experiment and validate the issues identified here. Definitely not in our interest to be causing headaches for folks :). |
Hehe thanks mdo. I think the 5.1 update took a few people by surprise. Some "breaking" things like this but on the face of it it looks like 5.0 but it's still a big update. |
Left a comment at #34734 (comment), but cross posting here and closing that as a duplicate. Please read and let me know what you all think about that potential solution. Seems like the best path forward to me so far. After investigating a bit more, I think the best path forward might to split out some of our reassigned Sass maps into a separate For example: // Configuration
@import "functions";
@import "variables";
$starorange: #df711b !default;
$custom-theme-colors: (
"starorange": $starorange
) !default;
$theme-colors: map-merge($theme-colors, $custom-theme-colors); // stylelint-disable-line scss/dollar-variable-default
@import "maps";
@import "mixins";
@import "utilities";
// Layout & components
// ... Inside |
Hi @mdo, |
@isaumya This is why: https://getbootstrap.com/docs/5.1/utilities/colors/#opacity. It allows for on the fly manipulation of colors. We've stated we're heading to more CSS variables since developing v5, and there's more coming for components (see #34443, #34600, and #34622). |
@mdo I think lots of people are missing the whole point of introducing RGBa with opacity CSS var. If i understand correctly, this is needed for new text and background opacity logic. But the same can be achieved with regular |
I love bootstrap but yikes. This all feels like woosh direction gone. I trust you guys to do what's best for the project and I'm in no position to know the big picture with it, but eish. To a long time user at this point I'm just going "I trust the devs" more than what I trust what I'm seeing. Maybe consider splitting bs up more or having opt in on stuff? The "core" compiled bs with very basic components is still huge. How many projects does the RGB thing even help? Maybe with 6 go back to reboot then add stuff to it? Thanks team. I appreciate the work tho! |
First up, it’s definitely my fault that these problems have occurred. I didn’t test these changes enough. We’ll be looking into some Sass testing we can add to help avoid situations like this in the future.
HSL is great, but setting colors up with three variables instead of one feels overkill. Experience has shown me most folks are familiar with hex and RGB, so we went that route. The goal wasn’t to really extend the palette—that’s optional for everyone, and the very use case we want to solve for with this issue—but rather to make components and utilities stronger and easier to customize. These RGB values also accomplish what you want with the alpha of HSLA—that’s how we use them for bg and color utilities now.
The same cannot be achieved with opacity utilities. These variable/utility changes apply to
Considering we only shipped it a month ago, I’m not entirely sure yet :). Give it time for folks to get used to though. For what it’s worth, Tailwind does something like this already and it seems super helpful. Give it a go and let me know what you think.
There won’t be any “going back” to Reboot. Reboot came out of Normalize.css and Bootstrap 4. We never started with that :). Bootstrap has been a compilation of components, layout, plugins, and more for the last decade. It’ll most likely stay that way for the next ;). Regarding file size, if you can find a way to decrease file size while doing all Bootstrap does, I’d love to hear about it :). We try to keep it lean but convenient. That aside, doing a custom build and using PurgeCSS is a great way to streamline it all :). |
That's my biggest hassle with this change, as CSS variables alone account of over 50-60kb (uncompressed). Would it be somehow feasible to add an option to include/exclude CSS variables? |
That could be possible. Where are you getting 50-60kb from though? It's maybe a few if you include literally every one of them. |
I just compared the final result with and without including |
|
In fact you are right, I just compared 5.0.2 without
Minifying this will give me a 6kb difference. So the above mentioned difference of 50-60kb was most likely due to not importing other Bootstrap components. |
Should this issue not be fixed in a 5.1.2?
This doesn't work entirely for These map-merges inside |
I know @tomsommer, @import "functions";
// Overwrite your variables
$custom-color1: red !default;
$custom-color2: blue !default;
$min-contrast-ratio: 3;
$link-decoration: none;
// Import the bootstrap variables
@import "variables";
$extra-theme-colors: (
"gray-dark": $gray-800,
"custom-color1": $custom-color1
);
$extra-general-colors: (
"custom-color2": $custom-color2
);
// Run map-merge() to mer our custom maps with the default Bootstrap maps
$colors: map-remove( $colors, "gray-dark" );
$colors: map-merge( $colors, $extra-general-colors );
$theme-colors: map-merge($theme-colors, $extra-theme-colors );
// ---------- MOST IMPORTANT ---------------
// Extra stuffs we need to do for generating the proper theme color classes (.bg-*, .text-*)
$theme-colors-rgb: map-loop($theme-colors, to-rgb, "$value");
$utilities-text: map-merge($theme-colors, $utilities-text);
$utilities-text-colors: map-loop($utilities-text, rgba-css-var, "$key", "text");
$utilities-bg: map-merge($theme-colors, $utilities-bg);
$utilities-bg-colors: map-loop($utilities-bg, rgba-css-var, "$key", "bg");
// And then continue importing the rest
@import "mixins";
@import "utilities";
// and so on... This will work. Try and enjoy. :) Fun fact: I faced the same |
The "problem" goes even beyond the maps. Modifying anything in Say I do Really something to think about how to make easier, my two cents. Or maybe we are just limited by the |
Yeah, this was definitely a shortcoming of Sass we didn’t fully explore at the time. It’s something we’ll 100% have to revisit in v6, but until then, I want to make sure we consider the best options and ship something that’s functional and as easy to document/explain as possible. What do folks think of that PR I opened to separate out Sass maps? I might need to include more maps there like |
I like the idea of splitting it to its own files. But would rather say that it should be split in such a way that if you don't need colours then don't include it (spaces, colours, et al) I honestly prefer "more" files vs larger files in cases like this. If the pr fixes the maps thing then I'm 100% behind it. Just gotten bitten again with a .bg-white suddenly disappearing on update again and used many short words again. (Just had to add the stuff in my custom variables file as per above again) |
Here's what the diff will look like when importing and customizing Bootstrap after #34942 is merged. Taking the example above, we add a custom variable, put that in it's own map, and then // Functions come first
@import "functions";
// Optional variable overrides here
+ $custom-color: #df711b;
+ $custom-theme-colors: (
+ "custom": $custom-color
+ );
// Variables come next
@import "variables";
+ // Optional Sass map overrides here
+ $theme-colors: map-merge($theme-colors, $custom-theme-colors);
+
+ // Followed by our default maps
+ @import "maps";
+
// Rest of our imports
@import "mixins";
@import "utilities";
@import "root";
@import "reboot";
// etc Here's a quick output of a debug test I did. As you can see, it outputs the correct keys for all maps that consume @import "../scss/functions";
$custom-color: #df711b;
$custom-theme-colors: (
"custom": $custom-color
);
@import "../scss/variables";
$theme-colors: map-merge($theme-colors, $custom-theme-colors);
@import "../scss/maps";
@debug "The following three maps should be the same, ending with the custom key.";
@debug map.keys($theme-colors);
@debug map.keys($theme-colors-rgb);
@debug map.keys($utilities-colors);
@debug "";
@debug "The following two maps should be the same: including custom and ending in black, white, and body.";
@debug map.keys($utilities-text-colors);
@debug map.keys($utilities-bg-colors); ➜ ~/work/bootstrap (split-vars-maps-import) npm run css-test
> [email protected] css-test /Users/mdo/work/bootstrap
> sass --style expanded --source-map --embed-sources --no-error-css test/index.scss:test/output.css
test/index.scss:35 Debug: The following three maps should be the same, ending with the custom key.
test/index.scss:36 Debug: "primary", "secondary", "success", "info", "warning", "danger", "light", "dark", "custom"
test/index.scss:37 Debug: "primary", "secondary", "success", "info", "warning", "danger", "light", "dark", "custom"
test/index.scss:38 Debug: "primary", "secondary", "success", "info", "warning", "danger", "light", "dark", "custom"
test/index.scss:40 Debug:
test/index.scss:41 Debug: The following two maps should be the same: including custom and ending in black, white, and body.
test/index.scss:42 Debug: "primary", "secondary", "success", "info", "warning", "danger", "light", "dark", "custom", "black", "white", "body"
test/index.scss:43 Debug: "primary", "secondary", "success", "info", "warning", "danger", "light", "dark", "custom", "black", "white", "body" |
It's certainly better, and certainly an improvement, but you have to ask where the limit go: You can't really include The problem is that people actually change As a quick-fix, the PR is fine, but it really is something that needs a deep think. I think Tailwind has solved all this with Webpack? |
I think test/index.scss:40 Debug: The following spacers should be doubled the default values.
test/index.scss:41 Debug: Default: 0, 0.25rem, 0.5rem, 1rem, 1.5rem, 3rem
test/index.scss:42 Debug: Updated: 0, 0.5rem, 1rem, 2rem, 3rem, 6rem The main issue seems to be updating maps that get re-used. |
The point is if you do
You end up with a similar problem of |
just checking, adding in a "primary" into the custom-theme map would then change the primary';s colour right? this isnt just for "extending" it?
this would mean this changes right?
would have to bring in the separate bootstrap files into our main style.scss then? |
If you just want to change an existing key's value in @import "../scss/functions";
$primary: purple;
@import "../scss/variables";
@import "../scss/maps";
// etc
// Test for it
@debug "Test: Primary should be purple instead of #0d6efd (blue).";
@debug "Variable is " + $primary;
@debug "Theme colors map value is " + map.get($theme-colors, "primary"); test/index.scss:29 Debug: Test: Primary should be purple instead of #0d6efd (blue).
test/index.scss:30 Debug: Variable is purple
test/index.scss:31 Debug: Theme colors map value is purple Looking at writing some tests for the spacers and more to see what we'd need to do. |
@tomsommer Yes, the same does apply there. I misunderstood your earlier point I think and was only looking to see if spacers are updated if you change the |
The only way to solve this everywhere is to never modify variables after including See #34756 (comment) :) |
The maps.scss file doesn't exist yet. The discussions here are around how to improve the situation in our next release. Look up higher to the comments that suggest customizing the map and re-declaring the additional maps. |
I'm not sure if this helps but I'm posting it here incase someone else tries the same thing. $theme-colors: map-merge($theme-colors, $custom-theme-colors);
$theme-colors-rgb: map-loop($theme-colors, to-rgb, "$value");
$utilities-colors: map-merge($utilities-colors, $theme-colors-rgb);
$utilities-text-colors: map-loop($utilities-colors, rgba-css-var, "$key", "text");
$utilities-bg-colors: map-loop($utilities-colors, rgba-css-var, "$key", "bg"); and that breaks for example |
so is @morkeleb 's post this the "fix" till 5.2? some how (and im not sure if its just me doing something silly) setting
results in the checkboxes being the default blue. (again this could just be me being stupid) have to add
to my variables file. |
@WilliamStam You should declare |
Not entirely sure if this is relevant here specifically, but I hope it is. While the solution from @isaumya works, it only works if the values in your custom color map are rgb or hex only, with no alpha value. If you have something like...
And you try to merge that into $theme-colors via the quoted method, any key in the color map with a value in rgba won't pass all the way through to becoming a bg- or text- class. (in this example, 'bg-primary25' just... didn't exist in the resulting css) So if anyone is using rgba values in their theme-colors map, they just... won't work. Even with this fix. You have to scrub those all out, replace them with the base rgb or hex value, and then use the 'opacity' classes in your html to achieve the same result. (And I guess maybe add custom opacity values via that utility, if needed?) |
I don't know if what I did was stupid... first imported everything with: @import "node_modules/bootstrap/scss/bootstrap"; then I make my changes, customizations, example: $aliceblue:#F0F8FF;
$antiquewhite:#FAEBD7;
$aqua:#00FFFF;
$aquamarine:#7FFFD4;
$azure:#F0FFFF;
$beige:#F5F5DC;
$bisque:#FFE4C4;
$blanchedalmond:#FFEBCD;
$blueviolet:#8A2BE2;
$brown:#A52A2A;
$custom-colors: (
"aliceblue": $aliceblue,
"antiquewhite": $antiquewhite,
"aqua": $aqua,
"aquamarine": $aquamarine,
"azure": $azure,
"beige": $beige,
"bisque": $bisque,
"blanchedalmond": $blanchedalmond,
"blueviolet": $blueviolet,
"brown": $brown
);
@each $color, $value in $custom-colors {
.bg-#{$color} {
background-color: $value !important;
}
.text-#{$color} {
color: $value !important;
}
}
$theme-colors: map-merge($theme-colors, $custom-colors); once I have already merged; I return import to get the css output. @import "node_modules/bootstrap/scss/bootstrap"; I do this twice already because for some unknown reason it wouldn't let me get the customized bg and text. |
In one of my projects I include the needed Bootstrap SCSS files manually and specifically exclude
bootstrap/scss/root
, first because I don't rely on most of the CSS variables and secondly because adding all CSS variables increases the CSS file size by about 50-60kb (uncompressed).With v5.1.0 all
.bg-*
and.text-*
utilities break, as the needed CSS variables are missing, e.g.Is there a simple way to disable CSS variables generation or to reduce the number of CSS variables? The only way forward right now would be to revert the utility sets back to
$theme-colors
, but that still leaves an additional--bs-text-opacity: 1;
and does not take care of--bs-body-color
and the like:The text was updated successfully, but these errors were encountered: