Cell reordering in a ListBindingSectionController - problems with the data source

[SwiftedMind]

General information

• IGListKit version: master branch
• iOS version(s): 12.0
• CocoaPods/Carthage version: 1.5.3
• Xcode version: 10.0
• Devices/Simulators affected: Simulator and all devices
• Reproducible in the demo project? (Yes/No): Yes
• Related issues:

Hey,

I've seen the examples showcasing how to implement cell reordering using IGListKit. For the past 2 days, I've been trying to implement this, but inside a ListBindingSectionController. There is strange behavior that I don't understand.

I've managed to implement the reordering, kind of. It is working, but it has some problems:

1. When you move a cell to a new position, and then tap on another cell (which deletes the cell, see demo project) a wrong cell is being deleted. This only happens when you moved a cell before.
2. When you move a cell, then tap on another cell (which deletes the wrong cell) and then move another cell, the cell is re-rendered with the wrong content/viewModel.

I recorded this and uploaded a gif:

The strange thing is that the array containing the models for the cell is actually correct (even after moving or deleting).

I'm pretty sure, I've implemented the move functionality in a way that's causing these problems:

override func moveObject(from sourceIndex: Int, to destinationIndex: Int) {
        let removedModel = groupModel.models.remove(at: sourceIndex)
        groupModel.models.insert(removedModel, at: destinationIndex)
    }

I tried calling update(animated: false, completion: nil) (to update the internal viewModels array) after moving the model, but that leads to more strange behavior (cells are being re-rendered with the wrong viewModel, like 2. from above but without deleting a cell first)

I created a demo project showing this:
Link to demo project: https://github.com/d3mueller/ReorderExample

I don't really know what to do. I can't find my mistake. Maybe I don't quite understand how ListBindingSectionController works. I'd appreciate any help with this :)

Thanks and have a nice day,
Dennis

SectionController class from the demo project:

class SectionController: ListBindingSectionController<GroupModel>, ListBindingSectionControllerDataSource {
    
    // MARK: - Properties
    // ========== PROPERTIES ==========
    private var groupModel: GroupModel = GroupModel(models: [
        ItemModel(id: 1),
        ItemModel(id: 2),
        ItemModel(id: 3),
        ItemModel(id: 4),
        ItemModel(id: 5),
        ItemModel(id: 6),
        ItemModel(id: 7),
        ItemModel(id: 8),
        ItemModel(id: 9),
        ItemModel(id: 10),
        ])
    // ====================
    
    // MARK: - Initializers
    // ========== INITIALIZERS ==========
    override init() {
        super.init()
        minimumLineSpacing = 2
        minimumInteritemSpacing = 2
        dataSource = self
    }
    // ====================
    
    // MARK: - Overrides
    // ========== OVERRIDES ==========
    override func canMoveItem(at index: Int) -> Bool {
        return true
    }
    
    override func moveObject(from sourceIndex: Int, to destinationIndex: Int) {
        let removedModel = groupModel.models.remove(at: sourceIndex)
        groupModel.models.insert(removedModel, at: destinationIndex)
    }
    // ====================
    
    
    // MARK: - Functions
    // ========== FUNCTIONS ==========
    func sectionController(_ sectionController: ListBindingSectionController<ListDiffable>, viewModelsFor object: Any) -> [ListDiffable] {
        return groupModel.models
    }
    
    func sectionController(_ sectionController: ListBindingSectionController<ListDiffable>, sizeForViewModel viewModel: Any, at index: Int) -> CGSize {
        return CGSize(width: 50, height: 50)
    }
    
    func sectionController(_ sectionController: ListBindingSectionController<ListDiffable>, cellForViewModel viewModel: Any, at index: Int) -> UICollectionViewCell & ListBindable {
        if let cell = collectionContext?.dequeueReusableCell(of: ItemCell.self, for: self, at: index) as? ItemCell {
            
            cell.label.text = "\(groupModel.models[index].id)"
            return cell
        }
        
        fatalError()
    }
    
    override func didSelectItem(at index: Int) {
        groupModel.models.remove(at: index)
        update(animated: false, completion: nil)
    }
    
    override func didDeselectItem(at index: Int) {
        
    }
    // ====================

}

Debug information

