leafo / lessphp

LESS compiler written in PHP
http://leafo.net/lessphp
Other
2.2k stars 530 forks source link

calculation error #422

Open marten-seemann opened 11 years ago

marten-seemann commented 11 years ago

A rounding error in lessphp completely breaks my layout.

I'm using a Twitter Bootstrap 24 column layout. The calculation e.g. for the .span5 elements (the other .spanX are calculated wrong equivalently) using lessphp yields:

.row-fluid .span5 {
  width: 20%;
  *width: 19.9473684211%;
}

whereas the calculation using lessjs gives:

.row-fluid .span5 {
  width: 19.831223628691983%;
  *width: 19.778592049744613%;
}

A remarkable difference of about 0.17% points. On the default Bootstrap page width of about 1200 px this difference is equivalent to 2 px, enough to break a page layout.

JeromeLebret commented 11 years ago

Hello there !

I just wanted to say that I'm facing the same problem. I've seen the problem mostly on responsive design for large screens (+1200px) - below it seems ok.

Any idea to fix this properly ? (the best would be without "altering" the main Bootstrap CSS/Less files... but a temporary fix by modifying/overwriting the variables.less, responsive.less or responsive-1200px-min.less would already be nice).

Edit : after searching a bit, it seems that the temporary solution could be by overwriting mixins.less on lines 646 & 647

.span (@columns) {
  width: (@fluidGridColumnWidth * @columns) + (@fluidGridGutterWidth * (@columns - 1));
  *width: (@fluidGridColumnWidth * @columns) + (@fluidGridGutterWidth * (@columns - 1)) - (.5 / @gridRowWidth * 100 * 1%);
}

(This is NOT the solution)

ghost commented 11 years ago

Rounding issues with Twitter Bootstrap

atmediauk commented 11 years ago

Having the same issue here as well. Thought I was going mad. Tried a few themes including the stock bootstrap responsive with them all creating spans that were slightly to big resulting in them not sitting next to each other.

I don't mind longer compilation time if the precision is more accurate :)

marten-seemann commented 11 years ago

Maybe this will help you: https://github.com/leafo/lessphp/pull/429#issuecomment-20231266

atmediauk commented 11 years ago

@marten-seemann You legend lol. Works great. Thanks!

atmediauk commented 11 years ago

@marten-seemann Using this method and the fluid grid have you found when it hits a width that is roughly between 700-900px that the problem occurs again. The % columns that are generated for that media query seem to be slightly to big which like before results in items breaking over to the next line.

@media (max-width: 979px) and (min-width: 768px) .row-fluid .span6 { width: [[SLIGHTLY TO BIG]]%; }

The smaller resolution becomes a one column template so thats fine and the and larger resolutions seems to work fine as well, just this resizing in the middle.

markhalliwell commented 11 years ago

345

marten-seemann commented 11 years ago

I agree that this is a critical bug, and not at all solved by my workaround, though it mitigates the most severe calculation errors (at least in my case).

It is somehow related to inheritance of variables in (nested) function calls, otherwise it wouldn't make a difference if I call the same function twice in one document (e.g. bootstrap.less including responsive.less) or from two distinct files. Some develepor of lessphp should definitely have a look what exactly is going wrong here.

entr commented 11 years ago

Had the same problem with bootstrap. Turned out it was MY FAULT not importing responsive.less. I suppose I would have noticed that earlier if it wasn't for the fact even without responsive.less it produced the media queries. Looks more like a problem with mixin parsing than calculation error.

So just enqueue(excuse my WP terminology) responsive.less or copy its contents(without first too imports - variables.less and mixins.less) to the bottom of bootstrap.less if you want everything to be in one place. That's what I did. Everything seems to be OK now.