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

Diffing magic not working, Perform Updates reloads everything #632

Closed otymartin closed 7 years ago

otymartin commented 7 years ago

@rnystrom

Not sure what I'm doing wrong but I'm not seeing much diffing magic yet😓

giphy

I have an immutable data model

var peopleSection = PeopleSection()
...
class PeopleSection {

    let people: [Person]

    init(_ people: [Person]) {
        self.people = people
    }
}

My datamodel conforms to IGListDiffable protocol

extension PeopleSection: IGListDiffable {

    func diffIdentifier() -> NSObjectProtocol {
        return DiffingIdentifier.people //people is an NSString
    }

    func isEqual(toDiffableObject object: IGListDiffable?) -> Bool {
        guard let object = object as? PeopleSection else { return false }
        if self === object { return true }
        return self.people == object.people
    }
}

So do the "Persons" in my "people" array

extension Person: IGListDiffable {

   func diffIdentifier() -> NSObjectProtocol {
        return self.objectId as NSObjectProtocol
    }

   func isEqual(toDiffableObject object: IGListDiffable?) -> Bool {
        guard let object = object as? Person else { return false }
        return self.objectId == object.objectId
    }
}

extension Person: Equatable {

    public static func ==(lhs: Person, rhs: Person) -> Bool {
        return lhs.objectId == rhs.objectId
    }
}

I query for "New People" from the database and return a new instance of my dataModel

extension PeopleSection {
    func add(_ newPeople: [Person]) -> PeopleSection {
        return PeopleSection(self.people + newPeople) 
       //Returns a new instance of my PeopleSection with new "Persons" appended to "People Array"
    }
}

I proceed to handle business in my IGListAdapterDataSource

extension PeopleViewController: IGListAdapterDataSource {

    func objects(for listAdapter: IGListAdapter) -> [IGListDiffable] {

        var objects = [self.peopleSection] as [IGListDiffable]

        if self.activity == .start {
            objects.append(self.loading as IGListDiffable)
        }
        return objects
    }

    func listAdapter(_ listAdapter: IGListAdapter, sectionControllerFor object: Any) -> IGListSectionController {
        if object is PeopleSection {
            return PeopleSectionController()
        }
    }

As well as in my SectionController

class PeopleSectionController: IGListSectionController, IGListSectionType {

    var peopleSection: PeopleSection?
    ...
    func numberOfItems() -> Int {
        return peopleSection?.people.count ?? 0
    }
    ...
    func cellForItem(at index: Int) -> UICollectionViewCell {}
    ...
    func didUpdate(to object: Any) {
        self.peopleSection = object as? PeopleSection
    }
}

EXPECTED MAGIC NOTE: My cells have a background user Photo + Name Label on them.

WHAT HAPPENS INSTEAD Note - In the example GIF above, Im loading 3 people each time user pulls to refresh

Help😓 Update: Thinking out loud -- Could it have to do with me returning peopleSection.people.count in numberOfItems which sees every updated new instance of my dataArray as different causing the whole thing to reload?? Just thinking out loud...😓

Sherlouk commented 7 years ago

If you could provide an example project it might be easier to diagnose the issue.

From my understanding though, each cell is represented by a Person. On pull to refresh you're adding three more Person to the single PersonSection that you have. By virtue of doing that it's now deemed a different object by isEqual(toDiffableObject) and therefore refreshes the entire view.

Your layout choice here means you want to have cells next to each other so having a section per item (which is recommended) is not going to work unless you have a custom layout.

otymartin commented 7 years ago

@Sherlouk I had a hunch that may be the issue. totally makes sense. @zhubofei I previously tried to use your IGListGridCollectionViewLayout but could never get a perfect grid out of it. Any developments since?

otymartin commented 7 years ago

@Sherlouk

I finally managed to achieve the behaviour I wanted thanks to your analysis. In this example I return one "Person" every time user pulls to refresh. The loading indicator number varies by design.

giphy

My dataModel is no longer immutable


var peopleSection = People()
...
class People {

    var people: [Person]
    weak var delegate: PeopleViewController?

    init(_ people: [Person]) {
        self.people = people
    }

    convenience init() {
        self.init([Person]())
    }

}

I now append new results to the same instance of my "People" array

private func add(_ people: [Person]) {
        self.people.append(contentsOf: self.sorted(people))
        self.delegate?.peopleActivity(.stop)
    }

In my list adapter

 func objects(for listAdapter: IGListAdapter) -> [IGListDiffable] {

        var objects = self.peopleSection.people as [IGListDiffable].
...

For my collectionViewLayout I use IGListCollectionViewLayout I think IGListGridFlowLayout was deprecated in exchange for this.

In my sectionController I do all this... NOTE: My collectionView leading and trailing margins are offset by 4 points which I did in the storyboard using autolayout. I found it a tough balancing act to get the grid to form by setting the left and right insets using UIEdgeInsets.

class PeopleSectionController: IGListSectionController, IGListSectionType {

    var person: Person?

    override init() {
        super.init()
        self.minimumInteritemSpacing = 4
        self.inset = UIEdgeInsets(top: 0, left: 0, bottom: 4, right: 0)
    }

    func numberOfItems() -> Int {
        return 1
    }

    func sizeForItem(at index: Int) -> CGSize {
        return self.itemSize
    }
...
   fileprivate var itemSize: CGSize {
        let collectionViewWidth = collectionContext?.containerSize.width ?? 0
        let itemWidth = ((collectionViewWidth - 8) / 3)
        let heightRatio: CGFloat = 1.5
        return CGSize(width: itemWidth, height: itemWidth * heightRatio)
    }
Sherlouk commented 7 years ago

Would it be possible to upload an example project to GitHub and I'll have a more detailed look. With the code posted I can't see how you're doing the pull-to-refresh behaviour!

otymartin commented 7 years ago

@Sherlouk Hey, my problem is fixed now. unfortunately I can't upload an example project but for the pull-to-refresh behaviour I followed the "Load More" example.

Its literally the same code except, I return a random number of activity cells based on how many "Person" cells there are on the last row. I then interpolate the activity cell's contentView background color from a randomColor to anotherRandom Color.

extension PeopleViewController: IGListAdapterDataSource {

    func objects(for listAdapter: IGListAdapter) -> [IGListDiffable] {

        var objects = self.peopleSection.people as [IGListDiffable]

        if self.activity == .start {
            objects.append(self.loading as IGListDiffable)
//LOADING IS A STRING "loading"
        }
        return objects
    }

    func listAdapter(_ listAdapter: IGListAdapter, sectionControllerFor object: Any) -> IGListSectionController {
        if let object = object as? String, object == self.loading {
            return ActivitySectionController()
        }
        return PeopleSectionController()
    }
....
extension PeopleViewController: UIScrollViewDelegate {

    func scrollViewWillEndDragging(_ scrollView: UIScrollView, withVelocity velocity: CGPoint, targetContentOffset: UnsafeMutablePointer<CGPoint>) {

        if scrollView.panGestureRecognizer.translation(in: scrollView).y > 0 {
            return
        }

        let distance = scrollView.contentSize.height - (targetContentOffset.pointee.y + scrollView.bounds.height)
        if self.activity == .stop && distance < 200 {
            guard let currentUser = self.currentUser  else { return }
            self.peopleSection.getPeople(near: currentUser)
        }
    }
}
....
class ActivitySectionController: IGListSectionController, IGListSectionType {
...
    func numberOfItems() -> Int {
        return self.activityIndicators
    }

    func cellForItem(at index: Int) -> UICollectionViewCell {

        let cell = self.collectionContext?.dequeueReusableCellFromStoryboard(withIdentifier: Cell.activityCell, for: self, at: index) as! ActivityCell
        if let delegate = self.viewController as? PeopleViewController {
            cell.configure(delegate)
        }
        cell.start() //START INTERPOLATING BACKGROUND COLORS

        return cell
    }

    func sizeForItem(at index: Int) -> CGSize {
        let width = collectionContext?.containerSize.width ?? 0
        let itemSize = floor((width - 8) / 3)
        return CGSize(width: itemSize, height: itemSize * 1.5)
    }

}

extension ActivitySectionController {
    //THIS DETERMINES HOW MANY ACTIVITY CELLS TO RETURN BASED ON EMPTY CELLS IN LAST ROW
    fileprivate var activityIndicators: Int {

        var peopleCount: Double?

        guard let peopleViewController = self.viewController as? PeopleViewController  else { return 0 }

        peopleCount = peopleViewController.peopleSection.people.count.toDouble

        if peopleViewController.activity == .stop {
            return 0
        }

        guard let count = peopleCount else { return 3 }

        let numberOfRows = ceil(count / 3)
        let emptyCellsInRow = (numberOfRows * 3) - count

        if emptyCellsInRow == 1 {
            return 4
        } else if emptyCellsInRow == 2 {
            return 2
        }
        return 3
    }
}
Sherlouk commented 7 years ago

@otymartin Closing the issue since it's fixed, if you have any more questions then feel free to reopen or submit a new issue!