Remove deprecated slash division in favor of math.div

[MoritzLost]

The em function in the _functions.scss file uses the deprecated slash syntax for division:

@function em($pixels, $base-font-size: 18) {
  @return #{$pixels/$base-font-size}em;
}

See Breaking Change: Slash as Division for details. Importing the tobii.scss causes a warning in Dart Sass and will stop working at all in Dart Sass 2.0. For now I'm just importing the pre-compiled tobi.min.css instead – though it would be nice to able to import the source files instead, this way our build pipeline rules (including autoprefixer rules) will apply to it as well.

Here's an updated version of that function:

@use "sass:math";

@function em($pixels, $base-font-size: 18) {
  @return #{math.div($pixels, $base-font-size)}em;
}

Though this will break all older sass compilers (node-sass in particular). If you want to support those, there are other options. Bootstrap replaced the slash division with a custom divide function that will work for all compilers.

A broader picture

Another option would be to drop that function altogether and just use em normally. Right now the function doesn't respect the actual --base-font-size anyway, so I'm not sure why it's there or what it's doing. Using native em units, relative units like vw/vh or just sticking with pixels would be preferable in my opinion.

[viliusle]

Actually I would go even further, why we need Sass at all, it is 2021, CSS is powerful alone. And for me CSS > SASS for small projects.

[midzer]

I think it would be a major change to have CSS alone. We should leave SASS for now, maybe reconsider for a tobii v3.

So we have several options for a fix of this issue:

• Bootstrap compiler
• Dart Sass 2.0 -> would be breaking change for tobii imo
• replace the function

To be honest, I can't decide right now which way to go. If you have a clever opinion on this, go ahead :)

[MoritzLost]

@midzer Drop the function. It's kind of superfluous, there are so many good ways to get relative font sizes. And looks like the function is doing the exact opposite. It takes a pixel value and the base font size just to output an em unit that will result in the same pixel size regardless of base font size. So what's even the point of using em? If you want a hardcoded size in pixel why not just use px?

If you want to support relative font sizes, two good options:

• Set the base font size on the root element of the lightbox and just use regular em units.
• Use calc() with a custom property for the base font size. This would allow users of the lightbox to specify the base font size the same way they can with SCSS variables right now.

[viliusle]

I agree, that this function should be removed and we should use em for fonts and rem for buttons width and height.

So:

• if user has default font size as "tiny" - our GUI should have small elements.
• if user has default font size as "medium" - our GUI should have medium elements. (default for most users)
• if user has default font size as "large" - our GUI should have large elements.

p.s. I believe we should remove "Sass" as soon as possible, because it gives no benefits, but gives troubles during development.

[midzer]

I would be happy about any PR to get progress going on in this issue.

[11bits]

I see this warning still appears. I think the most straightforward solution is adding calc():

@function em($pixels, $base-font-size: 18) {
  @return #{calc($pixels / $base-font-size)}em;
}

Not sure if it works in all sass compilers.

[11bits]

I sent a PR if you want to check it.

[viliusle]

Are we 100% sure we want to do it in wrong way? This function should be removed.

p.s. Thank you for PR, but I am trying to look at broader picture.

[viliusle]

I can push commit, which removes this function, if everybody agrees.

[11bits]

I didn't know it was a bug, but if my PR has been helpful in solving an old issue, that's ok.

[felixranesberger]

In my opinion you shouldn't use calculation functions like these for em values. If switching from pixel to em values in the source code is not possible right now, the pull request should use sass math compiler's like @MoritzLost suggested, since the values basically never change and calculating it inside CSS provides no benefits, because the font-size can already be changed by adjusting the parent containers font-size.

[midzer]

Finally some progress with https://github.com/midzer/tobii/commit/eb2d1de806699104333596e6d040de7d32a086e4.

Too bad I did not manage to recover the original 1:1 computed px values from the demo. There is probably some tweaking necessary.

[viliusle]

@midzer if you would add prefix "#89 " to commit message, github.com automatically attach log to related issue. Very helpful for everybody.

[MoritzLost]

Finally some progress with https://github.com/midzer/tobii/commit/eb2d1de806699104333596e6d040de7d32a086e4.

@midzer In my opinion, trying to force specific pixel dimensions with values like 0.27778em or 2.22222rem is not a good idea. If I understand correctly, those values are set to generate fixed pixel values based on the previous base size of 18px. However, you've changed the --tobii-base-font-size to 1rem, which will be 16px on most devices. Don't try to force specific pixel weights, and don't assume that everyone will use the default base font size. The purpose of the relative units and the --tobii-base-font-size variable should be the ability to scale the entire UI with a single variable (or browser setting). Use simple fractions (like 1.25em, 0.25 etc) to create a nice, even, relative scales. Then I can use something like --tobii-base-font-size: 1.25rem to scale up the entire UI if I want to, and the UI scales nicely by default with the font size defined in the user's browser. It doesn't matter if the relative units don't create round pixel values, most devices have a DPR > 1 and support sub-pixel rendering, anyway.

[midzer]

@MoritzLost Thanks for your explanation!

Wanna open a PR with those em tweaks and we can have a review together?

[MoritzLost]

@midzer Sure: https://github.com/midzer/tobii/pull/105

I've converted all the values to approximately match the previous values (based on the base-size of 1rem). If the base size isn't modified, all of those values now result in clean pixel values. If it is, they will scale nicely along the base size.

Is it significant that ~50px appear in both --tobii-slide-max-height and in top: 2.77778em; of tobii__slider? In this case, that should probably be another property so it can be changed with a single custom property.

[midzer]

Done by https://github.com/midzer/tobii/commit/eb2d1de806699104333596e6d040de7d32a086e4