angular-ui / ui-scroll

Unlimited bidirectional scrolling over a limited element buffer for AngularJS applications
http://angular-ui.github.io/ui-scroll/demo/
MIT License
327 stars 107 forks source link

A problem with multiple items per row (inline, float-left) #8

Open dhilt opened 9 years ago

dhilt commented 9 years ago

The story of inline and float-left elements is still not over. Just run this demo and try to scroll up.

mfeingold commented 9 years ago

@SaintFlipper can you check if this fixes your issue?

SaintFlipper commented 9 years ago

Hi, the new #8 seems to change the behaviour but not quite fix it: a) It still results in duplicate items when scrolling back to the top (see image) b) Usually it doesn't allow scrolling back to item 1, as shown in the image where item 7 is as low as it goes. It's not clear what the circumstances are for this, but making the results frame 6 items wide in http://jsfiddle.net/mn59dvjn/7/ seems to allow this to be reproduced.

c) Having said that, in my real-world code (not stand-alone) it's improved as it consistently allows scrolling back to item 1, but still shows the same "bouncing" behaviour where it alternates between requesting low and high item regions; here are the last few item requests when scrolling up:

data source get : index=10, count=10 data source get : index=62, count=10 data source get : index=7, count=10 data source get : index=59, count=10 data source get : index=4, count=10 data source get : index=56, count=10 data source get : index=1, count=10 data source get : index=53, count=10 data source get : index=-2, count=10 data source get : index=50, count=10

By the way, is there a version identifier in ui-scroll that can be queried or seen in the source? It's difficult to be sure that the latest version is being loaded (and the older version isn't being cached in a real-world case).

image

dhilt commented 9 years ago

@SaintFlipper I've forked your fiddle and tuned the edge cases logic within your controller: http://jsfiddle.net/q4xstonz/ It seems that the duplicate issue is gone.

@mfeingold But what I really dislike is that the scrollbar heigth is being recalculated incorrectly (during scroll-up) and there is no chance to scroll to the top of the list (long enough list I mean) from the very bottom without additional clicking...

SaintFlipper commented 9 years ago

Thanks for that; my previous logic for handling out-of-range items explains the different behaviour of the stand-alone case, but not the "bouncing range" data source request or the difficulty scrolling to the top of the list which you mention. I suspect those symptoms are related. Have you looked at the console to see the data source requests which the JSfiddle data source sees? Below is what I see with a 6-image wide results pane. As you scroll upwards the data source sees alternate low & high range requests which I think garbles the display. For example as you initially scroll down the leftmost item number moves up in multiples of 6 (1,7,13,19...) as expected, but when scrolling up it jumps around almost randomly.

Eventually the display sorts itself out when it reaches item 1, but it doesn't look right until that point.

12:07:22.163 "data source get : index=1, count=10"      // Initial load
12:07:22.235 "data source get : index=11, count=10"
12:07:22.478 "data source get : index=21, count=10"
12:07:22.573 "data source get : index=31, count=10"
12:07:22.697 "data source get : index=41, count=10"
12:07:22.800 "data source get : index=-9, count=10"
12:07:28.389 "data source get : index=51, count=10"     // Scrolling down starts here
12:07:28.762 "data source get : index=61, count=10"
12:07:29.611 "data source get : index=71, count=10"
12:07:30.976 "data source get : index=81, count=10"
12:07:31.142 "data source get : index=91, count=10"
12:07:34.372 "data source get : index=101, count=10"
12:07:35.775 "data source get : index=45, count=10"     // Scrolling up starts here
12:07:35.833 "data source get : index=103, count=10"
12:07:35.972 "data source get : index=41, count=10"
12:07:36.031 "data source get : index=99, count=10"
12:07:36.386 "data source get : index=37, count=10"
12:07:36.617 "data source get : index=95, count=10"
12:07:37.189 "data source get : index=27, count=10"
12:07:37.408 "data source get : index=67, count=10"
12:07:37.524 "data source get : index=17, count=10"
12:07:37.619 "data source get : index=69, count=10"
12:07:37.807 "data source get : index=7, count=10"
12:07:37.850 "data source get : index=65, count=10"
12:07:38.413 "data source get : index=-3, count=10"     // Displays item #1 here
12:07:38.706 "data source get : index=61, count=10"
mfeingold commented 9 years ago

