-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
IGListStackedSectionController's children need to know numberOrItems before didUpdate is called #348
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! |
… numberOrItems before didUpdate is called Summary: The reloadData method in IGListStackedSectionController was being called too soon. It was moved from the constructor to the `didUpdateToObject` method. Tests were updated to reflect the change. - [x] All tests pass. Demo project builds and runs. - [x] I added tests, an experiment, or detailed why my change isn't tested. - [ ] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes. - [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md) Closes #390 Differential Revision: D4419751 Pulled By: rnystrom fbshipit-source-id: 7c812d306b23dd251c160425873930eb8022b1a5
… numberOrItems before didUpdate is called Summary: The reloadData method in IGListStackedSectionController was being called too soon. It was moved from the constructor to the `didUpdateToObject` method. Tests were updated to reflect the change. - [x] All tests pass. Demo project builds and runs. - [x] I added tests, an experiment, or detailed why my change isn't tested. - [ ] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes. - [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md) Closes #390 Differential Revision: D4419751 Pulled By: rnystrom fbshipit-source-id: 7c812d306b23dd251c160425873930eb8022b1a5
done in 8d74b8f |
…d to know numberOrItems before didUpdate is called Summary: The reloadData method in IGListStackedSectionController was being called too soon. It was moved from the constructor to the `didUpdateToObject` method. Tests were updated to reflect the change. - [x] All tests pass. Demo project builds and runs. - [x] I added tests, an experiment, or detailed why my change isn't tested. - [ ] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes. - [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md) Closes Instagram#390 Differential Revision: D4419751 Pulled By: rnystrom fbshipit-source-id: 7c812d306b23dd251c160425873930eb8022b1a5 # Conflicts: # CHANGELOG.md
New issue checklist
README
and documentationGeneral information
IGListKit
version: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:
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.
The text was updated successfully, but these errors were encountered: