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

subtract add don't act as expected in some cases #33953

Closed
oliwerAR opened this issue May 12, 2021 · 3 comments · Fixed by #34047
Closed

subtract add don't act as expected in some cases #33953

oliwerAR opened this issue May 12, 2021 · 3 comments · Fixed by #34047
Labels

Comments

@oliwerAR
Copy link

The functions do not act as I would expect (as I think is intuitive) when the result of the add function is used as an argument for the subtract function.

Eg.
subtract(100%, add(1rem, 1em, false));
returns
calc(100% - 1rem + 1em)

I would expect
calc(100% - (1rem + 1em))
or
calc(100% - 1rem - 1em)

@ffoodd
Copy link
Member

ffoodd commented May 17, 2021

Thanks for reporting!

Would you mind providing a real example please, to help figure it out? I see your point but your examples are incorrect since our functions would have solved the rem addition or subtraction before any output.

Regardless, always providing parenthesis could break things (also I don't know which, just a possibility). I say we have two ways to handle this:

  1. either we always add parenthesis and try to minimize side effects, considering there might not be any?
  2. or we provide another parameter to change the output, just like $return-calc does already.

Not sure what's the best way for now, still thinking out loud.

@oliwerAR
Copy link
Author

Please, notice that the second number is in rem and the third is in em so they wouldn't be reduced to one number by the add function.

A real word example - one that actually made me realize the problem. A have the .btn element, with a text and and and icon inside. Inside of the .btn element I want to have a (absolutely positioned) ::before pseudo element. The width of the ::before should equal the with of the button minus the size of the of the icon and doubled horizontal padding of the button.
It would seam logical to me, to construct the formula like this (because the size of the icon and padding might (or might not) be expressed with the same unit):

width: subtract(100%, add(2 * $btn-padding-x, $btn-icon-size, false));

Now let's say:

$btn-padding-x: 0.75rem ;
$btn-icon-size: 1.2rem;

That outputs:

width: calc(100% - 2.7rem);

which is perfectly correct.

But if the size of the icon is expressed in em (instead of rem) which is a perfectly valid case:

$btn-padding-x: 0.75rem;
$btn-icon-size: 1.2em;

you get:

calc(100% - 1.5rem + 1.2em);

Witch is incorrect!!!

So one expression outputs correct and incorrect values depending on the units of the variables - that's very misleading.

It should output:

calc(100% - (1.5rem + 1.2em));

or

calc(100% - 1.5rem - 1.2em);

There's actually another way to tackle that problem - make the subtract function do the work.
If the arguments are not compatible (are expressed with different units) then it should check if the second argument is a complex expression (not a single value) and if it is :
option a) put it in parentheses (this would be more human understandable approach I think);
option b) change the sign of every component (this give slightly shorter output but may be problematic if custom properties are used within, though still doable I think).

As far as I know you may use parentheses in calc however you want (to establish computation order when needed - see mdn page). Actually I think calc with calc is valid as well (though not necessary).

@ffoodd
Copy link
Member

ffoodd commented May 18, 2021

Option a) seems a good idea to me!

Changing the sign will very likely introduce new edge cases, seems more risky.

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

Successfully merging a pull request may close this issue.

3 participants