I believe we know the root cause of the problem. will have a fix shortly

On Sat, Aug 1, 2015 at 6:34 AM, SaintFlipper notifications@github.com wrote:

Thanks for that; my previous logic for handling out-of-range items explains the different behaviour of the stand-alone case, but not the "bouncing range" data source request or the difficulty scrolling to the top of the list which you mention. I suspect those symptoms are related. Have you looked at the console to see the data source requests which the JSfiddle data source sees? Below is what I see with a 6-image wide results pane. As you scroll upwards the data source sees alternate low & high range requests which I think garbles the display. For example as you initially scroll down the leftmost item number moves up in multiples of 6 (1,7,13,19...) as expected, but when scrolling up it jumps around almost randomly.

Eventually the display sorts itself out when it reaches item 1, but it doesn't look right until that point.

12:07:22.163 "data source get : index=1, count=10" // Initial load 12:07:22.235 "data source get : index=11, count=10" 12:07:22.478 "data source get : index=21, count=10" 12:07:22.573 "data source get : index=31, count=10" 12:07:22.697 "data source get : index=41, count=10" 12:07:22.800 "data source get : index=-9, count=10" 12:07:28.389 "data source get : index=51, count=10" // Scrolling down starts here 12:07:28.762 "data source get : index=61, count=10" 12:07:29.611 "data source get : index=71, count=10" 12:07:30.976 "data source get : index=81, count=10" 12:07:31.142 "data source get : index=91, count=10" 12:07:34.372 "data source get : index=101, count=10" 12:07:35.775 "data source get : index=45, count=10" // Scrolling up starts here 12:07:35.833 "data source get : index=103, count=10" 12:07:35.972 "data source get : index=41, count=10" 12:07:36.031 "data source get : index=99, count=10" 12:07:36.386 "data source get : index=37, count=10" 12:07:36.617 "data source get : index=95, count=10" 12:07:37.189 "data source get : index=27, count=10" 12:07:37.408 "data source get : index=67, count=10" 12:07:37.524 "data source get : index=17, count=10" 12:07:37.619 "data source get : index=69, count=10" 12:07:37.807 "data source get : index=7, count=10" 12:07:37.850 "data source get : index=65, count=10" 12:07:38.413 "data source get : index=-3, count=10" // Displays item #1 here 12:07:38.706 "data source get : index=61, count=10"

— Reply to this email directly or view it on GitHub https://github.com/angular-ui/ui-scroll/issues/8#issuecomment-126904359.

mfeingold commented 9 years ago

@SaintFlipper @dhilt guys, can you try it again? I think I eliminated the jumping. Can you confirm?

dhilt commented 9 years ago

