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

Result of multiplication & division by % values off by 100× #1366

Closed
lucasrizoli opened this issue Jun 12, 2013 · 24 comments
Closed

Result of multiplication & division by % values off by 100× #1366

lucasrizoli opened this issue Jun 12, 2013 · 24 comments

Comments

@lucasrizoli
Copy link

In LESS 1.4, it seems that the result of multiplying by a % value is 100× what it should be; dividing by a % value, 100× smaller than it should be.

For example, foo: ( 50% * 256 ) will result in foo: 12800%, and bar: ( 256 / 100% ) results in bar: 2.56%.

One can work around it with, say, foo: ( 256 * ( 50% / 100 ) ); but that seems like a complicated way to get a result that, intuitively, seems straight-forward.

I cannot figure out if this odd result of multiplying and dividing by percentages in LESS is a known issue or if I'm just being thick—or both.

@lukeapage
Copy link
Member

Its not specific to 1.4, its always been like that (testing on http://less2css.org/)

It is related to the fact we ignore units - 12cm + 0.1m is not 22cm it is 12.1cm before 1.4. In 1.4 we fixed that.. but we haven't touched %'s.

I wish it wasn't a breaking change.. because I agree 50% * x should halve it.. but as it is it could break peoples less.

@lucasrizoli
Copy link
Author

Ah. Thanks.

I dunno why I'd assumed percentages would be an exception to this. Guess it was just the first time I'd come across an unintuitive result.

@lukeapage
Copy link
Member

I would like to keep this open.. I think you are right..

@extemporalgenome
Copy link
Contributor

Could we not add another initialization flag, in the vein of strictUnits? That way, if false (the default to begin with), it would not break any existing code.

Additionally, the expression (100cm/1m) should produce 1 theoretically (since the units are compatible), but in 1.4 does not, with or without strictUnits. I'm sure there are other open issues for this particular variation on unit handling, but it does seem slightly related to the issue of percent handling.

@jonschlinkert
Copy link
Contributor

We should also weigh in the fact that CSS calc accepts different units of measure, such as: width: calc(100% - 80px); (from the mozilla docs https://developer.mozilla.org/en-US/docs/Web/CSS/calc).

@extemporalgenome
Copy link
Contributor

With strictMath, we already side-step that issue somewhat. Although calc((100% - 80px)) is also valid CSS, there'd be little reason to do it outside of something like LESS. On the other hand, there are some expressions which must be parenthesized, such as calc((100% - 4px) / 3), that could still cause problems.

Another few options for getting around this: 1) avoid evaluating expressions, even when stored in variables, until required (such as when being passed into a defined function, which calc is not), or 2) make "expressions" the primary unit of storing and passing arithmetical notation, which would likely require a major rewrite of the internals, but could also simplify something like tree.Dimension such that the unit would be a single string rather than an object containing numerator and denominator lists.

@matthew-dean
Copy link
Member

We're thinking about this incorrectly. "%" is a valid unit designation, meaning that you are multiplying the total amount of units against your multiplier value. 50% * 10 = 500% for LESS, not 5 (sans unit). That's true because % is a CSS unit. Changing this behavior for % as units would break functionality for percentage units. You cannot infer that the user wanted to abandon the units, and wanted to instead interpret % as a math operation. The author may legitimately want to have 5x the percentage value, not reduce value based on a mathematical percentage. In the case of LESS, the % is just an arbitrary unit.

Changing the behavior only for % would break consistency of CSS units, whereas to do a math operation, a user only needs to refactor as (0.5 * 10).

The way calc() works is different because calc() runs in the context of the browser, so there are context values which are inferred but not visible in the equation.

So, for example, this formula:

width: calc(100% + 4em);

is really interpreted as:

width: calc(100%(x) + 4em(y));

with x being available width, and y being inherited em.

The usages for % are complicated enough that we cannot convert it's value. In the case of LESS, it's not a math symbol, it's a unit value. It only converts to a math operation in the context of a browser, and it does so on an invisible variable. It's similar to viewport units, which are designated only with vh and vw, and similarly, cannot be used to divide another unitless number by 100.

So, from my perspective, this is working as it should and always has. I understand why, for some, it's unexpected at first glance, because of the need to treat it as a unit within the context of LESS operations.

Agree with @lucasrizoli that this should be closed.

@extemporalgenome
Copy link
Contributor

@matthew-dean: reviewing the CSS3 spec, you're correct, and while percentage arithmetic is fully defined for addition and subtraction, it doesn't seem to be defined for multiplication and division when the other operand has a non-percentage unit. e.g. 50% * 3 is valid, but not necessarily 50% * 3px. If it truly is undefined, the question becomes: should we define our own behavior in the latter case, and treat 50% as a unitless 0.5 when attempting to resolve that expression, or should we continue to produce an incompatible type error when percentage is mixed with anything with a non-percentage unit?

In an ideal world, there'd be a middle ground where the LESS expression (8mm + 20% + 3cm) produces a tree.Anonymous of 38mm + 20%, which would only produce an error is passed to a builtin requiring a numeric, or in other cases when an expression must be concretely resolved.

@Soviut
Copy link

Soviut commented Jun 17, 2013

