search

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
298 stars 44 forks source link

Variables should be affixed with `!default` in case custom overrides. #63

Open titanism opened 1 year ago

titanism commented 1 year ago

e.g. if a user wants to use a custom $blue value in dark mode

https://github.com/ForEvolve/bootstrap-dark/blob/master/scss/_dark-variables.scss

Carl-Hugo commented 1 year ago

I can't remember why I did not add the !default suffix; maybe it does not work as one would expect because Bootstrap already defines that suffix, and the dark variables are already overriding them... To be validated.

That said, feel free to validate if it works, and if so, let me know and/or open a PR.

TBH, I'm pretty busy with work atm, and I don't think I'll have the opportunity to review this for a while; your help would be most appreciated.

titanism commented 1 year ago

Couldn't we just move @import 'dark-variables' to before @import 'node_modules/bootstrap/scss/variables'; and affix all the vars with !default?

titanism commented 1 year ago

Will open a PR and test it out now if it works

titanism commented 1 year ago

Tested and seems to work OK. See https://github.com/ForEvolve/bootstrap-dark/pull/64.

Can you merge and release a new major or minor version to npm? This is breaking so at the minimum do a minor version release, if not a major for safety.

titanism commented 1 year ago

Just an update - that PR is broken and should not be merged. We'll try to find time to dive in that again in the near future.