A new issue was found (https://www.youtube.com/watch?v=xwjupzIwQXA). We are going to fix it and cover it with tests. The 1.3.1 release is comming soon.

dhilt commented 9 years ago

The 1.3.1 version is released (https://github.com/angular-ui/ui-scroll/pull/12). And now you can also obtain the release data from js-sources.

marknadig commented 9 years ago

@dhilt @PowerKiKi is there a manual step to publish this to npm, or is it a timing issue? Not seeing 1.3.1 out there. Thank you.

SaintFlipper commented 9 years ago

I'm not seeing a big difference from the previous version. Using http://jsfiddle.net/q4xstonz/1/ and with the results pane 6 items wide I still see the "bouncing data source range" behaviour (see below log) and the leftmost displayed item number doesn't follow the expected sequence when scrolling upwards (ie. not 1 + 6N where N is the row number).

Can I ask again whether there's some way to query the ui-scroll version number, or at least to see the version number somewhere in the source? It's not obvious by viewing the js source that the latest ui-scroll is being used.

data source get : index=1, count=10  // Initial load
data source get : index=11, count=10
data source get : index=21, count=10
data source get : index=31, count=10
data source get : index=41, count=10
data source get : index=-9, count=10
data source get : index=51, count=10  // Scrolling down
data source get : index=61, count=10
data source get : index=71, count=10
data source get : index=81, count=10
data source get : index=91, count=10
data source get : index=101, count=10
data source get : index=111, count=10
data source get : index=121, count=10
data source get : index=63, count=10     // Start of scrolling up
data source get : index=121, count=10
data source get : index=59, count=10
data source get : index=117, count=10
data source get : index=55, count=10
data source get : index=119, count=10
data source get : index=51, count=10
data source get : index=115, count=10
data source get : index=47, count=10
data source get : index=117, count=10
data source get : index=43, count=10
data source get : index=113, count=10
data source get : index=39, count=10
data source get : index=115, count=10
data source get : index=35, count=10
data source get : index=111, count=10
data source get : index=31, count=10
data source get : index=113, count=10
data source get : index=27, count=10
data source get : index=109, count=10
data source get : index=23, count=10
data source get : index=111, count=10
data source get : index=19, count=10
data source get : index=107, count=10
data source get : index=15, count=10
data source get : index=109, count=10
data source get : index=11, count=10
data source get : index=105, count=10
data source get : index=7, count=10
data source get : index=107, count=10
data source get : index=3, count=10
data source get : index=103, count=10
data source get : index=-1, count=10  // Back to item no. 1
PowerKiKi commented 9 years ago

@marknadig packages are published automatically by Travis as soon as a tag is pushed. This was not the case for v1.3.1, because we explicitly told Travis to ignore tags. It is now fixed via https://github.com/angular-ui/ui-scroll/commit/89bc6fc53961dfd0523dd112751a754100b24a76. Meanwhile I published v1.3.1 manually and it is now accessible.

dhilt commented 9 years ago

@SaintFlipper on the version number through the source code, just added (since v1.3.1) a comments to the top of js-artefacts from dist/ folder (like https://github.com/angular-ui/ui-scroll/blob/master/dist/ui-scroll.js#L4), and threre is no release data within coffee sources

nikitagromov commented 9 years ago

@dhilt I also can reproduce this issue. I see this problem in jsfiddle and also have it in our project. I have downloaded and compiled last version and it's still reproducible.

mfeingold commented 9 years ago

can you be more specific? what exactly is off? If you have a fiddle can you share it?

SaintFlipper commented 9 years ago

Hi Michael

I know you were asking @nikita for this, but my fiddle at http://jsfiddle.net/q4xstonz/1/ also shows unexpected behaviour:

a) Set the result pane to 6 images wide then scroll down - the top-left item number goes up in steps of 6 (1,7,13,19,25,31...). Now scroll up and watch the top-left item number - it doesn't step down through the same sequence, for example I just saw 31,27,23,19,15,25,19,13,7,1. At the same time the rest of the items jump around, for example item 25 jumps from being at the left to 2nd from right.

b) Watching the logged data source requests shows the requested range jumping between low and high items, for example:

23:38:17.643 "data source get : index=1, count=10" angular.js:9101:18
// Initial load
23:38:17.699 "data source get : index=11, count=10" angular.js:9101:18
23:38:17.745 "data source get : index=21, count=10" angular.js:9101:18
23:38:17.793 "data source get : index=31, count=10" angular.js:9101:18
23:38:17.830 "data source get : index=41, count=10" angular.js:9101:18
23:38:17.915 "data source get : index=-9, count=10" angular.js:9101:18
23:38:20.961 "data source get : index=51, count=10" angular.js:9101:18   //
Scrolling down
23:38:21.195 "data source get : index=61, count=10" angular.js:9101:18
23:38:22.063 "data source get : index=71, count=10" angular.js:9101:18
23:38:28.138 "data source get : index=15, count=10" angular.js:9101:18   //
Scrolling up
23:38:28.188 "data source get : index=73, count=10" angular.js:9101:18
23:38:28.313 "data source get : index=11, count=10" angular.js:9101:18
23:38:28.354 "data source get : index=69, count=10" angular.js:9101:18
23:38:28.486 "data source get : index=7, count=10" angular.js:9101:18
23:38:28.521 "data source get : index=71, count=10" angular.js:9101:18
23:38:28.722 "data source get : index=3, count=10" angular.js:9101:18
23:38:28.765 "data source get : index=67, count=10" angular.js:9101:18
23:38:29.479 "data source get : index=-1, count=10" angular.js:9101:18   //
Display is back at item #1

I imagine that this is related to a), presumably the UI is jumping between rendering high and low items, resulting in items jumping around and the leftmost item not following the same predictable sequence shown when scrolling down.

