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

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

Open
dhilt opened this issue Jul 20, 2015 · 34 comments
Open

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

dhilt opened this issue Jul 20, 2015 · 34 comments

Comments

@dhilt
Copy link
Member

dhilt commented Jul 20, 2015

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

@mfeingold
Copy link
Contributor

@SaintFlipper can you check if this fixes your issue?

@SaintFlipper
Copy link

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
Copy link
Member Author

dhilt commented Jul 31, 2015

@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
Copy link

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
Copy link
Contributor

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
#8 (comment).

@mfeingold
Copy link
Contributor

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

@dhilt
Copy link
Member Author

dhilt commented Aug 4, 2015

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
Copy link
Member Author

dhilt commented Aug 5, 2015

The 1.3.1 version is released (#12).
And now you can also obtain the release data from js-sources.

@dhilt dhilt closed this as completed Aug 5, 2015
@marknadig
Copy link
Contributor

@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
Copy link

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
Copy link
Contributor

@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 89bc6fc. Meanwhile I published v1.3.1 manually and it is now accessible.

@dhilt
Copy link
Member Author

dhilt commented Aug 6, 2015

@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
Copy link

@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
Copy link
Contributor

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

@SaintFlipper
Copy link

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
#8 (comment).

@stnash
Copy link

stnash commented Aug 28, 2015

The fix posted in ardas@6080a88 resolved this issue for me and I don't see any duplicate items. Would suggest merging this change into baseline.

@SaintFlipper
Copy link

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/ui-scroll@6080a88
ardas@6080a88
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
#8 (comment).

@stnash
Copy link

stnash commented Aug 28, 2015

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
Copy link
Contributor

@dhilt can you have a look?

@SaintFlipper
Copy link

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
#8 (comment).

@SaintFlipper
Copy link

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
#8 (comment)
.

@stnash
Copy link

stnash commented Aug 29, 2015

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

@tylkomat
Copy link

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
Copy link

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
#8 (comment).

@tylkomat
Copy link

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
Copy link

hornetnz commented Mar 2, 2016

Your fiddle isnt working tylkomat.

@tylkomat
Copy link

tylkomat commented Mar 2, 2016

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

@hornetnz
Copy link

hornetnz commented Mar 2, 2016

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

@batista
Copy link

batista commented Aug 18, 2016

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
Copy link

@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
Copy link

batista commented Aug 18, 2016

@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
Copy link
Contributor

mfeingold commented Aug 18, 2016

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
Copy link

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

Thanks in advance!

@Climax777
Copy link

Anyone try this with a flex-box setup yet?

@angular-ui angular-ui deleted a comment May 13, 2020
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