From 4e9b0bb4c43301fcc969c1051393f54d518c2d90 Mon Sep 17 00:00:00 2001 From: Ryan Nystrom Date: Wed, 29 Mar 2017 14:58:43 -0700 Subject: [PATCH] Prevent negative spaces for items and supplementaries Summary: If a negative height/width comes down, cap it to `0.0`. Issue fixed: #566 Also fixes t16455632 internally. - [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. Closes https://github.com/Instagram/IGListKit/pull/583 Differential Revision: D4797520 Pulled By: jessesquires fbshipit-source-id: 3eeec5244a445bb451460286b4f006ca0d9c67d1 --- CHANGELOG.md | 2 ++ Source/IGListAdapter.m | 6 ++-- Tests/IGListAdapterTests.m | 28 +++++++++++++++++++ Tests/Objects/IGListTestSection.h | 2 ++ Tests/Objects/IGListTestSection.m | 9 +++++- .../IGTestStoryboardSupplementarySource.m | 6 ---- Tests/Objects/IGTestSupplementarySource.h | 2 ++ Tests/Objects/IGTestSupplementarySource.m | 14 ++++++---- 8 files changed, 54 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c73fa9ce..8d35b4240 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,6 +90,8 @@ This release closes the [3.0.0 milestone](https://github.com/Instagram/IGListKit - Fix a potential crash when a section is moved and deleted at the same time. [Ryan Nystrom](https://github.com/rnystrom) [(#577)](https://github.com/Instagram/IGListKit/pull/577) +- Prevent section controllers and supplementary sources from returning negative sizes that crash `UICollectionViewFlowLayout`. [Ryan Nystrom](https://github.com/rnystrom) [(#583)](https://github.com/Instagram/IGListKit/pull/583) + 2.1.0 ----- diff --git a/Source/IGListAdapter.m b/Source/IGListAdapter.m index b9ab96b54..4f3e84cff 100644 --- a/Source/IGListAdapter.m +++ b/Source/IGListAdapter.m @@ -458,14 +458,16 @@ - (CGSize)sizeForItemAtIndexPath:(NSIndexPath *)indexPath { IGAssertMainThread(); IGListSectionController *sectionController = [self sectionControllerForSection:indexPath.section]; - return [sectionController sizeForItemAtIndex:indexPath.item]; + const CGSize size = [sectionController sizeForItemAtIndex:indexPath.item]; + return CGSizeMake(MAX(size.width, 0.0), MAX(size.height, 0.0)); } - (CGSize)sizeForSupplementaryViewOfKind:(NSString *)elementKind atIndexPath:(NSIndexPath *)indexPath { IGAssertMainThread(); id supplementaryViewSource = [self supplementaryViewSourceAtIndexPath:indexPath]; if ([[supplementaryViewSource supportedElementKinds] containsObject:elementKind]) { - return [supplementaryViewSource sizeForSupplementaryViewOfKind:elementKind atIndex:indexPath.item]; + const CGSize size = [supplementaryViewSource sizeForSupplementaryViewOfKind:elementKind atIndex:indexPath.item]; + return CGSizeMake(MAX(size.width, 0.0), MAX(size.height, 0.0)); } return CGSizeZero; } diff --git a/Tests/IGListAdapterTests.m b/Tests/IGListAdapterTests.m index 8bea0119b..1b2704558 100644 --- a/Tests/IGListAdapterTests.m +++ b/Tests/IGListAdapterTests.m @@ -1083,4 +1083,32 @@ - (void)test_whenSectionEdgeInsetIsNotZero { IGAssertEqualSize([self.adapter containerSizeForSectionController:controller], 98, 98); } +- (void)test_whenSectionControllerReturnsNegativeSize_thatAdapterReturnsZero { + self.dataSource.objects = @[@1]; + IGListTestSection *section = [self.adapter sectionControllerForObject:self.dataSource.objects[0]]; + section.size = CGSizeMake(-1, -1); + const CGSize size = [self.adapter sizeForItemAtIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]]; + XCTAssertEqual(size.width, 0.0); + XCTAssertEqual(size.height, 0.0); +} + +- (void)test_whenSupplementarySourceReturnsNegativeSize_thatAdapterReturnsZero { + self.dataSource.objects = @[@1]; + [self.adapter reloadDataWithCompletion:nil]; + + IGTestSupplementarySource *supplementarySource = [IGTestSupplementarySource new]; + supplementarySource.collectionContext = self.adapter; + supplementarySource.supportedElementKinds = @[UICollectionElementKindSectionFooter]; + supplementarySource.size = CGSizeMake(-1, -1); + + IGListSectionController *controller = [self.adapter sectionControllerForObject:@1]; + controller.supplementaryViewSource = supplementarySource; + supplementarySource.sectionController = controller; + + const CGSize size = [self.adapter sizeForSupplementaryViewOfKind:UICollectionElementKindSectionHeader + atIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]]; + XCTAssertEqual(size.width, 0.0); + XCTAssertEqual(size.height, 0.0); +} + @end diff --git a/Tests/Objects/IGListTestSection.h b/Tests/Objects/IGListTestSection.h index 6f5179b87..55502e7d6 100644 --- a/Tests/Objects/IGListTestSection.h +++ b/Tests/Objects/IGListTestSection.h @@ -16,6 +16,8 @@ @property (nonatomic, assign) NSInteger items; +@property (nonatomic, assign) CGSize size; + @property (nonatomic, assign) BOOL wasSelected; @end diff --git a/Tests/Objects/IGListTestSection.m b/Tests/Objects/IGListTestSection.m index 360ba8c88..98b8a3edf 100644 --- a/Tests/Objects/IGListTestSection.m +++ b/Tests/Objects/IGListTestSection.m @@ -11,6 +11,13 @@ @implementation IGListTestSection +- (instancetype)init { + if (self = [super init]) { + _size = CGSizeMake(100, 10); + } + return self; +} + - (NSArray *)cellClasses { return @[UICollectionViewCell.class]; } @@ -20,7 +27,7 @@ - (NSInteger)numberOfItems { } - (CGSize)sizeForItemAtIndex:(NSInteger)index { - return CGSizeMake(100, 10); + return self.size; } - (UICollectionViewCell *)cellForItemAtIndex:(NSInteger)index { diff --git a/Tests/Objects/IGTestStoryboardSupplementarySource.m b/Tests/Objects/IGTestStoryboardSupplementarySource.m index 9e3956c6d..7fe656a2d 100644 --- a/Tests/Objects/IGTestStoryboardSupplementarySource.m +++ b/Tests/Objects/IGTestStoryboardSupplementarySource.m @@ -26,10 +26,4 @@ - (CGSize)sizeForSupplementaryViewOfKind:(NSString *)elementKind atIndex:(NSInte return CGSizeMake([self.collectionContext containerSize].width, 45); } -- (CGSize)estimatedSizeForSupplementaryViewOfKind:(NSString *)elementKind - atIndex:(NSInteger)index { - return CGSizeZero; -} - - @end diff --git a/Tests/Objects/IGTestSupplementarySource.h b/Tests/Objects/IGTestSupplementarySource.h index f6a005fdd..87cf1c79e 100644 --- a/Tests/Objects/IGTestSupplementarySource.h +++ b/Tests/Objects/IGTestSupplementarySource.h @@ -15,6 +15,8 @@ @property (nonatomic, assign) BOOL dequeueFromNib; +@property (nonatomic, assign) CGSize size; + @property (nonatomic, strong, readwrite) NSArray *supportedElementKinds; @property (nonatomic, weak) id collectionContext; diff --git a/Tests/Objects/IGTestSupplementarySource.m b/Tests/Objects/IGTestSupplementarySource.m index 34f57e0da..646a37883 100644 --- a/Tests/Objects/IGTestSupplementarySource.m +++ b/Tests/Objects/IGTestSupplementarySource.m @@ -13,6 +13,13 @@ @implementation IGTestSupplementarySource +- (instancetype)init { + if (self = [super init]) { + _size = CGSizeMake(100, 10); + } + return self; +} + #pragma mark - IGListSupplementaryViewSource - (UICollectionReusableView *)viewForSupplementaryElementOfKind:(NSString *)elementKind @@ -34,12 +41,7 @@ - (UICollectionReusableView *)viewForSupplementaryElementOfKind:(NSString *)elem } - (CGSize)sizeForSupplementaryViewOfKind:(NSString *)elementKind atIndex:(NSInteger)index { - return CGSizeMake([self.collectionContext containerSize].width, 10); -} - -- (CGSize)estimatedSizeForSupplementaryViewOfKind:(NSString *)elementKind - atIndex:(NSInteger)index { - return CGSizeZero; + return self.size; } @end