ForEvolve / bootstrap-dark

Bootstrap 4 dark theme that supports togging dark/light themes as well. There is no fluff, it changes the color of Bootstrap and that's it, no new thing to learn or unlearn, just Bootstrap, but Dark!
MIT License
299 stars 44 forks source link

fix: variables should be overridable (fixes #63) #64

Closed titanism closed 1 year ago

titanism commented 1 year ago

@Carl-Hugo can you trigger the tests to run again? not sure why they failed

titanism commented 1 year ago
Screen Shot 2023-02-14 at 10 29 52 AM
Carl-Hugo commented 1 year ago
Screen Shot 2023-02-14 at 10 29 52 AM

I'm not sure how to fix the build of forks; GitHub Actions don't want to create a release of the artifacts on GitHub, and I found no way to deploy a version of the fork on NPM either since it does not inject the NPM key into the build (security reasons).

I have a few workaround in mind; worst case I'll deploy the package manually so you can try it out.

titanism commented 1 year ago

You could just merge this PR and then pull locally to test - then bump and manually release. Looks like CI workflow config issue or something.

Carl-Hugo commented 1 year ago

You could just merge this PR and then pull locally to test - then bump and manually release.

If I do that will trigger the release of 4.0.0 to NPM.

I pulled the changes and deployed the pre-release version manually to NPM. Please try out 4.0.0-g2dfbff68ae and let me know how it goes.

Looks like CI workflow config issue or something.

Yes, since the code is under your account, GitHub actions do not inject the secrets during the pipeline run.

titanism commented 1 year ago

Just tested and it is extremely breaking, for example, the body background color is #fff. I think some of the variables need not have !default.

titanism commented 1 year ago

Trying this out manually:

+$body-bg:     #191d21;
-$body-bg:     #191d21 !default;
+$body-color:  #d3d3d3; //$gray-200 !default; //#D4D4D4 !default;
-$body-color:  #d3d3d3 !default; //$gray-200 !default; //#D4D4D4 !default;
titanism commented 1 year ago

Actually, this commit needs redone - the only vars we should set !default on are probably colors like $blue and $green. I can try to revert.

titanism commented 1 year ago

The only other thing I noticed was $orange being used for warning instead of $yellow as in bootstrap defaults. The other variable changes looked fine to me, e.g. different grays for better contrast 👍