fix: deprecation warning about using / for division #1452
Conversation
🦋 Changeset detectedLatest commit: 10a8573 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
@mktcode @plegner Thanks for bringing this to our attention. 🙇 Yep, seems using / for division will be removed at some point.
As for this PR, I think we use LibSass for github.com, so using math.div(x, y) won't work. Not sure if there is a plan to add it to LibSass and Ruby Sass in the future?
Maybe an alternative to using division is using multiply with a fraction as decimal. Like
/ 2->* 0.5/ 3->* 0.3333333333/ 4->* 0.25- etc.
Well, at least for now until the other libraries add support for division.
/cc @jonrohan @vdepizzol
I like that idea. But what about variables? Lines 39 to 41 in 0b3584c I also just found that both Ruby Sass and LibSass reached their end of life: Oh and... Postcss also doesn't seem to like it. Should I have run the tests locally? Absolutely. Did I? Just now, yes... :D |
I think
Ooh.. 😱 yeah, we probably should look into replacing LibSass sooner or later. |
It's still early here and I just got up.... don't know why I referenced that line. o_O This is what I mean: css/src/marketing/support/mixins.scss Line 79 in d2431fb |
src/layout/layout.scss
Outdated
| @@ -37,7 +37,7 @@ | |||
| } | |||
|
|
|||
| .Layout-main { | |||
| grid-column: 2 / span 2; | |||
| grid-column: math.div(2, span) 2; | |||
There was a problem hiding this comment.
As @simurai pointed out, this is not a division, but a CSS grid syntax. Same with a couple of other lines below :)
There was a problem hiding this comment.
Brutally force pushed now... things never happened. :D But I still don't know how to fix the $variable situation.
There was a problem hiding this comment.
Ahhh gosh... today's really not my day. Don't say anything... saw it myself.
... ok now.
src/support/variables/layout.scss
Outdated
| @@ -33,7 +33,7 @@ $spacer: 8px !default; | |||
|
|
|||
| // Our spacing scale | |||
| $spacer-0: 0 !default; // 0 | |||
| $spacer-1: round($spacer / 2) !default; // 4px | |||
| $spacer-1: round(math.div($spacer, 2)) !default; // 4px | |||
There was a problem hiding this comment.
| $spacer-1: round(math.div($spacer, 2)) !default; // 4px | |
| $spacer-1: $spacer * 0.5 !default; // 4px |
using / for division is deprecated and the math module (math.div(x,y)) not guaranteed to be available
|
Well, won't this still be an issue? css/src/marketing/support/mixins.scss Line 79 in d2431fb Since the variable makes it impossible to handle it with multiplication, I assume this just has to wait? |
Replaced all occurrences with
math.div(x, y).Searches I made:
\$[\w-]+\s?\/\s?\$[\w-]+(e.g.$var/ $var2)[\d]+\s?\/\s?[\d]+(e.g.1 / 12)\$[\w-]+\s?\/\s?[\d]+(e.g.$var2 / 123)[\d]+\s?\/\s?\$[\w-]+(e.g.123/ $var2)/cc @primer/ds-core