search

rdallasgray / roto

A simple, flexible, touch-capable scrolling plugin for jQuery.
http://rdallasgray.github.com/roto/
BSD 2-Clause "Simplified" License
72 stars 10 forks source link

Naja #24

Closed najamelan closed 11 years ago

najamelan commented 11 years ago

sorry about the whitespace, my editor is set to remove trailing whitespace automatically...

the only relevant commit is: 425acd3cc444117d511a9f83f007725cf1d27c39

rdallasgray commented 11 years ago

Hi Naja -- what's the reason for the change?

najamelan commented 11 years ago

Making the container to small wraps the kids. (this is on a container with about 50 spans for which outerWidth returns a value not integer. Over the total I get about 3 pixels short due to the rounding down. That's enough for firefox to wrap the last item to a next line.

However, I noticed today that it is still not entirely right. It wraps no longer but now it scrolls a bit to far to the left, leaving some extra space on the right. Also I have the problem that the event rotoChanged returns the second element instead of the first.

I tried to understand the code, but it is not easy and I checked minOffset (which seems to have a determining factor in how far one can scroll), but that seems to be calculated correctly.

For it claiming the second element to be the first visible in the container, I had a look at getNearestRotoKidTo but couldn't yet figure it out.

I should make a self contained sample to demonstrate, also to make sure that it is reproducible in a simple context, but it seems to me that setting the width of the container to narrow for it to contain all the elements can't be good.

najamelan commented 11 years ago

This demonstrates the wrapping:

just put it in your demo folder and run https://raw.github.com/najamelan/roto/f87a9c2643aeb67d2ed5b9b579653e17e0fa7cdf/demo/index2.htm

najamelan commented 11 years ago

Actually, even with just 2 elements it wraps. I think as soon as the user adds an element with a non integer outerWidth, it will wrap.

If you take the version of roto.js that I proposed, it no longer wraps, but by scrolling to the right we get a bit of extra whitespace. Making the container a bit bigger must upset some calculation elsewhere...

I also think that it's best not Math.ceil either, but you're to know better why you wanted integers in the first place.

najamelan commented 11 years ago

I did some testing and it appears that rotoChange correctly sets the first element if all elements are integer with, so I suppose we should just consider that roto doesn't support non integer width elements at the moment.

That means you can't just put text in a span without setting it's width.

The source of my non-integer outerwidth was setting a marging using em.

rdallasgray commented 11 years ago

OK, yes, that seems like a necessary limitation, I'm afraid.