facebookarchive / AsyncDisplayKit

Smooth asynchronous user interfaces for iOS apps.
http://asyncdisplaykit.org
Other
13.4k stars 2.2k forks source link

Allow Setting ASCollectionView.delegate for Interop with TLYShyNavBar etc. #2245

Closed simoami closed 7 years ago

simoami commented 8 years ago

There are several 3rd party components that require access to scrollview delegates directly or indirectly (you just pass a reference to the scrollview). Categories include "Pull to Refresh" and "Hide Navbar on scroll down" components, such as: TLYShyNavBar, BLKFlexibleHeightBar, AMScrollingNavbar...etc

Currently we get the following error:

Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'ASCollectionView uses asyncDelegate, not UICollectionView's delegate property.'

Issues opened because of this limitation: https://github.com/telly/TLYShyNavBar/issues/72 https://github.com/telly/TLYShyNavBar/issues/158

Furthermore, It's not feasible for these 3rd party components to support asyncDelegate.

One suggestion is to support switching between asyncDelegate and delegate by passing a configuration, or by optionally chaining the two.

Chat log with @maicki:

simo [12:25 PM]  
asyncDelegate vs delegate
perhaps something that could be addressed in ASDK 2
so currently I get the following error "ASCollectionView uses asyncDelegate, not UICollectionView's delegate property."
It’s not because I’m explicitly accessing .delegate. It’s because I’m binding the ASCollectionNode scroll view to a plugin that does it under the hood `shyNavBarManager.scrollView = collectionView.view`

I’m using this: ` var collectionView: ASCollectionNode `

maicki [12:30 PM]  
You could subclass `ASCollectionView` and handle connect `asyncDelegate <-> delegate`
we had something similar in the past, just didn’t remember exactly how we solved it

simo [12:32 PM]  
so my thought was, why not continue to support  access to .delegate. We know there are tons of 3rd party components that can’t support ASDK for this. Encourage asyncDelegate, but allow access to delegate

maicki [12:33 PM]  
Currently the reason we don’t allow it is that we have a proxy between `delegate <-> asyncDelegate` (edited)

simo [12:33 PM]  
ah ok. It seems like if a 3rd party component wanted to support asyncDelegate, they would have a dependency on ASDK which is not recommended for standalone components, as it could create version mismatch

maicki [12:35 PM]  
As said I think you could actually support it if you plugin a proxy between `setDelegate -> asyncDelegate`. Basically it would be `collectionView.delegate -> proxy -> collectionView.asyncDelegate -> proxy -> super.delegate`. You could archive that I guess by overwritting `setDelegate:` in your `ASCollectionView` subclass

Does that makes sense?

simo [12:36 PM]  
what does `proxy` represent?

maicki [12:37 PM]  
Look into how `NSProxy` is working

[12:37]  
It basically passes through all of the methods to the target (edited)

[12:38]  
We use it as we have to intercept certain methods that get’s called from the `UICollectionView` (edited)

simo [12:39 PM]  
yeah, I’m not there yet knowledge-wise but I’ll look. perhaps I’ll find something of interest in ASDK code base

maicki [12:39 PM]  
Look into `ASDelegateProxy` and `ASCollectionViewProxy` (edited)

[12:39]  
There you can see which methods we intercept

[12:40]  
Just for further undestanding we intercept, e.g. `scrollViewWillEndDragging:withVelocity:targetContentOffset:` to support batch fetching (edited)

simo [12:41 PM]  
I see. does subclassing not allow you to override the behavior? not the case with delegates?

maicki [12:42 PM]  
No as we want to intercept certain `delegate` / `dataSource`, subclassing will not help in this case (edited)

[12:43]  
@simo If you would like to see a change for that behavior I encourage you to open a Github Issue. (edited)

simo [12:44 PM]  
Yeah, it would really be nice for you guys to think of ways to support it, perhaps through configuration.

maicki [12:44 PM]  
As said we are open for discussion, just open a Github Issue with some examples or even better some ideas around how we could support it
Adlai-Holler commented 7 years ago

Unfortunately we need to revert this because I broke IGListKit.

I'm marking this as wontfix since it's very old, and it'll be hard to provide support for frameworks like TLYShyNavBar safely, especially without buy-in from them.