Thanks

On 10 August 2015 at 20:01, Michael Feingold notifications@github.com wrote:

can you be more specific? what exactly is off? If you have a fiddle can you share it?

— Reply to this email directly or view it on GitHub https://github.com/angular-ui/ui-scroll/issues/8#issuecomment-129566761.

stnash commented 9 years ago

The fix posted in https://github.com/ardas/ui-scroll/commit/6080a88993c949d894c60c8521b0cf8d14b2ebec resolved this issue for me and I don't see any duplicate items. Would suggest merging this change into baseline.

SaintFlipper commented 9 years ago

The duplicate items had a different cause (handling the edge case when the data source requested items < 1), but v1.3.1 still showed the problem with the displayed items jumping around when scrolling up. Try http://jsfiddle.net/q4xstonz/1/ with the result pane set to 6 items wide - when scrolling down the leftmost item number follows the expected sequence (1,7,13,19,25...) but when scrolling up again item numbers jump around randomly (eg, 31,27,23,19,15,25,19...) until item #1 is eventually reached.

On 28 August 2015 at 18:24, stnash notifications@github.com wrote:

The fix posted in ardas@6080a88 https://github.com/ardas/ui-scroll/commit/6080a88993c949d894c60c8521b0cf8d14b2ebec resolved this issue for me and I don't see any duplicate items. Would suggest merging this change into baseline.

— Reply to this email directly or view it on GitHub https://github.com/angular-ui/ui-scroll/issues/8#issuecomment-135838830.

stnash commented 9 years ago

Does your JSFiddle incorporate the changes per the commit I referenced? I had issues with 1.3.1 also. But with the changes in that commit, I'm seeing consistent refreshes now scrolling up.

mfeingold commented 9 years ago

@dhilt can you have a look?

SaintFlipper commented 9 years ago

My apologies, no. I had inferred from the code comments that this was still v1.3.1.

On 28 August 2015 at 18:47, stnash notifications@github.com wrote:

Does your JSFiddle incorporate the changes per the commit I referenced? I had issues with 1.3.1 also. But with the changes in that commit, I'm seeing consistent refreshes now scrolling up.

— Reply to this email directly or view it on GitHub https://github.com/angular-ui/ui-scroll/issues/8#issuecomment-135843159.

SaintFlipper commented 9 years ago

Updated the fiddle: http://jsfiddle.net/q4xstonz/2/ to use the updated ui-scroll.js ( https://raw.githubusercontent.com/ardas/ui-scroll/6080a88993c949d894c60c8521b0cf8d14b2ebec/dist/ui-scroll.js ).

The updated ui-scroll greatly improves the behaviour and items no longer move around within each row (so the leftmost item follows the same sequence when scrolling down and up). It still shows the "bounce" behaviour though when using the scroll wheel or up/down keys. As ui-scroll nears the first row it jumps back down so the same row keeps reappearing at the top - I see the row starting at item 13 many times before the row with item 7 finally appears.

On 29 August 2015 at 12:16, Philip White pwhite000@gmail.com wrote:

My apologies, no. I had inferred from the code comments that this was still v1.3.1.

On 28 August 2015 at 18:47, stnash notifications@github.com wrote:

Does your JSFiddle incorporate the changes per the commit I referenced? I had issues with 1.3.1 also. But with the changes in that commit, I'm seeing consistent refreshes now scrolling up.

— Reply to this email directly or view it on GitHub https://github.com/angular-ui/ui-scroll/issues/8#issuecomment-135843159 .

stnash commented 9 years ago

Agreed, I see that same issue still as well. But at least it's one step closer ;-)

tylkomat commented 8 years ago

I found a solution for this problem which works with the latest version 1.3.2. The Trick is to let the datasource map the requested index and count to internal values and the result should be mapped to reflect the desired structure. Which means, if if I want to display 2 items per line, I will multiply each index - 1 (to make it zero based) and count that is requested at the datasource.get method with 2. So if index is 1 and count 3 the returned array of the datasource looks like this:

[
  [item1, item2],
  [item3, item4],
  [item5, item6]
]

The scroll view then handles the requested 3 items while 6 are displayed in reality, just view has to be updated to handle the new data structure. I didn't notice any scroll glitches so far. If someone is interested I can update one of the previous fiddles with an example.

