Skip to content

Keep loading elements until parent overflows #21

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
dragantl opened this issue Oct 21, 2016 · 12 comments
Closed

Keep loading elements until parent overflows #21

dragantl opened this issue Oct 21, 2016 · 12 comments

Comments

@dragantl
Copy link
Contributor

Thanks a lot for the lib. Its really easy to integrate. I ran into an issue only on a small detail. My page uses a flexible grid whose items shrink and grow as the window resizes. As such, it may be the same that on various screens, the initial load may not result in enough items to overflow the parent. If that happens, user has no way of loading more as there is no scroll. Is it possible for the lib to call on-infinite until the list container overflows the parent? There is another potential issue with resizing of the screen that will not be picked up to have more items loaded.

Do you have any suggestions?

@dragantl
Copy link
Contributor Author

I guess the logic can be optimized further. If the distance property uses the spinner of the infinite-loading element to make calculations, the on-infinite can be called as long as the bottom of the infinite-loading element is above the scroll parent and the complete hasn't been called. Did that make sense? This can then easily be hooked to monitor resize event of the parent.

@PeachScript
Copy link
Owner

Glad you like it :)

You're right, this is an interesting problem, thanks!

You mean it should to check the distance immediately after received $InfiniteLoading:loaded event, if the distance is still close to the bottom, it means content is not enough, we need to load more data again, all right?

@dragantl
Copy link
Contributor Author

Yes, that's exactly right! If the distance is above the bottom after the load event, more should be fetched. Otherwise, should wait for scroll event to bring it close to the bottom.

@JeanLucEsser
Copy link

JeanLucEsser commented Oct 25, 2016

That may resolve my own issue which was to load content on page load if the page was already too long to have the proper distance to trigger loading. Take the case of a right column with many filters that could be quite long, when you load the page nothing is loaded as the distance to the bottom is not met.

I dealt with it this way in my Vue instance, but it is a temporary fix:

    mounted: function () {
        this.$refs.infiniteLoading.isLoading = true;
        this.onInfinite();
    },

@dragantl proposal would solve this.

Sort of a init property with auto/fill/none values to address @dragantl issue or even the number of results to get on first call. This way you could even start blank if you want to and trigger the first load on your own terms, let's say after a filter has been set.

The idea is to separate the auto loading dependent on distance (which would be triggered only if isFirstLoad = false) from the first load which could be auto, none, fill... or manually triggered.

One last thing, it would be great to have an option to manually call for more (button load more results) in place of the distance parameter, if it better suits our needs.

Almost forgot: Really great and useful component, simple and easy to use 👍

@dragantl
Copy link
Contributor Author

I have modified my local library with the following changes and it seems to work fine:

events: {
    ...
    '$InfiniteLoading:loaded': function loaded() {
        this.listenTransitionEndOnce(() => {
            this.isFirstLoad = false;
            const currentDistance = getCurrentDistance(this.scrollParent);
            if (this.isLoading && !this.isComplete && p <= this.distance) {
                this.onInfinite.call();
            } else {
                this.isLoading = false;
            }
        });
    },
    ...
},

In my code, I then used $nextTick to call loaded and complete events. However, the version of this library I had did not yet have the listenTransitionEndOnce listener. As such, it may not be necessary with the newer code. The reason it was required was because the view wasn't added in the same tick so getCurrentDistance would not account for the newly added elements and would enter an infinite loop. With the addition of listenTransitionEndOnce, this I believe is not necessary as the callback is called irregardless of whether there is a transition specified or not.

@PeachScript Your thoughts? Can you look at and apply the changes? Should I make the PR?

PS: This added logic can be extracted to a separate function and called in loaded event as well as during resize of the container element. It can also then be provided as a public method to manually load more elements.

@PeachScript
Copy link
Owner

@JeanLucEsser Thanks for your solution and glad you like it!

You mean this component need to add a property to indicate whether or not to check the distance immediately, is that right?

@PeachScript
Copy link
Owner

@dragantl Thanks for your work! But I made a mistake before, I trying to let this component support transition feature of Vue.js( issue #11 ), and I encountered some problems that cause the work cannot continue, as you can see, the listenTransitionEndOnce cannot work properly forever, so the code of the master branch is wrong and I cannot merge PR temporarily, I'm so sorry.

At the beginning I should create a new branch to program, now I will fix the transition problem as soon as possible, I also think we should extracted to a separate function to check the distance and call onInfinite callback for scroll event listener, $InfiniteLoading:loaded event handler and resize event listener. Could you please make the PR after that?

@dragantl
Copy link
Contributor Author

@PeachScript Sure, just let me know when and I'll make a PR.

@PeachScript
Copy link
Owner

@dragantl Maybe I need more time to support the transition feature, but there has many problems that need to be resolved, so I revert the unfinished changes just now, you can make a PR base on the latest code on master branch, make sure the unit test can be passed before you commit it please, thank you very much!!

@dragantl
Copy link
Contributor Author

Ok, I'll make PR today/tomorrow

@dragantl
Copy link
Contributor Author

dragantl commented Nov 3, 2016

Got to it just now.

@dragantl
Copy link
Contributor Author

dragantl commented Nov 7, 2016

I think this can be closed now that #23 has been merged. Thanks!

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

3 participants