Skip to content

Commit

Permalink
fix supplementary view crash when using IGListCollectionViewLayout
Browse files Browse the repository at this point in the history
Summary:
* Issue: Calling `[UICollectionView layoutAttributesForSupplementaryElementOfKind...]` with an indexPath that doesn't have a supplimentary view.
* Cause: `IGListCollectionViewLayout` always returns a non-nil `UICollectionViewLayoutAttributes` when calling `layoutAttributesForSupplementaryViewOfKind` which tells the `UICollectionView` that it's fair game to ask the dataSource for a supplementary view at the indexPath. But when it does, the section controller could return nil, which throws an expection.

In the Apple docs about `[UICollectionViewDataSource collectionView:viewForSupplementaryElementOfKind:atIndexPath:]`

> This method must always return a valid view object. If you do not want a supplementary view in a particular case, your layout object should not create the attributes for that view.

https://developer.apple.com/documentation/uikit/uicollectionviewdatasource/1618037-collectionview?language=objc

* Fix: Just like `UICollectionViewFlowLayout`, if the supplementary view size is empty, lets return a nil attribute.

Reviewed By: Ziewvater

Differential Revision: D17326461

fbshipit-source-id: 507e98f43e951216cf2eafe2449f87df25439e11
  • Loading branch information
Maxime Ollivier authored and facebook-github-bot committed Sep 13, 2019
1 parent 2542d9e commit cddb297
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag

- Fixed logic flaw in `[IGListCollectionViewLayout shouldInvalidateLayoutForBoundsChange:]`. [Allen Hsu](https://github.com/allenhsu) (tbd)

- Fixed crash when calling `[UICollectionView layoutAttributesForSupplementaryElementOfKind...]` with `IGListCollectionViewLayout` and the section controller doesn't actually return a supplementary view [Maxime Ollivier](https://github.com/maxolls) (tbd)

3.4.0
-----

Expand Down
22 changes: 14 additions & 8 deletions Source/IGListCollectionViewLayout.mm
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ - (UICollectionViewLayoutAttributes *)finalLayoutAttributesForDisappearingItemAt
// do not add zero height headers/footers or headers/footers that are outside the rect
const CGRect frame = attributes.frame;
const CGRect intersection = CGRectIntersection(frame, rect);
if (!CGRectIsEmpty(intersection) && CGRectGetLengthInDirection(frame, self.scrollDirection) > 0.0) {
if (attributes && !CGRectIsEmpty(intersection) && CGRectGetLengthInDirection(frame, self.scrollDirection) > 0.0) {
[result addObject:attributes];
}
}
Expand Down Expand Up @@ -339,13 +339,19 @@ - (UICollectionViewLayoutAttributes *)layoutAttributesForSupplementaryViewOfKind
} else if ([elementKind isEqualToString:UICollectionElementKindSectionFooter]) {
frame = entry.footerBounds;
}

attributes = [UICollectionViewLayoutAttributes layoutAttributesForSupplementaryViewOfKind:elementKind withIndexPath:indexPath];
attributes.frame = frame;
adjustZIndexForAttributes(attributes);
_supplementaryAttributesCache[elementKind][indexPath] = attributes;

return attributes;

if (CGRectIsEmpty(frame)) {
// Just like UICollectionViewFlowLayout, if the header/footer size is empty, do not not return an attribute.
// If we did return something, calling [UICollectionView layoutAttributesForSupplementaryElementOfKind...] would too,
// which could then crash if the UICollectionViewDelegate is not expecting to actually return a supplimentary view.
return nil;
} else {
attributes = [UICollectionViewLayoutAttributes layoutAttributesForSupplementaryViewOfKind:elementKind withIndexPath:indexPath];
attributes.frame = frame;
adjustZIndexForAttributes(attributes);
_supplementaryAttributesCache[elementKind][indexPath] = attributes;
return attributes;
}
}

- (CGSize)collectionViewContentSize {
Expand Down
18 changes: 18 additions & 0 deletions Tests/IGListCollectionViewLayoutTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -1167,6 +1167,24 @@ - (void)test_whenQueryingAttributes_withItemOOB_thatReturnsNil {
XCTAssertNil([self.layout layoutAttributesForItemAtIndexPath:genIndexPath(0, 4)]);
}

- (void)test_whenQueryingSupplementaryAttributes_withSizeEmpty_thatReturnsNil {
[self setUpWithStickyHeaders:NO topInset:0 stretchToEdge:YES];

[self prepareWithData:@[
[[IGLayoutTestSection alloc] initWithInsets:UIEdgeInsetsZero
lineSpacing:0
interitemSpacing:0
headerHeight:0
footerHeight:0
items:@[
[[IGLayoutTestItem alloc] initWithSize:CGSizeMake(33, 33)],
]],
]];

XCTAssertNil([self.layout layoutAttributesForSupplementaryViewOfKind:UICollectionElementKindSectionHeader atIndexPath:genIndexPath(0, 0)]);
XCTAssertNil([self.layout layoutAttributesForSupplementaryViewOfKind:UICollectionElementKindSectionFooter atIndexPath:genIndexPath(0, 0)]);
}

- (void)test_whenUpdatingSizes_thatLayoutUpdates {
[self setUpWithStickyHeaders:NO topInset:0];

Expand Down

0 comments on commit cddb297

Please sign in to comment.