From 2e996e283e9d35422162df89db21df30c1f30ec2 Mon Sep 17 00:00:00 2001 From: Ryan Nystrom Date: Sat, 4 Mar 2017 16:08:00 -0800 Subject: [PATCH] Test and fix layout fitting width and inset sections with newline Summary: Fixes internally reported issue where the layout was new lining a section that had proper width math. Also, when inset sections were all on the same line, the max height added inset for each section, causing the newline to be too low. - [x] All tests pass. Demo project builds and runs. - [x] I added tests, an experiment, or detailed why my change isn't tested. Closes https://github.com/Instagram/IGListKit/pull/522 Reviewed By: jessesquires Differential Revision: D4655554 Pulled By: rnystrom fbshipit-source-id: 5e759451e285ccd636ed59dcee0777ce590797ad --- Source/IGListCollectionViewLayout.mm | 23 ++++++++++++----------- Tests/IGListCollectionViewLayoutTests.m | 25 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/Source/IGListCollectionViewLayout.mm b/Source/IGListCollectionViewLayout.mm index 4712fb7f..8399a061 100644 --- a/Source/IGListCollectionViewLayout.mm +++ b/Source/IGListCollectionViewLayout.mm @@ -320,8 +320,8 @@ static void adjustZIndexForAttributes(UICollectionViewLayoutAttributes *attribut auto sectionData = std::vector(sectionCount); CGFloat itemY = 0.0; - CGFloat maxRowHeight = 0.0; CGFloat itemX = 0.0; + CGFloat nextRowY = 0.0; // union item frames and optionally the header to find a bounding box of the entire section CGRect rollingSectionBounds; @@ -342,10 +342,14 @@ static void adjustZIndexForAttributes(UICollectionViewLayoutAttributes *attribut // header height is subtracted from the sectionBounds when calculating the header bounds after items are done // this bumps the first row of items down enough to make room for the header itemY += headerSize.height; + nextRowY += headerSize.height; // add the left inset in case the section falls on the same row as the previous // if the section is newlined then the x is reset itemX += insets.left; + + // the farthest right the frame of an item in this section can go + const CGFloat maxX = width - insets.right; for (NSInteger item = 0; item < itemCount; item++) { NSIndexPath *indexPath = [NSIndexPath indexPathForItem:item inSection:section]; @@ -358,11 +362,10 @@ static void adjustZIndexForAttributes(UICollectionViewLayoutAttributes *attribut // if the x + width of the item busts the width of the container // or if this is the first item and the header has a non-zero size // newline to the next row and reset - if ((itemX + itemWidth > paddedWidth) || - (item == 0 && headerExists)) { - itemY += maxRowHeight; + if (itemX + itemWidth > maxX + || (item == 0 && headerExists)) { + itemY = nextRowY; itemX = insets.left; - maxRowHeight = 0.0; // if newlining, always append line spacing unless its the very first item of the section if (item > 0) { @@ -376,10 +379,8 @@ static void adjustZIndexForAttributes(UICollectionViewLayoutAttributes *attribut size.height); sectionData[section].itemBounds[item] = frame; - // track the max size of the row to find the y of the next row - if (size.height > maxRowHeight) { - maxRowHeight = size.height; - } + // track the max size of the row to find the y of the next row, adjust for top inset while iterating items + nextRowY = MAX(CGRectGetMaxY(frame) - insets.top, nextRowY); // increase the rolling x by the item width and add item spacing for all items on the same row itemX += itemWidth + interitemSpacing; @@ -410,8 +411,8 @@ static void adjustZIndexForAttributes(UICollectionViewLayoutAttributes *attribut // bump the x for the next section with the right insets itemX += insets.right; - // account for the top/bottom insets of the section since maxRowHeight only uses the item size.height - maxRowHeight += insets.top + insets.bottom; + // find the lowest point in the section and add the bottom inset to find the next row's Y + nextRowY = MAX(nextRowY, CGRectGetMaxY(rollingSectionBounds) + insets.bottom); } _sectionData = sectionData; diff --git a/Tests/IGListCollectionViewLayoutTests.m b/Tests/IGListCollectionViewLayoutTests.m index 9f8cd441..81b5b4c2 100644 --- a/Tests/IGListCollectionViewLayoutTests.m +++ b/Tests/IGListCollectionViewLayoutTests.m @@ -587,4 +587,29 @@ XCTAssertEqual(CGRectGetHeight(expected), CGRectGetHeight(frame)); \ IGAssertEqualFrame([self cellForSection:0 item:1].frame, 30, 20, 40, 20); } +- (void)test_ { + [self setUpWithStickyHeaders:NO topInset:0]; + self.collectionView.frame = CGRectMake(0, 0, 414, 736); + + NSMutableArray *data = [NSMutableArray new]; + for (NSInteger i = 0; i < 6; i++) { + [data addObject:[[IGLayoutTestSection alloc] initWithInsets:UIEdgeInsetsMake(1, 1, 1, 1) + lineSpacing:0 + interitemSpacing:0 + headerHeight:0 + items:@[ + [[IGLayoutTestItem alloc] initWithSize:(CGSize){136, 136}], + ]]]; + } + [self prepareWithData:data]; + + XCTAssertEqual(self.collectionView.contentSize.height, 276); + IGAssertEqualFrame([self cellForSection:0 item:0].frame, 1, 1, 136, 136); + IGAssertEqualFrame([self cellForSection:1 item:0].frame, 139, 1, 136, 136); + IGAssertEqualFrame([self cellForSection:2 item:0].frame, 277, 1, 136, 136); + IGAssertEqualFrame([self cellForSection:3 item:0].frame, 1, 139, 136, 136); + IGAssertEqualFrame([self cellForSection:4 item:0].frame, 139, 139, 136, 136); + IGAssertEqualFrame([self cellForSection:5 item:0].frame, 277, 139, 136, 136); +} + @end