Instagram / IGListKit

A data-driven UICollectionView framework for building fast and flexible lists.
https://instagram.github.io/IGListKit/
MIT License
12.87k stars 1.54k forks source link

Allow subclassing of IGListCollectionView? Remove IGLK_SUBCLASSING_RESTRICTED? #240

Closed lucabartoletti closed 7 years ago

lucabartoletti commented 7 years ago

In my project I have a CCTransparentTableView

It's a UITableView subclass that takes care to make the topMargin transparent to show what is behind. I'm trying to substitute one of the CCTransparentTableView with a IGListCollectionView. The problem is that I'd like to reimplement the transparency logic in a CCTransparentCollectionView : IGListCollectionView and use it as IGListCollectionView. But IGLK_SUBCLASSING_RESTRICTED is blocking that.

Is IGLK_SUBCLASSING_RESTRICTED really necessary?

jessesquires commented 7 years ago

Hey @lucabartoletti 😄

In general, we've taken pretty firm stances against subclassing in the Insta codebase, which is why this macro exists.

  1. Can you elaborate on what CCTransparentCollectionView does? Could this behavior be achieved without subclassing, (or by providing a custom UICollectionViewLayout)?
  2. I'll leave this open as a proposal. However, this is the first request we've gotten for this. If more people ask for this, we'll consider lifting the restrictions — but I don't think subclassing UICollectionView is very common.
lucabartoletti commented 7 years ago

Thanks for the reply @jessesquires

On the long term I plan to implement a custom UICollectionViewLayout to achieve the same effect. My need for subclassing was due to the fact that I was substituting a UITableView subclass with a UICollectionView that uses IGListKit, I don't have time to spent on implementing a custom layout and the fastest thing was to adopt the same code that we have in the UITableView subclass on a UICollectionView subclass.

I understand your point about IGLK_SUBCLASSING_RESTRICTED and your internal policy against subclassing.

rnystrom commented 7 years ago

@lucabartoletti Feel free to also fork the project and remove the subclassing restriction if that's a requirement.

Got any examples of the design you're trying to achieve with this? We're happy to help guide custom layout stuff 😃

jessesquires commented 7 years ago

Going to close this for now, since we're passing on changing this at the moment.

Will re-open if there's more interest.

jessesquires commented 7 years ago

Note, we're discussing this again in #409