Skip to content

Confirmation on how get descriptor prepend/append works #65

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

Closed
MartyBolton opened this issue Mar 14, 2016 · 6 comments
Closed

Confirmation on how get descriptor prepend/append works #65

MartyBolton opened this issue Mar 14, 2016 · 6 comments

Comments

@MartyBolton
Copy link

So I'm using the get function with descriptor and append/prepend logic that we talked about in #63 and I have some weird jogging (race conditions) going on and I not sure if I'm interpreting the logic based on what is being called. I'm loading an object array lets call it posts and buffer size is 20. I should note that about 5 posts fit on a page, so I want to load up 20 at a time. It loads the first twenty fine, but when I scroll down, it looks like its calling for the next twenty which is correct, but immediately calls for the previous twenty starting from the current position -3 (which is the internal buffer overlap I guess).

So problem (a) - why do we need load the previous items that we already have?
Problem (b) seems to be this race condition where it calls get for the next twenty and previous 20 in a loop but offset each time. Here's the console results.

page load
load count(20), index (1), append(undefined), prepend(undefined) : 20 loaded first time - perfect
load count(20), index (-19), append(undefined), prepend(id:179) : 0 loaded - perfect
user scrolls down 20 
load count(20), index (21), append(id:43), prepend(undefined) - next 20 perfect
load count(20), index (-3), append(undefined), prepend(id:97) : 16 loaded - problem (a) 
load count(20), index (19), append(id:96), prepend(undefined) : 20 loaded - problem (b)
load count(20), index (-4), append(undefined), prepend(id:104) : 15 loaded - problem (a/b)
.
.

...and so on... I hope you can find that I'm doing something wrong. A few questions.

  1. Is there a demo using get descriptor prepend/append?
  2. I am loading an object array as reference, I assume this is okay, I don't have to push a copy of the array values right?

Here is the code I used to do cursor scrolling of results - it seems like the right approach, just not sure why this version of get is calling me in this fashion.

 var feedsource;
    feedsource = {};
    feedsource.get = function (descriptor, success) {
        var max_id, since_id;
        if (descriptor.append) {
            max_id = descriptor.append.aId;
        }
        if (descriptor.prepend) {
            since_id = descriptor.prepend.aId;
        }
        console.log('load count(' + descriptor.count + '), index (' + descriptor.index + '), append(' + max_id + '), prepend(' + since_id + ') ');
        loadFeedCursor(descriptor.count, max_id, since_id).then(function (resp) {
            console.log(':' + resp.items.length);
            return success(resp.items);   // array of my json objects - assuming this is okay
        });
    };



        <div ui-scroll-viewport fill-height>
            <ul class="list-group list-group-sm no-radius m-b-none m-t-n-xxs">
                <li ui-scroll="post in feedsource" buffer-size="20" class="list-group-item clearfix b-l-3x" adapter="feedAdapter.adapter">
                    <div ng-include="'tpl/feed.detail.post.html'"></div>
                </li>
            </ul>
        </div>
Marty
@dhilt
Copy link
Member

dhilt commented Mar 14, 2016

It's a bit difficult to catch your case without repro, e.g. what is loadFeedCursor(), why do you need an additional promise here, what is the height of your viewport and the height of your item (regarding to p.(a); also it is interesting to look at your items template itself)... It would be very helpful if you provide us with a demo (something like https://jsfiddle.net/dhilt/ypdbmrt8/). Thank you!

@MartyBolton
Copy link
Author

Hi Denis, thanks for your feedback. I was able to mock up a working fiddle for this that might also be used as a good example. At first, I could not create an issue but I remember asking Michael I think about irregular heights of list items. Indeed, I think this is an issue, so I created an example where you can toggle random height of list items, and then as you scroll the scroller loads the next and previous depending on a state and it can wind up in a finite or in my case, worse race condition that makes for poor rendering. I added console.log messages that matched my issue.

https://jsfiddle.net/MartyBolton/s38zdu8c/6/

Let me know what you think. I don't know the internals but I would guess this would not be difficult to fix, i.e. consider two buffers so there is room for rendering slack.

I really want to use this in my cordova mobile app, so for now, I'm going to use fixed heights.

I also want to know if I can show a spinner in the list item - if you have an example of that - would be cool.
thanks

first time using fiddle, let me know if you cannot get to that.

@MartyBolton
Copy link
Author

Hi Denis, were you able to check out the repo above? I am using ui-scroll right now with fixed height items for a feed but later this year I would like to use this with dynamic height. I'm not sure if the padding feature would help here or not? Let me know if you need more info.
https://jsfiddle.net/MartyBolton/s38zdu8c/6/
thanks

@dhilt
Copy link
Member

dhilt commented Mar 22, 2016

Now I see that we are talking about Average items height problem. My solution which I commited into paddings branch 3 days ago solves this problem. I've tried to inject this code into your demo and it works fine. But right now we are trying to find some more lightweight solution, so this work isn't finished.

@MartyBolton
Copy link
Author

Oh great! I was able to add padding to my app with buffer=20 and padding=1 and I am now able to use variable item height @:) (Sorry, at first read, I thought padding was a HTML padding thing :)). Also, I still see in my case anyway that I can find a spot where I scroll up and down slightly and get an append and prepend very close together, i.e. there should not be append/prepend datasource get calls that close together if buffer and padding are used. Maybe you have seen that. If you want to close this, I'm fine. I'll try to work with this solution for now with the extra padding.
thanks again! great work on this!

@dhilt
Copy link
Member

dhilt commented Apr 17, 2016

Fixed in v1.4.1.

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

2 participants