IGListAdapter 0x60000107f260:
  Updater type: IGListAdapterUpdater
  Data source: <ReorderExample.ViewController: 0x7fa8a2d1e1d0>
  Collection view delegate: (null)
  Scroll view delegate: (null)
  Is in update block: No
  View controller: <ReorderExample.ViewController: 0x7fa8a2d1e1d0>
  Is prefetching enabled: No
  Registered cell identifiers:
  {(
    "ReorderExample.ItemCell"
)}
  IGListAdapterUpdater instance 0x60000007da40:
    Moves as deletes+inserts: No
    Allows background reloading: Yes
    Has queued reload data: No
    Queued update is animated: Yes
    State: Idle
  Section map details:
    IGListBindingSectionController 0x600001c74b00:
  Data source: <ReorderExample.SectionController: 0x600001c74b00>
  Selection delegate: (null)
  Object: test
  View models:
  ItemModel 3: 3
  ItemModel 4: 4
  ItemModel 5: 5
  ItemModel 6: 6
  ItemModel 7: 7
  ItemModel 9: 9
  Number of items: 6
  View controller: <ReorderExample.ViewController: 0x7fa8a2d1e1d0>
  Collection context: <IGListAdapter: 0x60000107f260>
  Section: 0
  Is first section: Yes
  Is last section: Yes
  Supplementary view source: (null)
  Display delegate: (null)
  Working range delegate: (null)
  Scroll delegate: (null)
  Collection view details:
    Class: UICollectionView, instance: 0x7fa8a3010000
    Data source: <IGListAdapter: 0x60000107f260>
    Delegate: <IGListAdapter: 0x60000107f260>
    Layout: <IGListCollectionViewLayout: 0x7fa8a2c07020>
    Frame: {{0, 0}, {375, 812}}, bounds: {{0, -44}, {375, 812}}
    Number of sections: 1
      6 items in section 0
    Visible cell details:
      Visible cell at section 0, item 0:
      <ReorderExample.ItemCell: 0x7fa8a2f1f590; baseClass = UICollectionViewCell; frame = (0 0; 50 50); layer = <CALayer: 0x600002b12160>>
      Visible cell at section 0, item 1:
      <ReorderExample.ItemCell: 0x7fa8a2f2b910; baseClass = UICollectionViewCell; frame = (52 0; 50 50); layer = <CALayer: 0x600002b123c0>>
      Visible cell at section 0, item 2:
      <ReorderExample.ItemCell: 0x7fa8a2f2caa0; baseClass = UICollectionViewCell; frame = (104 0; 50 50); layer = <CALayer: 0x600002b125e0>>
      Visible cell at section 0, item 3:
      <ReorderExample.ItemCell: 0x7fa8a2f2f3b0; baseClass = UICollectionViewCell; frame = (156 0; 50 50); layer = <CALayer: 0x600002b12840>>
      Visible cell at section 0, item 4:
      <ReorderExample.ItemCell: 0x7fa8a2f312a0; baseClass = UICollectionViewCell; frame = (208 0; 50 50); layer = <CALayer: 0x600002b12aa0>>
      Visible cell at section 0, item 5:
      <ReorderExample.ItemCell: 0x7fa8a2f0e2a0; baseClass = UICollectionViewCell; frame = (260 0; 50 50); layer = <CALayer: 0x600002b12d00>>

[rnystrom]

It definitely seems like the binding SC is missing a call to rearrange its view models when moving happens. I found that adding this to the binding SC:

- (void)moveObjectFromIndex:(NSInteger)sourceIndex toIndex:(NSInteger)destinationIndex {
    NSMutableArray *viewModels = [self.viewModels mutableCopy];
    [viewModels exchangeObjectAtIndex:sourceIndex withObjectAtIndex:destinationIndex];
    self.viewModels = viewModels;
}

And then changing your implementation to:

override func moveObject(from sourceIndex: Int, to destinationIndex: Int) {
    super.moveObject(from: sourceIndex, to: destinationIndex)
    groupModel.models.swapAt(sourceIndex, destinationIndex)
}

Fixes the issue. Using swapAt(from:, to:) was important as the arrays could get out of sync.

@d3mueller are you interested in submitting a fix for this? Would be an awesome contribution!

Also thanks a ton for sending us an example!

[SwiftedMind]

Thanks! That method looks like the solution. But shouldn't the objects in the array be moved instead of swapped? After swapping, the viewModels array doesn't represent the order of the visible cells anymore, which causes problems after moving a cell a second time.

This seems to be working:

- (void)moveObjectFromIndex:(NSInteger)sourceIndex toIndex:(NSInteger)destinationIndex {
    NSMutableArray *viewModels = [self.viewModels mutableCopy];
    
    id removedModel = [viewModels objectAtIndex:sourceIndex];
    [viewModels removeObjectAtIndex:sourceIndex];
    [viewModels insertObject:removedModel atIndex:destinationIndex];
    
    self.viewModels = viewModels;
}

override func moveObject(from sourceIndex: Int, to destinationIndex: Int) {
    super.moveObject(from: sourceIndex, to: destinationIndex)
    let object = groupModel.models.remove(at: sourceIndex)
    groupModel.models.insert(object, at: destinationIndex)
}

Yes, I'd love to try and contribute 😀.

[rnystrom]

Oh duh ya, shouldn’t be swapped. But yes this would be a really great contribution! We would want to make sure binding SC subclasses are required to call super too. That should be doable with annotations.

_{Sent with GitHawk}