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

Optional CSS Variables and multi-theme ability #2981

Closed
wants to merge 40 commits into from

Conversation

Tofandel
Copy link

@Tofandel Tofandel commented Jun 5, 2020

Over the years, there have been tons of demands and tries to get css variables into bulma
#1775
#2749
#1837
#3028

This PR does this successfully with a lot of ingeniosity by maximizing backward compatibility, the framework can be compiled exactly like before with no css variables, it can be compiled with all the css variables for outside theming or it can be compiled with a set of themes for bundled theming

This is a new feature.

Proposed solution

Wrapping every variable in a few custom functions and to register them all beforehand
A theme mixin to output the used variables and the modified ones on successive call

I also revisited the build process from a simple cli command to a more complete nodejs script

Tradeoffs

The only tradeoff of this solution is that because of the wrapping of each variable, the git diff is large and will cause a lot of merge conflicts and will require future PR to follow the new syntax (though it's not a hard syntax, just slightly repetitive)
There will also be a minor BC break because some variables have been modified (Only the map $colors that was renamed to $colors-list to keep BC and print a deprecation warning and the map $shades that are now only a list of names)
The other variables will stay compatible as long as you build without theming enabled (by default)

Todos

  • Check that the HSL transformation corresponds to the output of lighten, saturate and adjust-hue
  • Update doc
  • Add a theme switcher in the doc
  • Update changelog
  • Cleanup
  • Remove files according to guidelines and squash commits

@jgthms
Copy link
Owner

jgthms commented Jun 6, 2020

Interesting, thanks.

It seems quite overkill at first sight. I do like the idea of having a separate themable css file but I think most of the JS functions could have been done in Sass/Ruby.

For example the v() function doesn’t need 2 parameters as you could simply infer the name from the variable name.

Also there’s duplication in the css vars: if the hsl values are available seperately, we could use them instead of having a literal value as well. Because then we’d only need to change a color in a single location.

I also think the final output is missing the point of having the css vars, as the initial variable names are completely gone. For example “—bulma-success” should have a value of “var(—bulma-green)”. Same for “ dropdown-item-hover-color” for example. It has a literal value in the output, but should actually reference “var(—text-strong)” which itself should reference “var(—grey-darker)” which has a value of “#0a0a0a”.

Because right now if you want to update the grey-darker value you need to rebuild the whole CSS file with the compiler, so it’s missing the point of having CSS variables in the first place.

I have a branch with an initial CSS vars implementation but I might take inspiration of a few things here.

@Tofandel
Copy link
Author

Tofandel commented Jun 6, 2020

I tried infering the variable name from the v function but it's not possible with sass, have a look also at the color loops and you will understand, sass is very poor in term of functionalities as you cannot dynamically reference a variable by it's name, also I am a nodejs engineer, not a ruby developer so I just went for what I knew I could do ;)
There is not a lot of functions and mostly no computing in them only outputting, but if you think some can be done differently feel free to point them out

For the duplication I was just thinking about only using hsl if available, but that would mean that if you had a variable not hsl before and in a later version use an adjustHSL function on it, you will lose the original value and create a backward compatibility issue

Referencing the variables in the declaration would also be possible

I'll make nesting v() functions possible so it will do the trick and reference correctly the other variable

The other use case of this, is that you can compile themes with the provided command, it's not documented yet

But basically it will only generate the modified css variables

I had a look at all the css branches and available things out there, nothing was very promising and I need to move things up as I have a trading app project using this lib and we need a css theming ability

@Tofandel
Copy link
Author

Tofandel commented Jun 7, 2020

@jgthms I've given some thoughts over your points during the weekend and found a way to solve them all

For the v function I can make it sass only and one param only just as long as I make a register function and call it on every variable before use

Basically the solution can be pure sass

The only missing point is the one I talked about regarding BC breaks with HSL and non HSL values

One option could be to make every color HSL only but that may increase the size of the css quite a bit

I'll implement everything tomorrow, I think you'll be pleased

It'll be ready to ship very soon

@Tofandel
Copy link
Author

Tofandel commented Jun 10, 2020

I've implemented the new solution, it's quite more of a breaking change since two variables have now been modified

@jgthms I've solved the BC break with HSL and non HSL values by registering everything in HSL and adding another calculated variable using the HSL values

As an optimization only the used variables are output

It did increase the size quite a bit (39Kb gzipped for themeable vs 25Kb gzipped for non themeable)

I now need to do some cleanup but it will give you a rough idea of the process

Let me know your thoughts

@@ -10032,12 +10005,12 @@ label.panel-block:hover {
}

