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

Numbers get rounded when using @arguments #2057

Closed
vkbansal opened this issue Jun 15, 2014 · 14 comments
Closed

Numbers get rounded when using @arguments #2057

vkbansal opened this issue Jun 15, 2014 · 14 comments

Comments

@vkbansal
Copy link

I'm using the following code.


.transitions(...){
    @arg : ~`"@{arguments}".replace(/[\[\]]*/g, '')`;
    -webkit-transition: @arg;
       -moz-transition: @arg;
        -ms-transition: @arg;
         -o-transition: @arg;
            transition: @arg;
}

.transitions(opacity 0.3s ease-in 0.3s, max-height 0.6s linear, margin-bottom 0.4s linear;);

And it produces the following result.


-webkit-transition: opacity 0s ease-in 0s, max-height 1s linear, margin-bottom 0s linear;
-moz-transition: opacity 0s ease-in 0s, max-height 1s linear, margin-bottom 0s linear;
-ms-transition: opacity 0s ease-in 0s, max-height 1s linear, margin-bottom 0s linear;
-o-transition: opacity 0s ease-in 0s, max-height 1s linear, margin-bottom 0s linear;
transition: opacity 0s ease-in 0s, max-height 1s linear, margin-bottom 0s linear;

As you can see 0.3, 0.4 get rounded to 0 and 0.6 gets rounded to 1

@vkbansal
Copy link
Author

The problem is with gulp-less not less. My bad.

@seven-phases-max
Copy link
Member

Just in case it does look like Less issue, e.g. v.1.7.1:

.transitions(...) {
    @arg: ~`"@{arguments}".replace(/[\[\]]*/g, '')`;
    1: @arg;              // rounded to integers
    2: ~`"@{arguments}"`; // rounded to integers
    3: @arguments;        // OK
}

But I'm not reopeing this since you don't need inline javascript there since Less v1.3.2 as soon as you use ; as the args delimiter (see [3] above).

@seven-phases-max
Copy link
Member

Oh, no, I guess I'll reopen this. It looks like it's a regression bug (introduced with #1814?) because it's compiled as expected with v1.6.x.
(@vkbansal but this does not cancel the recommendation to clean your code up of a legacy hacks).

@marekhrabe
Copy link

This also affects all mixins in LESS Hat. Are there any workarounds or is this a bug in core?

@vkbansal
Copy link
Author

@seven-phases-max I got this idea from LESS Hat, which is heavily depended on such 'legacy hacks'. Its very useful. This is a mixin, I created for positioning of elements after having a look at LESS Hat.

@petrbrzek
Copy link

This commit caused the problem. Quick fix is to use the interpolation syntax ~"0.5".

@seven-phases-max
Copy link
Member

@petrbrzek

Yes, most likely. Inline JavaScript stuff is not massively covered by the tests so we could just overlook the regression. I'll try submit a fix sooner (though it won't probably get into release soon since v1.7.1 has been just released).

@seven-phases-max
Copy link
Member

@vkbansal

This is a mixin, I created for positioning of elements

Same w/o javascript (I only changed property delimiter to , so the whole thing can be more generic).

(offtopic: Btw. this is the use-case where #1857 would come handy, implementation could become more simple (with args-pattern-matching instead of endless length(@style) > n)).

@vkbansal
Copy link
Author

@seven-phases-max
Thanks for the example. I did not know that it could be achieved that way. I was following the LESS Hat approach.

@lukeapage
Copy link
Member

@seven-phases-max see commit, it was caused by weird code (that I don't think would have ever made sense) in the jsify function.

I'm planning a 1.7.2 release for this issue and one effecting alot of grunt users.

@seven-phases-max
Copy link
Member

@lukeapage Thanks!

@vkbansal
Copy link
Author

@lukeapage Thanks!

@seven-phases-max Thank you too

@bluetidepro
Copy link

I have this same bug, is it fixed?

@vkbansal
Copy link
Author

@bluetidepro have a look at @lukeapage's reply above

I'm planning a 1.7.2 release for this issue and one effecting alot of grunt users.

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

No branches or pull requests

6 participants