pronebird / UIScrollView-InfiniteScroll

UIScrollView ∞ scroll category
MIT License
1.06k stars 148 forks source link

Trigger offset added #7

Closed GorkaMM closed 8 years ago

GorkaMM commented 9 years ago

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 commented 9 years ago

If you merge this, please release a new version.

Thank you!

pronebird commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

I addressed this improvement in the following commits:

  1. Added custom action offset handler: commit b008db705878adb5728830c91e59efcf2416f0fe
  2. Added demo handler: commit 97a50356bae165344e737d8306382e069074843c

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 commented 8 years ago

When this feature will be released?

pankkor commented 8 years ago

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

zvonicek commented 8 years ago

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

maximveksler commented 8 years ago

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

pankkor commented 8 years ago

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

zvonicek commented 8 years ago

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

mamouneyya commented 8 years ago

@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 commented 8 years ago

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!