.hero.is-white.is-bold {
background-image: linear-gradient(141deg, #e6e6e6 0%, white 71%, white 100%);
background-image: linear-gradient(141deg, #e8e3e4 0%, white 71%, white 100%);
Copy link
Author

@Tofandel Tofandel Jun 12, 2020

Choose a reason for hiding this comment

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

This is due to the adjust hue being executed at the same time than the adjust lightness, while before the adjust lightness was executed after the adjust hue (which does nothing on white, but strangely enough does on black)

It is a side effect of the new vAdjust function rather than the combination of adjust-hue, saturate, Iighten
I consider this a bug fix and the difference is invisible to the eye

Tofandel added 2 commits June 12, 2020 18:24
# Conflicts:
#	css/bulma.css
#	css/bulma.css.map
#	css/bulma.min.css
#	docs/css/bulma-docs.css
#	docs/css/bulma-docs.min.css
#	package.json
#	sass/base/helpers.sass
#	sass/components/card.sass
#	sass/components/dropdown.sass
#	sass/components/level.sass
#	sass/components/list.sass
#	sass/components/media.sass
#	sass/components/menu.sass
#	sass/components/modal.sass
#	sass/components/panel.sass
#	sass/components/tabs.sass
#	sass/elements/button.sass
#	sass/elements/content.sass
#	sass/elements/notification.sass
#	sass/elements/tag.sass
#	sass/form/select.sass
#	sass/utilities/_all.sass
#	sass/utilities/derived-variables.sass
#	sass/utilities/functions.sass
#	sass/utilities/initial-variables.sass
@Tofandel
Copy link
Author

All features to ship are now tested and ready

@jgthms
Copy link
Owner

jgthms commented Jun 14, 2020

Thanks. It’s a complex PR. I’ll try to have a more in-depth look.

I like the idea of having a separate themable Bulma CSS file. I’m not super keen of introducing a JS build process but it seems like the only solution for a few of the problems.

The major issue I see right now is how most of the initial and derived variables become deprecated, even for non-themable setups. That’s not something I can introduce as it will break far too many setups. These variables are part of the core of Bulma and deprecating them is not possible.

I also think it’s not necessary to transform all Bulma variables into CSS variables. For theming purposes and dark mode support, colors would largely suffice.

I think the way to go will be a toned-down version of the ideas you’ve introduced.

@Tofandel
Copy link
Author

Tofandel commented Jun 14, 2020

@jgthms I actually solved the issue of the js build, it's completely optional and I only kept it because it's easier to use in the build scripts and I added useful flags to it.

To make it work outside of the script I made a postcss plugin that takes care of optimizing the variables, after generation, so it works with webpack, sass-cli and pretty much anything out there as well, the js build just comes with it preconfigured, it's documented, you can have look in the build with webpack build with sass cli sections

For non themeable setups, variables still work and can still be used after bulma completely like before, it's highly backward compatible, I marked it as deprecated only for new build and theme makers/publishers
If people don't want the theming ability then it's 100% safe to use variables but I'm recommending the register version as it means if later on, you decide you need the theming ability you don't have to rewrite your code or If you want to publish the theme you made for yourself it becomes super easy to do so

The advantage of this is really it's flexibility that variables do not offer just because sass does not support dynamic variable reference (and that's really an important feature)
If you have a look at the generated css for the docs you will see that only the variables that were modified in the other theme get converted to css variables, which is really all that is about, being able to bundle a bunch of themes into one css as light as possible

A toned down version of it would not be such a good idea as it's an all or nothing kind of library and It would really be super confusing if users start needing to work with variables as well as registered variables or frustrating if you were not able to modify variables for specific elements only in your theme, I know for a fact a lot of themes modify sizes, border radius and font family (you can just check in bulmaswatch, everything is used), so no, colors only is not an option

Customizing what gets toned down is however possible, it's not documented but there is some inner variables, functions that if declared correctly could output only initial and derived variables names instead of the ones more specific to components

@Tofandel
Copy link
Author

Tofandel commented Jun 15, 2020

Also I didn't mention it but I don't plan on having the full themeable file included in the css dir, it's there as a demonstration and will be removed from the default build script

The way to use this feature would really be the one mentioned in the Themes section of the doc I added and this file's point was to show all the variables you can change and you could get and to test that everything was working properly

It could be used for js customization though, but since probably only less than a dozen people will use it, they can build it themselves with the full flag or without using the postcss plugin to optimize the variables

@Tofandel Tofandel mentioned this pull request Jun 22, 2020
@wtho
Copy link

wtho commented Jul 1, 2020

Should the PostCSS-Plugin postcss-var-optimize be included in this repository? I think this would be best if it's use beyond bulma is not expected. It could still be published as own npm package.

I think a fully-themeable, minified version of bulma is also highly interesting to some use cases (especially for users which are not comfortable using node/sass). In my opionion it should be deployed via CDNs and included in the published npm package.


Some features I would like to see (does not have to be at the first throw):

  • optional addition of fallback variables for IE11 support:
    .element {
      color: #ad4359;
      color: var(--primary);
    }
    
  • optional addition of transition elements:
    .element {
      color: var(--primary);
      background-color: var(--primary-invert);
      transition: color 500ms ease-in-out, background-color 500ms ease-in-out;
    }
    

@Tofandel
Copy link
Author

Tofandel commented Jul 5, 2020

@wtho
"Optional addition of fallback variables": IE11 is not to be supported by bulma but you can already do that with a post css plugin: postcss-custom-properties

"Optional addition of transition elements": If this PR is ever review and accepted (:roll_eyes:) you could make a PR or your own post css plugin for that

I'm not too much for the idea of having the full file deployed by default because it's not optimized and people might over use it when they could make an optimized version instead, on the other end the only use case is dynamic theming by the user where you'd need to make everything customizable via js, but as I said there wouldn't be enough use of this to have it deployed by default, that's why I'd recommend people to build it themselves because this version assumes you know what you're doing.

@Tofandel Tofandel changed the title Optional CSS Variables and themeing ability Optional CSS Variables and multi-theme ability Aug 1, 2020
This was referenced Aug 13, 2020
@LoopsGod
Copy link
Contributor

LoopsGod commented Aug 15, 2020

It's insane that this is not being merged / worked on. I appreciate your work @Tofandel

Thanks for putting so much time in this PR, even without any support.

@chrishumphrey1985
Copy link

This is definitely a feature that Bulma needs. What's holding it up?

@adriansalvatori
Copy link

How can I use this PR instead of main branch? I really need this feature.

@stellarbear
Copy link

It's 2021. Bulma still do not support css variables 🤔

@acallaghan
Copy link

Would this just be better as a fork of Bulma, instead of mainline? Given that this changes the direction of the project quite a bit & hasn't had support from the creator?

@chrishumphrey1985
Copy link

I'm not even sure if Bulma is being actively developed anymore.

@Tofandel
Copy link
Author

Days of work have been put into this PR which didn't get support from the creator

I stopped using bulma so will not be resolving the conflicts on this PR and in the end this work will just be lost and serve no purpose, so if anybody actively using bulma wants to take it over, feel free to fork my PR, as it was finished and now only needs conflict resolution

@Tofandel Tofandel closed this Feb 18, 2022
@daniil4udo
Copy link

@Tofandel you've done a tremendous job! 👏

Could you please take a look at bulvar repo, dedicated to this issue. I would love to hear from you and hear your suggestions for the further improvement.

bulvar is complete rewrite of Bulma & Buefy that using CSS variables and replaced @import with @use & @forward

@Tofandel
Copy link
Author

Tofandel commented Mar 7, 2022

@daniil4udo Thanks, it seems you as well. As I explained I'm not using bulma in any of my projects anymore so I won't go into a thorough review, though I have one feedback, the fact that you're using a mix of css variables and scss vars makes it very hard to debug, read and maintain, my solution for this was a custom library to handle all this with scss vars and mixins https://github.com/Tofandel/bulma/blob/fbfe63286ddafe70d3b51ef091c578bf327f5dda/sass/utilities/functions.sass#L274 (everything after this line)

You need to register each variable with +register('var-name', value)

And use it with v('var-name'), each util like darken had its own counterpart (vDarken('var-name', value)) to create a new derived variable when used

In the end the theme mixin just outputs all the used variables at the root, this allowed for tree shaking and then for a postcss plugin I made to interpret all variables in the css so if you had only one theme, you would not have any variable because they would all be converted back. But if you had multiple themes only the variables that change in the theme would be kept, making it super optimized and lightweight, I had the full documentation written out in the PR

It might be a bit hard to change that now that it's already all written, and I'm not going to force you or anything, but my recommendation would really be to use this custom lib I made because it's just magical engineering and if you get to use it you'll have the best feeling when everything just works out of the box🎊.

I even planned to extract it as a separate project because it can be used to convert any scss lib to a variable css lib super easily (really after this functions were developed I just did a replace all with regex in the project)

And I definitely still will when I get some time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants