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

How to handle mixed unit math #3047

Open
seven-phases-max opened this issue Apr 9, 2017 · 6 comments
Open

How to handle mixed unit math #3047

seven-phases-max opened this issue Apr 9, 2017 · 6 comments

Comments

@seven-phases-max
Copy link
Member

seven-phases-max commented Apr 9, 2017

div {
    @value: 1px;
    a: @value * 1rem;       // 1px  - ok
    b: @value * 1px * 1rem; // 1px  - ok
    d: @value / 1px;        // 1px  - ok
    c: @value / 1px * 1rem; // 1rem - unexpected
    e: @value / 1px + 0rem; // 1rem - unexpected
}

c and d results above contradict with http://lesscss.org/features/#features-overview-feature-operations stating the result should have a leftmost unit of an expression (i.e. all expressions above should produce 1px). Tests cover only a / b / c expression which accidentally produces the expected result.
Related to #2418 and #2472.

@matthew-dean
Copy link
Member

matthew-dean commented Jun 27, 2018

Honestly, I've never been a fan of being able to mix units in Less. It makes no sense from a mathematical perspective, and as you point out, it's all based on the evaluation order.... which also makes no sense.

However, when I looked at the strictUnits code, that also made no sense to me. That also doesn't seem to do what I thought it would do.

So, yeah, that's weird, but I'm not inclined to see it fixed. I'm more inclined to see mixed-unit math go away. But that's just me. Like the result of 1px * 1px should be 1px^2 (not useful, but true). And the result of 1px / 1px should be 1. Sass is overly-complex about some kinds of math, but it uses basic math principles to do things like switch units of mix units. For example.

@value / 1px * 1rem

The math here is 10px / 1px. This should produce 1 (not in Less, just in principle, and would in Sass). 1 * 1rem = 1rem. No offense to Alexis in the origins of this feature, but picking the first unit of an expression is just kinda weird, hard to evaluate, and can lead to these buggy edge cases.

Just my $0.02.

@matthew-dean matthew-dean changed the title Div in complex arithm expressions results in a wrong unit value. How to handle mixed unit math Jul 18, 2018
@matthew-dean
Copy link
Member

matthew-dean commented Jul 18, 2018

I think what should happen is that, like the changes to math, there's a change in mixed units so that there's a third option between the two:

  1. strictUnits: false - attempt to do math and coerce units
  2. strictUnits: ? - e.g. 1px / 1px // output 1, 100vh - 10px // output 100vh - 10px i.e. with mixed units, leave the expression as-is and output - useful for sub-expressions assigned to vars and used in calc() - future default?
  3. strictUnits: true - throw an error (I don't see how this is useful.)

@seven-phases-max
Copy link
Member Author

The issue is not about personal opinions of units/arithm handling. It's about compiler not following its own documentation. (For the rest see for example #1366 (comment)).

@matthew-dean
Copy link
Member

The issue is not about personal opinions of units/arithm handling. It's about compiler not following its own documentation

Yeah, I get that. But ultimately it's about how to "fix" it, right? So I'm just saying in a roundabout way that I don't think this is worth the time to fix, and instead it should have a more intuitive behavior. But yeah, that's just, like, my opinion, man.

@matthew-dean matthew-dean added this to the 4.0 milestone Oct 11, 2019
@matthew-dean
Copy link
Member

@seven-phases-max Just a note: I've re-written most of the operate methods for units, and they indeed had some weird acrobatics for determining the final unit. I've gotten it to be much more straightforward where the left-hand-side unit wins when coercing units is the option passed in.

@seven-phases-max
Copy link
Member Author

(There were a few threads where a different opinions on possible strict units improvements but since that was spread all over the tracker (or at least I can't find a dedicated thread quickly) I guess I simply sound the key part of:)
My personal opinion on

strictUnits: ? - e.g. 1px / 1px // output 1, 100vh - 10px // output 100vh - 10px

is something like: would not be worth it (because of outcome/efforts ratio). In other words it's nothing to improve actually (except just fixing the obvious bugs like above).

Notice that to have it truly generic (in oppose to harcoding these and that minimalistic values/ops patterns/examples) you'll need to properly handle all the usual arithmetic equalities and thus introduce "imaginary" units like px² and similar. E.g. for a: 1px; b: 2px; c: 4px; an expected result of a*b/c (obviously accepting(a*b)/c = a*(b/c)) would be .5px meaning that the result of a*b have to be 2px² (at least internally).

There're other implementation concerns (obviously it would look strange to also count things like 1/1hz = 1s etc. but formally...) Counting in cents: I'd said CSS Units is just too big bear to poke it w/o a reason ("just because something like 1px/1px = 1 looks intuitive").

strictUnits: true - throw an error (I don't see how this is useful.)

Basically if one does not like the unit propagation and/or mixed units thing he may introduce a stricter coding style not allowing such arithmetic (thus current strictUnits: true is basically an option to force this coding style).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants