them-es / starter-bootstrap

Bootstrap 5 WordPress Starter Theme.
https://them.es/starter-bootstrap
GNU General Public License v2.0
149 stars 53 forks source link

Sass math div #6

Closed davidhbrown closed 3 years ago

davidhbrown commented 3 years ago

Provided to resolve #5

them-es commented 3 years ago

Thanks for your help and for providing this PR, @davidhbrown

This is definitely a good approach but Bootstrap's solution to resolve this issue seems more elegant as no @use "sass:math"; is needed. And Libsass will still work. Furthermore it's usually better that projects based on frameworks align with the original codebase (i.e. Bootstrap). What do you think?

A fix for these warnings (*.5 instead of /2)will be included in the next release - so no further action is required on your side.

them-es commented 3 years ago

This has been fixed in https://github.com/them-es/themes-starter-bootstrap/commit/cc74a3722d7fbe349e2ac657d49cf97eac120e17#diff-3a56bfc93d029f425c8fbd43b763c034935c43cea022202ea319c19555a7eb8a

davidhbrown commented 3 years ago

That makes great sense... just because your gulp is now using Dart Sass doesn't mean everyone will. In fact, I'd done that sort of * 0.___ in some of my own scss (when the divisor worked out as a non-repeating decimal which isn't always) while waiting for JetBrains to update their Sass library to include math.div.