SaintFlipper commented 8 years ago

Hi Matthias

That's a very useful observation, which hopefully might help the developers find a longer term solution to this problem.

It would be helpful if you could update the JSfiddle with your workaround, as I'm not completely clear on your explanation - are you suggesting that if get() receives a count of X items then it should return X_N contiguous items, where N is the number of items per row ? Or are you also saying that the item index is also multiplied by N, so, for example rather than querying items 1,2,3 you would query items 1,3,5,7,9,11 ? I created a quick test case assuming 4 items per row, returning X_4 contiguous items for each request, but I see duplicate items in the list, so maybe I'm misunderstanding your explanation.

One drawback with this workaround is that from your explanation it seems that get() needs to know how many items per row are being displayed. In the case of a resizeable DIV with float:left items that will be variable, and there would need to be a calculation to determine the count of items per row, which would have to take into account the item styling (borders, padding, margins etc).

On 14 October 2015 at 14:02, Matthias Tylkowski notifications@github.com wrote:

I found a solution for this problem which works with the latest version 1.3.2. The Trick is to let the datasource map the requested index and count to internal values and the result should be mapped to reflect the desired structure. Which means, if if I want to display 2 items per line, I will multiply each index - 1 (to make it zero based) and count that is requested at the datasource.get method with 2. So if index is 1 and count 3 the returned array of the datasource looks like this:

[ [item1, item2], [item3, item4], [item5, item6] ]

The scroll view then handles the requested 3 items while 6 are displayed in reality, just view has to be updated to handle the new data structure. I didn't notice any scroll glitches so far. If someone is interested I can update one of the previous fiddles with an example.

— Reply to this email directly or view it on GitHub https://github.com/angular-ui/ui-scroll/issues/8#issuecomment-148042714.

tylkomat commented 8 years ago

Everything is multiplied with N. My explanation was more focused on my use case where I have ajax requests in my datasource, there I don't need add the index to the count. Anyway here is a fiddle: http://jsfiddle.net/q4xstonz/17/. The only important things there are buffer-size and itemsPerRow. Hope it helps

Yes the drawback is that you have to know the items per line. But that can be fixed or has to be calculated.

The problem with the jumping around with dynamic items per line is that the scroller expects 10 items (buffer-size=10) which results in a height of the content area of 10 * itemHeight, but the real content area height is 10 * itemHeight / itemsPerLine. So when you scroll and new Items are added the scroll position is also adjusted, which results in jumping around as the real and the calculated positions don't match, because the scroller can not know how many items there are per line. The expectation is one.

hornetnz commented 8 years ago

Your fiddle isnt working tylkomat.

tylkomat commented 8 years ago

It's working for me. What is not working for you?

hornetnz commented 8 years ago

I had to change the ui-scroll js link to rawgithub.com. It gave an error otherwise.

batista commented 8 years ago

any updates on this one? am trying to follow up on @tylkomat fiddle, but with 8 elements per row. it works ok-ish, but if i keep scrolling the second time it loads, it will roll back up the scroll and start from the top. check the fiddle here

tylkomat commented 8 years ago

@batista your algorithm is off, the first line has only three items, if you put a clear on the div with the ui-scroll directive, you will see what I mean. Anyway you need that, the lines should not mix. If you have six lines they should be six lines on any screen and not 4 on a large and 10 on a small. Otherwise it messes with the continuous scroll detection of ui-scroll.

As for the jumping back and forth I also don't have an idea.

batista commented 8 years ago

@tylkomat thanks! i was fiddling with yours and forgot to update the if condition for the push to temp array.

here's the updated (and more dynamic) way: if (i % itemsPerRow === itemsPerRow - 1) {

also updated fiddle here where the itemsPerRow var works as expected.

mfeingold commented 8 years ago

This is a slippery one. My next big idea was to refactor all functions replacing code relaying on height to relay on positions, Unfortunately at the moment I do not have the bandwidth to deal with this. Any volunteers?

gjwilk01 commented 7 years ago

Any plans to address the backward scroll issue discussed in this topic? Any good workarounds?

Thanks in advance!

Climax777 commented 5 years ago

Anyone try this with a flex-box setup yet?