abitgone / jQuery-Plugins

A bunch of plugins that ease development for front-end designers with little or no JavaScript or jQuery knowledge.
http://abitgone.github.com/jQuery-Plugins
Other
44 stars 14 forks source link

Pixels from next image exposed in some browsers in some cases #11

Closed Andrej2 closed 11 years ago

Andrej2 commented 11 years ago

In the horizontal scrolling scenario, when there are 3 images only, in some browsers (tested on Opera 12.16/Win, more may be affected) we can see 1 extra pixel to the right of the second image and 2 extra pixels to the right of the third image.

The cause is that in this case, container width gets 300%, and item width is set to 33.33% (which is smaller than 1/3). As a result, item width is becomes a rational number that gets truncated and ends up finally as 1px smaller than it should. (Other browsers I tested use rounding, which is better.)

I was able to fix it by simply removing that line (line 110: //$items.width(100/items + '%'); as items have a set width from CSS.

I'd suggest setting an absolute pixel value that we already know (container's original width in pixels), instead of a percentage that's prone to rounding/truncation.

(Or maybe skip setting a width if there is a width set by a style - overriding inline styles requires !important that used to "not work" in all cases.)

abitgone commented 11 years ago

This, unfortunately, is down to browser rounding errors. Because of the responsive nature of a lot of the sites I'm working with at the moment, it's not possible to use pixel- or em-based widths/heights on the carousel or the carousel items.

Without putting a lot of extra code in to constantly watch for when the browser resizes – a costly operation at the best of time, which results in a lot of 'jank' – it won't be possible to put a one-size-fits-all fix in for this.

If you have a fixed size design, with a fixed number of carousel items, and you know what you're dealing with, you could use the 'roll-your-own' styles and use attribute selectors. You'd need to override the position: absolute styles on the data-carousel-item divs and have them displayed correctly, but say, for example, that your data-carousel-item width was 200px, you could use the following styles:

[data-carousel-item=0] data-carousel-items {
    left: 0;
}
[data-carousel-item=1] data-carousel-items {
    left: -200px;
}
[data-carousel-item=2] data-carousel-items {
    left: -400px;
}

IE7 and above support attribute selectors. If you need to support IE6, you have bigger problems.

abitgone commented 11 years ago

Having realised that this won't actually solve the problem for you, I've added another data attribute - data-carousel-noabsolute-items. Add this to your data-carousel, ensure you're using data-carousel-style="class-only" and it will prevent the carousel items from being absolutely positioned inside the data-carousel-items container.

Coupled with the attribute selectors above, this should help you achieve what you need.

Andrej2 commented 11 years ago

Hey, thanks for the workaround. You are completely right about responsive designs, I forgot about them completely! I agree that recalculating pixel value on window drag is a waste of resources. So I had to come up with a different solution (if you are interested) using table layout. This works perfectly fine in all browsers I tested (except IE6, where the horizontal variant did not work anyway). Had to add a fallback to IE7 because display:table is supported from IE8 onwards, and IE7 is the popular compatibility mode for all IE's and of many Chinese browsers. So here is the diff to the CSS-only fix (no change to JS necessary):

@@ -351,7 +351,9 @@
 }

 .carousel-horizontal.carousel-ready .carousel-item { 
-    float: left; 
+    display:table-cell; 
+    *display: block;
+    *float: left;
 }
 .carousel-horizontal .carousel-items {
     -webkit-transition: 0.5s left ease-in-out;
@@ -359,6 +361,10 @@
     -o-transition: 0.5s left ease-in-out;
     transition: 0.5s left ease-in-out;
 }
+.carousel-horizontal.carousel-ready .carousel-items {
+    display: table;
+    *display: block;
+}
 .carousel-vertical.carousel-ready .carousel-item { 
     float: none;
     clear: both;

This addresses the CSS file, that is on the example page. I unfortunately cannot find the CSS file in the repository - but the JS is not self-contained, it really needs the CSS.

Now that I have the fix, I think it's not rude to talk a bit more about the background. I though the problem is because of some browsers' way of using only 2 decimal digits in percentage values, like 11.11%, but it's not. For testing, I changed your example page and increased the number of items to 9. I used an earlier Safari to test the plugin, and on default width it worked perfect.

When there are 9 elements, the width is 1/9, i.e. 0.11111111111111111111111111111111111111111111111111111 (etc.)

Safari for example uses 14 decimal digits, i.e. 11.11111111111111%, but even with this precision, I was able to find a width that results in 1px wrong values for width. (By looking at the last item, shrinked the browser until the bug appeared.)

The container width is 4509px - and the item width is 500px. (should be 501px but calculation is as follows. 4509px * 11.11111111111111% = 500,9999999999999499px which just misses the mark.)

Well, the problem with this fix is that Firefox does not support ''position: relative'' on table cells, so if we rely on positioning inside the carousel-items, we have to add extra markup :(