The way to think about percentages is as floating point values. 50% is 0.5 and 100% is 1.0. Percentages should always react the same way floats do. For example, 0.5 * 10 = 5.0. I think because percentages are represented as whole numbers rather than decimal fractions, people tend to get confused about their behaviour with other units.

@matthew-dean
Copy link
Member

@Soviut No, that's how one would think about them if they were doing simple math.

In the context of being a CSS pre-processor, the way we should think about them is percentage units. We are modifying the unit value.

So, as a test of unit math, swap pw (percentage width) in for %.

.span5 {
  width: (10pw * 5);
}
// result
.span5 {
  width: 50pw;
}

Now, it's obvious that what the author wants to do is have a row that spans 5 columns which are 10 percentage units wide. It's the same type of math as viewport units:

.span5 {
  width: (10vw * 5);
}
// result
.span5 {
  width: 50vw;
}

If we interpreted % as a float and dropped the unit as a result, you'd end up with something completely different:

.span5 {
  width: (10pw * 5);  
}
// result
.span5 {
  width: 0.5;
}

The result makes no sense in CSS, even though it makes sense in percentage math only if you reswap the type of unit to be represented by a %. If you want to do operations on a float value, use a float value. Don't use a CSS unit value. A % is a valid CSS unit value and must be preserved.

@Soviut
Copy link

Soviut commented Jun 17, 2013

I didn't mean literally interpret percentages as float units. I mentioned it as a way of letting people sort out potentially confusing percentage equations in their heads. "Think of it as a floating point amount rather than hundreds of something".

@matthew-dean
Copy link
Member

I see. So I'll close then, shall I?

@lucasrizoli
Copy link
Author

@matthew-dean, may as well.

Calculations with percentages (either in calc() or in pre-processing) are confusing; trading intuitiveness for consistency seems okay in this case.

@matthew-dean
Copy link
Member

Okey dokes.

@lukeapage
Copy link
Member

I disagree.

at least in strictUnits mode I'd like to do this

a: 10% * 10%; // error in strict units
b: 10% * 10;   // 100%
c: 10% * 10px; // 1px in strictUnits

strictUnits ensures the units are kept mathematically correct.. and the above is mathematically correct.

b/c of backwards compat I'd propose leaving non strictUnits

@lukeapage lukeapage reopened this Jun 20, 2013
@lukeapage
Copy link
Member

p.s. I realise the above is not what the original bug said!

@extemporalgenome
Copy link
Contributor

I'd like to re-emphasize that LESS handling multiplication and division involving percent and another unit makes sense (since none of the applicable CSS proposals have defined behavior for this aside from a percent times a unit-less value), but that LESS shouldn't handle addition or subtraction involving percents unless both operands are percents. Thus 10% * 10px or 10px * 10% is okay to resolve into 1px, but 10% - 10px and 10px - 10% should be either pass-through or an arithmetical error.

10% * 10% could be reasonably interpreted as resulting in 1%, but it would indeed be odd to see in practice.

@matthew-dean
Copy link
Member

@lukeapage Your examples are reasonable, but, as you note, that's the opposite of the request made. However, I see what you're wanting to accomplish. If we multiply a percentage by a unit, it's treated as math. If we multiply a percentage by a non-unit value, we assume the % IS the CSS unit, and treat it as unit math, not percentage math.

Seems alright, although I wonder if people would be confused by the different behavior. Seems weird to support such "switching", when % is a valid CSS unit. What's wrong with asking people to just use float values? If they use a % character, it LOOKS like they're implying the CSS % unit.

@extemporalgenome
Copy link
Contributor

@matthew-dean for one thing it aids compression with producing equivalent results to what the browser would accomplish. In some cases, it can do some things that a browser will not. For example, iirc, a browser is not required to interpret the attribute width: 100% / 6 unless it's wrapped in a calc call. LESS could handle that, and there are cases where it's reasonable to do so -- it's much more readable to do width: (100% / 6) (assume one or both of the operands is actually a variable) than it is to do width: (100 / 6)%.

Other than that, percent is a special-case unit, and LESS doesn't try to protect users from a lack of understanding when it comes to CSS -- if you understand CSS, then you should expect special handling of '%' in any case.

@lukeapage
Copy link
Member

The reason I would enable it for strictUnits is that at the moment I think 12% * 100px (with strictUnits on true) throws an error for incompatible units. Might need to check.

@acli
Copy link

acli commented Aug 26, 2017

Can we just have an option to completely disable calculations?

@seven-phases-max
Copy link
Member

seven-phases-max commented Nov 7, 2017

@acli

Can we just have an option to completely disable calculations?

The option is there for a while.

@seven-phases-max
Copy link
Member

seven-phases-max commented Nov 7, 2017

My two cents for the topic would be:
I just can't see how a magical of operator (as in A% of B) can be associated with the multiplication operator (as in A * B -> A repeated B times then A * B = B * A and so on).
Yeah, I know, many programs do this, even my calculator does this.. but this is still silly (these things are also dumb when it comes to operator precedence and missing most of other base arithmetic lessons from the grade 1, so that they are barely to be used as a model for a programming language).

I think the current Less behaviour for the use-case statements (both su on & off modes) is fine. And if there's a need for an of operator - it should be just that - the of operator (How did * or / come to be associated with that percentage thing at all? Doh! ).

@stale
Copy link

stale bot commented Apr 17, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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

8 participants