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

Replace division with multiplication #2669

Conversation

senchden
Copy link
Contributor

Addresses #2668. Removed slash character to be compatible with the newer versions of sass compiler. Tested and does not brake compatibility since resulting CSS is not changed.

Remove slash character to be compatible with newer versions of sass compiler
@NicolasCARPi
Copy link
Collaborator

Thanks, indeed this should not cause any issues. But the sass doc recommends using math.div. I think it is better to use math.div as it might handle better different values of $amnt (compared to the * operator). But this is pure supposition, maybe the check if > 1 above is enough to make sure no weird values are there. Anyway, I would still side with the documented change.

@senchden
Copy link
Contributor Author

senchden commented Oct 13, 2021

Yes, it could be replaced with math.div. But I have done it this way if to keep backwards compatibility with libsass/node-sass (just like in bootstrap). In fact, multiplication is what official sass migrator came up with. If you don't need it, I can replace it with math.div

@NicolasCARPi
Copy link
Collaborator

For me you provided a valid reason to use the multiplication so let's use that then! Thanks!

@NicolasCARPi NicolasCARPi merged commit 2d16ffa into snapappointments:v1.14-dev Oct 13, 2021
@NicolasCARPi
Copy link
Collaborator

dammit, I fat fingered the commit message.... oh well....

@senchden senchden deleted the remove-sass-deprecated-division branch October 14, 2021 00:06
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.

2 participants