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

Trigger offset added #7

Closed
wants to merge 2 commits into from
Closed

Trigger offset added #7

wants to merge 2 commits into from

Conversation

GorkaMM
Copy link

@GorkaMM GorkaMM commented Feb 26, 2015

I added a property to set an offset between the real end of the scroll view content and the scroll position, so the handler can be triggered before reaching end. I also updated the example.

@GorkaMM
Copy link
Author

GorkaMM commented Feb 26, 2015

If you merge this, please release a new version.

Thank you!

@pronebird
Copy link
Owner

Hi, Thanks for your PR. I am curious, what's the use case for that?

p.s.: a pod spec with link to your fork is a part of PR now :-)

@GorkaMM
Copy link
Author

GorkaMM commented Feb 27, 2015

Hi,

I added this because I wanted the fetching of more data to happen before the user really reached the end, so it could be there faster for him/her. I don't know if this will affect any other calculations you perform in the Category though.

Thank you!

@pronebird
Copy link
Owner

Alright, I see.

I need to test it first, because we have couple of edge cases like when there is not much content in scrollview, we add extra bottom inset to position scroll indicator below visible bounds.

Also, maybe it would be good to use relative dimensions instead of points. (e.g. preload when user is half-way through, 0..1)

@pronebird
Copy link
Owner

I've been thinking about that improvement and I suggest that we implement it as a block that receives enough information to calculate custom offset.

For example:

CGPoint offset = triggerOffset( proposedTriggerOffset, contentSize, scrollViewSize );

Custom implementation that preloads more content when we reach 75% of scroll view content would look then as following:

CGPoint(^customOffsetHandler)(CGPoint proposed, CGSize contentSize, CGSize scrollViewSize) {
    CGPoint pt;
    pt.x = proposed.x;
    pt.y = proposed.y - contentSize.height * 0.25;

    return pt;
};

scrollView.infiniteScrollTriggerOffsetHandler = customOffsetHandler;

Certainly some flag should be maintained to avoid repetitive calls to infinite scroll handler in case if no more content is available. Right now it is handled nicely because indicator stays within content inset bounds and scroll is always pushed up if content size didn't change after all animations finished.

@pronebird
Copy link
Owner

I addressed this improvement in the following commits:

  1. Added custom action offset handler: commit b008db7
  2. Added demo handler: commit 97a5035

Default formula is:

content.height or bounds.height (whatever is larger) – bounds.height + originalBottomInset

Since trigger offset can be customized, I expect scroll view will be spamming infinite scroll handler in case if there is no more content to preload and content area does not grow.

But the good thing, due to our delayed messaging system, it won't be spamming it all the time, but only when scroll view stops.

Not sure if that should be addressed.

I would be grateful if you could take a look at custom-action-offset branch and test it.

@whitefoxy
Copy link

When this feature will be released?

@pankkor
Copy link

pankkor commented Dec 8, 2015

This is definitely a must have feature for seamless infinite scrolling experience

@zvonicek
Copy link

zvonicek commented Mar 2, 2016

This feature is really needed and I'd love to see it merged!

@maximveksler
Copy link
Contributor

Sounds like a good idea, who tested the branch and can confirm no issues?

@pankkor
Copy link

pankkor commented Mar 8, 2016

I've merged it into my fork and used it on production. No issues spotted so far

@zvonicek
Copy link

I'm testing this PR too and it works nicely.

@mamouneyya
Copy link

mamouneyya commented May 4, 2016

@pronebird Honestly, I am against the relative dimension idea, as the 50% would be translated to very large numbers after many scrolls (10,000+ pt?) in a very long scroll view implementation (e.g. Infinite news feed). The point-based fixed distance sounds more logical to me.

@pronebird
Copy link
Owner

pronebird commented Jul 3, 2016

I had to manually merge a part of it due to lots of changes since the PR. This is now a part of 0.9.1 and available on CocoaPods. Thanks everyone for your input and testing, @GorkaMM for implementation!

@pronebird pronebird closed this Jul 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants