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

RFC: Implement UITableView Support #584

Closed
amonshiz opened this issue Mar 24, 2017 · 12 comments
Closed

RFC: Implement UITableView Support #584

amonshiz opened this issue Mar 24, 2017 · 12 comments

Comments

@amonshiz
Copy link
Contributor

Purpose

I would like to add support for UITableView within IGListKit. Ever since @rynstrom started formulating what would become IGListKit I thought it would be great if it worked with UITableView, but I didn't have the time to do that. Additionally, Instagram doesn't have many table views left so the impact would have been small. However, I still believe that there is meaningful value in supporting UITableView, and for Instagram it would be great to convert the remaining UITableViews over to this more sane framework.

This issue is to track the following:

  • desired additions
  • analysis of required changes to IGListKit

As this will likely be 100% source breaking changes, I'm guessing this will be included in at earliest the 4.0.0 release.

Expected Changes

Below are my (likely) naive expectation of changes that will happen:

IGListAdapter

  • collectionView, collectionViewDelegate, and updater will no longer be direct properties on the object

IGListAdapter is relatively weakly coupled to UICollectionView. There are a number of collection view specific data source, delegate, and layout methods but none of them are critical to the adapter itself. I believe all of those could move out to an owned object that is specific to collection view without breaking the adapter.

IGListUpdatingDelegate

This protocol is also not 100% dependent on collection view but rather what a collection view can do for adding, deleting, and moving parts. Thankfully all of that is possible in UITableView also. Except for the completion block that is available with -[UICollectionView performBatchUpdates:completion:], but I feel confident that it will be possible to add that exact method via an extension to UITableView and effectively change this protocol into one that depends only on an object that implements some other specific protocol.

Add IGListUpdatePerformable Protocol

I'm good at naming. I based this on the Swift API Design Guideline

Protocols that describe a capability should be named using the suffixes able,ible, or ing

This protocol will be used by IGListUpdatingDelegate to encapsulate an object's ability to handle adds, deletes, moves, and reloads that IGListKit support. By abstracting this out it should be possible to allow objects that aren't even UIView subclasses be managed by IGListKit.

Add IGListEncapsulating Protocol

Names. This protocol would be for objects that actually own the object that is being managed by IGListKit and should implement any specific data source and delegate methods that should be intercepted/implemented. This object type would be what is owned by IGListAdapter and could become the actual collectionContext that is provided to IGListSectionController instances. That last part may get a bit weird, but maybe.

The overall goal is to make IGListAdapter independent of UICollectionView and provide some method for it to handle any kind of list-ing thing. The largest downside is that at some point a consumer is going to have to do type specific casts to go from a protocol to a specific list-ing type. Thus if there are implementations for UICollectionView and UITableView then at some point there will be a need to know which type is being used and then cast to that type. I don't know exactly where that is yet, but I'm sure it will shake out on its own and there will be great feedback.

Break UICollectionView Dependency in IGListSectionController

This one is potentially going to be difficult. The section controller needs the collection context and then has various delegate and data source objects that are specific to collection view functionality. The two that are most apparent are IGListSupplementaryViewSource and IGListDisplayDelegate. Both of these maybe could be easily converted but I'm not sure yet.

@jessesquires
Copy link
Contributor

This would be trivial with Swift, because we could unify table and collection views like this:
https://github.com/jessesquires/JSQDataSourcesKit/blob/develop/Source/ViewFactory.swift

We could probably get close to this in Objective-C, but there are a few things that will be much more cumbersome.

@robertjpayne
Copy link

I'd be hesitant to implement this given that UITableView may be on the chopping block, iOS 10 already includes a lot of private headers that indicate UICollectionView is going to get some API's to help it do everything UITableView can already do.

https://pspdfkit.com/blog/2017/the-case-for-deprecating-uitableview/

@amonshiz amonshiz self-assigned this Mar 28, 2017
@Adlai-Holler
Copy link
Contributor

I also would avoid implementing this. UICollectionView and UITableView have subtle differences of behavior that will lead to a ton more branching code than you might expect on the surface.

Most modern apps only have a couple of table views these days, and it's not too hard to implement diffing or a section controller architecture without IGListKit managing the updates etc.

@jessesquires
Copy link
Contributor

@vincent-peng
Copy link

vincent-peng commented Apr 12, 2017

I agree with @robertjpayne , may best to wait til WWDC and see what will be happening.

@ay8s
Copy link

ay8s commented Apr 13, 2017

I would also wait till after WWDC. We're actually making use of ASCollectionNode for our "tables" now without any additional logic needed and plan to use that throughout the app. Probably spoilt by the ease of ASDK and how it does layout.

@jessesquires
Copy link
Contributor

The science is in.

@amonshiz
Copy link
Contributor Author

Clearly all those "don't use" are just future users who gave up when it didn't support table view. ;)

@jessesquires jessesquires added this to the 4.0.0 milestone Apr 14, 2017
@jessesquires
Copy link
Contributor

jessesquires commented Apr 14, 2017

Ok, looks like there's enough interest and some good reasons to do this. Here's my suggestion for implementing this and rolling it out:

Implementation/release plan:

  • Release with 4.0 (potentially as the only major work for 4.0). Created the milestone for this.
  • We'll do all work on a dedicated branch from master, release-4.0-tableview-support, except for foundational refactoring work
  • If possible, let's identify any "rearrange the deck chairs" refactoring work and try to do this for 3.0 to minimize major-release churn
  • We can submit all PRs to this branch and merge directly on GH
  • When ready, we can PR this branch for master and merge into master
  • Then, merge to stable, tag release on GH, push to cocoapods

Thoughts?

EDIT: also, release-4.0-tableview-support will be constantly rebased on master to stay updated.

@amonshiz
Copy link
Contributor Author

That sounds great! I can put together an issue with the pieces of work as I see them and we can discuss what constitutes deck chair vs 4.0 release. Or is there a better way to document and discuss the breakdown of work?

@jessesquires
Copy link
Contributor

@amonshiz - sounds good to me! 👍

@Ziewvater
Copy link

Given the response in 692 I'm closing this out. There's no demand for implementing UITableView support internally, so developing this and supporting it would be a no-go for us

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants