pitiphong-p / CollectionViewShelfLayout

A UICollectionViewLayout subclass displays its items as rows of items similar to the App Store Feature tab without a nested UITableView/UICollectionView hack.
MIT License
376 stars 30 forks source link

Great job! #2

Closed heumn closed 6 years ago

heumn commented 8 years ago

I have a similar problem in my current project and I have been researching how to best achieve this type of layout without using the typical "UICollection/UITableView inside a UITableView/UICollectionVIewCell. I will much rather prefer a solution like you have engineered rather than the hack mentioned above 🌟 🎉 Will do some testing and probably be inspired by this solution. So thanks. That is all :)

pitiphong-p commented 8 years ago

The prepareForReuse method will be called when the cell is going to display. So your first issue is invalid, right?

heumn commented 8 years ago

I understood my first issue was related to how Apple use the re-use pool differently in iOS 10. Not related to your implementation (as far as I can see). If i wanted the old style of re-using cells you can always turn isPrefetchingEnabled off (as it it on by default) :)

The prepareForReuse method will be called when the cell is going to display. Not if you keep isPrefetchingEnabled = true and scroll to the right. It will re-create new cells for every "itemAtIndexPath".

You can still experience what I described by increasing the section contents to 50+ apps in the demo app and add a print to prepareForReuse ("re-used a cell" ?). This will not be called when scrolling to the right. But if you scroll backwards again (to the left) you will see it starts re-using cells. Some weird optimalization is going on I would say from UIKit because if why would you keep 50+ cells around?

Regardless, if you want "the old behaviour" (not prefetching and keeping cells around for such an extended period you can just set isPrefetchingEnabled to false

pitiphong-p commented 8 years ago

What I want to know that if we enable isPrefetchingEnabled then the collection would re-uses its cells or not even though it doesn't call the prepareForReuse method on the cell (Still do the re-using cell but just not calling the prepareForReuse method) Could you check that for me please?

heumn commented 8 years ago

Of course I can :)

isPrefetchingEnabled is enabled by default by the way

It does not re-use its cells. Here is the simple test I did. Set cell total to 250 cells in plist

skjermbilde 2016-10-28 kl 10 53 07

logged each creation / deletion as well as reuse

skjermbilde 2016-10-28 kl 10 53 52

and ran the app and scrolled all sections to the end

skjermbilde 2016-10-28 kl 10 51 45

also note the memory grow (not that this implies anything. this may be how apple intends prefetching and caching to work and since cells are so low in size they will discard them on demand).

skjermbilde 2016-10-28 kl 10 52 08 skjermbilde 2016-10-28 kl 10 51 56
pitiphong-p commented 8 years ago

Ummm, Interesting. I will take a look at this on this weekend. Thank you for pointing me this 😃

heumn commented 8 years ago

Thank you for sharing this :) I will let you know if I discover anything!

pitiphong-p commented 8 years ago

The prepareForReuse method will be called when the cell is going to display. Not if you keep isPrefetchingEnabled = true and scroll to the right. It will re-create new cells for every "itemAtIndexPath".

Where did you get this information?

heumn commented 8 years ago

This one is a great summary

https://littlebitesofcocoa.com/241-uicollectionview-cell-pre-fetching

If you turn set isPrefetchingEnabled to false you will have the "old" behavior where prepareForReuse will be called when a re-used cell is about to be reused about immediately after it scrolls off screen.

You can test it in your demo app as well with a print on prepareForReuse and setting isPrefetchingEnabled to false (thus get the pre-ios10 "vanilla" behaviour)

The rest of the information I got from testing :)

pitiphong-p commented 7 years ago

I think this is a bug in UIKit framework. They may assume that every cells should be scroll with the same direction as their collection view layout's scroll direction. I filed a radar for a reference.

heumn commented 7 years ago

Thanks for the update :) I found it really interesting to read your implementation

For reference I actually went with a UITableVIew with UICollectionViews with "proxy" delegates for each cell that forwards the delegate calls. I just found it easier to separate the different layers of UI logic into smaller components.

pitiphong-p commented 7 years ago

👍 My only requirement for this project is that trying to make this kind of UI working with the concept of UICollectionView's Data Source properly. With this Layout subclass, you only handle the collection view data source in a single place but with the embedded approach, I think you have to manage in other manner

heumn commented 7 years ago

There is a hack in my implementation and that is that the intermediate proxy has to know what section it is presenting (leaky abstraction), thus the underlying requests for "itemAtIndexPath" / "cellForRow" has to be forwarded after being mutated to reflect actual position in the datasource.

So the proxy for section 3 will get a request for item with section 0 and index 3, meanwhile it will forward it to reflect its section, say section 3, index 3. Not ideal but it works :)

pitiphong-p commented 7 years ago

👍 Yeah. It depends on the design decision that you made. Nothing is wrong :)