Iglistkit: IGListStackedSectionController's children need to know numberOrItems before didUpdate is called

Created on 21 Dec 2016  路  4Comments  路  Source: Instagram/IGListKit

New issue checklist

General information

  • IGListKit version:
  • iOS version(s): 10.2
  • CocoaPods/Carthage version: master
  • Xcode version: 8.2
  • Devices/Simulators affected:
  • Reproducible in the demo project? (Yes/No): NO
  • Related issues:

IGListStackedSectionController is incredibly useful for decomposing section controllers, but I'm running into an issue that appears to be a bug when one of the child section controllers has a variable number of cells, based on the object injected into it.

A child section controller's
func numberOfItems() -> Int
method is getting called before
func didUpdate(to object: Any)
That results in the section controller having to know how many items it contains BEFORE it has its data.

I tracked the issue down to the stacked section controller constructor calling it's reloadData method:

- (instancetype)initWithSectionControllers:(NSArray <IGListSectionController<IGListSectionType> *> *)sectionControllers {
    if (self = [super init]) {
        for (IGListSectionController<IGListSectionType> *sectionController in sectionControllers) {
            sectionController.collectionContext = self;
            sectionController.viewController = self.viewController;
        }

        _visibleSectionControllers = [[NSCountedSet alloc] init];
        _sectionControllers = [NSOrderedSet orderedSetWithArray:sectionControllers];

        self.displayDelegate = self;
        self.scrollDelegate = self;

        [self reloadData];
    }
    return self;
}

The reloadData method is asking its children for the number of items, but the children don't have their data injected yet.

I can easily workaround the issue by explicitly setting the data (i.e. view model in my case) on the child section controller when I build up stacked section controller, but that seems like a hack unless I'm missing something.

Let me know what you think. If you agree its a bug, I'd be happy to create a unit test and fix the issue, but I probably need a little guidance as to the appropriate fix.

bug

Most helpful comment

@rnystrom You're guidance on the fix was spot on. I submitted a pull request. The main thing to review is how I updated the unit tests to trigger the reloadData method before performing some of the tests. I called perform batch updates. Let me know if there's a better way!

All 4 comments

@jeffbailey doing some cleanup, if you're still interested in tackling this, that'd be awesome! As far as guidance goes, my bet is that we:

What do you think?

I'll take a look!

@rnystrom You're guidance on the fix was spot on. I submitted a pull request. The main thing to review is how I updated the unit tests to trigger the reloadData method before performing some of the tests. I called perform batch updates. Let me know if there's a better way!

done in 8d74b8f778d7104df7e3cf0dd513ceae1a69d880

Was this page helpful?
0 / 5 - 0 ratings