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 depreciated division operator with math.div function #3336

Closed
wants to merge 6 commits into from

Conversation

tcitworld
Copy link

This is a bugfix.

It requires #3335 and will fix #3333, by replacing uses of the depreciated division operator.

Proposed solution

Replace uses of the division operator with the math.div function.

Tradeoffs

Main drawback is that Bulma now requires sass >= 1.33 to be build and used. Earlier versions of sass won't be able to build and node-sass won't be either. Since this is a very hard change, I'd recommend waiting as long as possible before merging and releasing this¹.

Testing Done

Build the documentation and quick check for UI issues

Changelog updated?

Will do after #3335 is merged.

¹ The depreciated message from sass is quite annoying, however.

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
Signed-off-by: Thomas Citharel <tcit@tcit.fr>
Signed-off-by: Thomas Citharel <tcit@tcit.fr>
Signed-off-by: Thomas Citharel <tcit@tcit.fr>
Signed-off-by: Thomas Citharel <tcit@tcit.fr>
Signed-off-by: Thomas Citharel <tcit@tcit.fr>
@tcitworld tcitworld changed the title Replace node-sass with sass and use math.div Replace depreciated division operator with math.div function May 21, 2021
@service-paradis
Copy link
Contributor

Maybe some of those could use multiplication instead of a division. It would be compatible with older libsass while removing a lot of deprecation warning for dart. Ex.

margin-right: $block-spacing / 2

Would become:

margin-right: $block-spacing * 0.5

Instead of:

margin-right: math.div($block-spacing, 2)

@EtienneBruines
Copy link

EtienneBruines commented May 27, 2021

@service-paradis but if it only applies to "some", then measures still need to be taken for those cases where division cannot be replaced by multiplication.

(And it therefore would not make too much sense, replacing division by multiplication some of the cases.)

@service-paradis
Copy link
Contributor

service-paradis commented May 27, 2021

@service-paradis but if it only applies to "some", then measures still need to be taken for those cases where division cannot be replaced by multiplication.

(And it therefore would not make too much sense, replacing division by multiplication some of the cases.)

Sure, but it would (at least) reduce some of the warning waiting for a solution.

Another way to reduce the number of warnings would be to create a custom function like:

@function bulmaDivide($number, $div)  
  @return $number / $div

A single warning would be issued and it would be easier to replace with math.div when @jgthms wants to offer a Dart version.

@ghiscoding
Copy link

Sure, but it would (at least) reduce some of the warning waiting for a solution.

in latest version 1.34.0 of dart-sass, they added a flag --quiet-deps to remove warnings from other lib/project/dependencies, I just tried it and it works fine. After running a build, I see warnings from my lib but without seeing all the ones from Bulma (while previous version 1.33.0 was displaying a lot of warnings from every dependencies it was a nightmare to parse through but now it's fixed, they had lot of people complaining and they addressed it quickly). See latest dart-sass v1.34.0

@service-paradis
Copy link
Contributor

in latest version 1.34.0 of dart-sass, they added a flag --quiet-deps to remove warnings from other lib/project/dependencies, I just tried it and it works fine. After running a build, I see warnings from my lib but without seeing all the ones from Bulma (while previous version 1.33.0 was displaying a lot of warnings from every dependencies it was a nightmare to parse through but now it's fixed, they had lot of people complaining and they addressed it quickly). See latest dart-sass v1.34.0

The warning are annoying and I'm glad dart-sass provides a way to quiet those messages, but this is not a good workaround. I dont like to see those warning, but I dont want to silence them. They are a reminder that I wont be able to update to the next major version of dart-sass if Bulma does not offer a way to be compatible with the updated syntax.

@service-paradis
Copy link
Contributor

Bootstrap have a PR around this problem that might be a goop option for Bulma as well:
twbs/bootstrap#34245

@tcitworld
Copy link
Author

Closed in favor of #3362

@tcitworld tcitworld closed this Jun 18, 2021
@tcitworld tcitworld deleted the sass-divide branch June 18, 2021 20:52
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.

[Feature] SASS division without slash
4 participants