IGListBindingSectionController no longer asserts when reloading the entire section. #1213
Conversation
…’s backgroundColor property
… to each test method.
… numberOrItems before didUpdate is called
# Conflicts: # Source/Internal/IGListStackedSectionControllerInternal.h
Generated by 🚫 Danger |
jeffbailey
changed the title
|
To test the change, I updated IGTestDiffingObject::isEqualToDiffableObject to not always return YES. Now it does a simple compare for the number of objects associated with the key. That actually triggered the assert that was there previously when running IGListBindingSectionControllerTests::test_whenAdapterReloadsObjects_thatSectionUpdated. Now that I converted the assert to a warning, the test passes again. |
CHANGELOG.md
Outdated
| @@ -12,6 +12,8 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag | |||
|
|
|||
| - Added `IGListCollectionScrollingTraits` for exposing `UICollectionView` scrolling traits to section controllers via `IGListCollectionContext`. [Adam Stern](https://github.com/adamastern) (tbd) | |||
|
|
|||
| - Support reloading a `IGListBindingSectionController`. [Jeff Bailey](https://github.com/jeffbailey) (tbd) | |||
There was a problem hiding this comment.
2 nits:
- Replace tbd with the number and link to this PR (1213)
- Let's make the line a little more descriptive. Something like "no longer asserts when reloading..." etc etc.
| return self.objects.count == testDiffingObject.objects.count; | ||
| } | ||
|
|
||
| return NO; |
There was a problem hiding this comment.
Noting to myself that this actually tests not tripping the assert anymore. See comment
There was a problem hiding this comment.
Good idea. Pull request has been updated @rnystrom.
jeffbailey
changed the title
jeffbailey
changed the title
There was a problem hiding this comment.
@jeffbailey what's up with the comment about the stacked controller? Looks like commit history got kind of wonky? (that's fine if so, we squash everything into a single commit)
Also heads-up we're still planning on deprecating that feature!
|
Yeah, I'm not sure why the stacked controller change showed up there. Just squash to a single commit. |
There was a problem hiding this comment.
@rnystrom is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
…ntire section. (Instagram#1213) Summary: Issue fixed: Instagram#1174 - [X] All tests pass. Demo project builds and runs. - [x] I added tests, an experiment, or detailed why my change isn't tested. - [X] 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) Pull Request resolved: Instagram#1213 Differential Revision: D9003398 Pulled By: rnystrom fbshipit-source-id: 2c68f42e21abaea9191f26309668d866544f80b4
|
When will be this released? |
|
@shial4 We made 4.0.0 release yesterday, enjoy! |
Changes in this pull request
Issue fixed: #1174
Checklist
CHANGELOG.mdfor any breaking changes, enhancements, or bug fixes.