[ASStackLayoutSpec] Fix interitem spacing not being reset on new line…

…s and add snapshot tests #trivial (TextureGroup#491) * Add snapshot tests for interitem and interline spacings of stack spec * Improve comment * Make sure item spacings are properly considered and reset on new lines, snapshot tests included
AtomTickets · Apr 25, 2018 · 77a2a8d · 77a2a8d
1 parent 1cd5eed
commit 77a2a8d
Show file tree

Hide file tree

Showing 15 changed files with 120 additions and 6 deletions.
diff --git a/Source/Private/Layout/ASStackUnpositionedLayout.mm b/Source/Private/Layout/ASStackUnpositionedLayout.mm
@@ -485,7 +485,8 @@ static CGFloat computeItemsStackDimensionSum(const std::vector<ASStackLayoutSpec
                                                   });
 
   // Sum up the childrens' dimensions (including spacing) in the stack direction.
-  const CGFloat childStackDimensionSum = std::accumulate(items.begin(), items.end(), childSpacingSum,
+  const CGFloat childStackDimensionSum = std::accumulate(items.begin(), items.end(),
+                                                         childSpacingSum,
                                                          [&](CGFloat x, const ASStackLayoutSpecItem &l) {
                                                            return x + stackDimension(style.direction, l.layout.size);
                                                          });
@@ -647,22 +648,25 @@ static void flexLinesAlongStackDimension(std::vector<ASStackUnpositionedLine> &l
   std::vector<ASStackUnpositionedLine> lines;
   std::vector<ASStackLayoutSpecItem> lineItems;
   CGFloat lineStackDimensionSum = 0;
+  CGFloat interitemSpacing = 0;
 
   for(auto it = items.begin(); it != items.end(); ++it) {
     const auto &item = *it;
     const CGFloat itemStackDimension = stackDimension(style.direction, item.layout.size);
-    const CGFloat itemAndSpacingStackDimension = (lineItems.empty() ? 0.0 : style.spacing) + item.child.style.spacingBefore + itemStackDimension + item.child.style.spacingAfter;
-    const BOOL negativeViolationIfAddItem = (ASStackUnpositionedLayout::computeStackViolation(lineStackDimensionSum + itemAndSpacingStackDimension, style, sizeRange) < 0);
+    const CGFloat itemAndSpacingStackDimension = item.child.style.spacingBefore + itemStackDimension + item.child.style.spacingAfter;
+    const BOOL negativeViolationIfAddItem = (ASStackUnpositionedLayout::computeStackViolation(lineStackDimensionSum + interitemSpacing + itemAndSpacingStackDimension, style, sizeRange) < 0);
     const BOOL breakCurrentLine = negativeViolationIfAddItem && !lineItems.empty();
 
     if (breakCurrentLine) {
       lines.push_back({.items = std::vector<ASStackLayoutSpecItem> (lineItems)});
       lineItems.clear();
       lineStackDimensionSum = 0;
+      interitemSpacing = 0;
     }
 
     lineItems.push_back(std::move(item));
-    lineStackDimensionSum += itemAndSpacingStackDimension;
+    lineStackDimensionSum += interitemSpacing + itemAndSpacingStackDimension;
+    interitemSpacing = style.spacing;
   }
 
   // Handle last line

diff --git a/Tests/ASStackLayoutSpecSnapshotTests.mm b/Tests/ASStackLayoutSpecSnapshotTests.mm
@@ -110,6 +110,7 @@ - (void)testStackLayoutSpecWithStyle:(ASStackLayoutSpecStyle)style
    alignItems:style.alignItems
    flexWrap:style.flexWrap
    alignContent:style.alignContent
+   lineSpacing:style.lineSpacing
    children:children];
 
   [self testStackLayoutSpec:stackLayoutSpec sizeRange:sizeRange subnodes:subnodes identifier:identifier];
@@ -163,15 +164,17 @@ - (void)testStackLayoutSpec:(ASStackLayoutSpec *)stackLayoutSpec
 }
 
 - (void)testStackLayoutSpecWithAlignContent:(ASStackLayoutAlignContent)alignContent
+                                lineSpacing:(CGFloat)lineSpacing
                                   sizeRange:(ASSizeRange)sizeRange
                                  identifier:(NSString *)identifier
 {
   ASStackLayoutSpecStyle style = {
     .direction = ASStackLayoutDirectionHorizontal,
     .flexWrap = ASStackLayoutFlexWrapWrap,
     .alignContent = alignContent,
+    .lineSpacing = lineSpacing,
   };
-  
+
   CGSize subnodeSize = {50, 50};
   NSArray<ASDisplayNode *> *subnodes = @[
                                          ASDisplayNodeWithBackgroundColor([UIColor redColor], subnodeSize),
@@ -181,10 +184,17 @@ - (void)testStackLayoutSpecWithAlignContent:(ASStackLayoutAlignContent)alignCont
                                          ASDisplayNodeWithBackgroundColor([UIColor greenColor], subnodeSize),
                                          ASDisplayNodeWithBackgroundColor([UIColor cyanColor], subnodeSize),
                                          ];
-  
+
   [self testStackLayoutSpecWithStyle:style sizeRange:sizeRange subnodes:subnodes identifier:identifier];
 }
 
+- (void)testStackLayoutSpecWithAlignContent:(ASStackLayoutAlignContent)alignContent
+                                  sizeRange:(ASSizeRange)sizeRange
+                                 identifier:(NSString *)identifier
+{
+  [self testStackLayoutSpecWithAlignContent:alignContent lineSpacing:0.0 sizeRange:sizeRange identifier:identifier];
+}
+
 #pragma mark -
 
 - (void)testDefaultStackLayoutElementFlexProperties
@@ -1209,6 +1219,77 @@ - (void)testBaselineAlignmentWithStretchedItem
   [self testStackLayoutSpec:stackLayoutSpec sizeRange:kSize subnodes:children identifier:nil];
 }
 
+#pragma mark - Flex wrap and item spacings test
+
+- (void)testFlexWrapWithItemSpacings
+{
+  ASStackLayoutSpecStyle style = {
+    .spacing = 50,
+    .direction = ASStackLayoutDirectionHorizontal,
+    .flexWrap = ASStackLayoutFlexWrapWrap,
+    .alignContent = ASStackLayoutAlignContentStart,
+    .lineSpacing = 5,
+  };
+
+  CGSize subnodeSize = {50, 50};
+  NSArray<ASDisplayNode *> *subnodes = @[
+                                         ASDisplayNodeWithBackgroundColor([UIColor redColor], subnodeSize),
+                                         ASDisplayNodeWithBackgroundColor([UIColor yellowColor], subnodeSize),
+                                         ASDisplayNodeWithBackgroundColor([UIColor blueColor], subnodeSize),
+                                         ];
+
+  for (ASDisplayNode *subnode in subnodes) {
+    subnode.style.spacingBefore = 5;
+    subnode.style.spacingAfter = 5;
+  }
+
+  // 3 items, each item has a size of {50, 50}
+  // width is 230px, enough to fit all items without taking all spacings into account
+  // Test that all spacings are included and therefore the last item is pushed to a second line.
+  // See: https://github.com/TextureGroup/Texture/pull/472
+  static ASSizeRange kSize = {{230, 300}, {230, 300}};
+  [self testStackLayoutSpecWithStyle:style sizeRange:kSize subnodes:subnodes identifier:nil];
+}
+
+- (void)testFlexWrapWithItemSpacingsBeingResetOnNewLines
+{
+  ASStackLayoutSpecStyle style = {
+    .spacing = 5,
+    .direction = ASStackLayoutDirectionHorizontal,
+    .flexWrap = ASStackLayoutFlexWrapWrap,
+    .alignContent = ASStackLayoutAlignContentStart,
+    .lineSpacing = 5,
+  };
+
+  CGSize subnodeSize = {50, 50};
+  NSArray<ASDisplayNode *> *subnodes = @[
+                                         // 1st line
+                                         ASDisplayNodeWithBackgroundColor([UIColor redColor], subnodeSize),
+                                         ASDisplayNodeWithBackgroundColor([UIColor yellowColor], subnodeSize),
+                                         ASDisplayNodeWithBackgroundColor([UIColor blueColor], subnodeSize),
+                                         // 2nd line
+                                         ASDisplayNodeWithBackgroundColor([UIColor magentaColor], subnodeSize),
+                                         ASDisplayNodeWithBackgroundColor([UIColor greenColor], subnodeSize),
+                                         ASDisplayNodeWithBackgroundColor([UIColor cyanColor], subnodeSize),
+                                         // 3rd line
+                                         ASDisplayNodeWithBackgroundColor([UIColor brownColor], subnodeSize),
+                                         ASDisplayNodeWithBackgroundColor([UIColor orangeColor], subnodeSize),
+                                         ASDisplayNodeWithBackgroundColor([UIColor purpleColor], subnodeSize),
+                                         ];
+
+  for (ASDisplayNode *subnode in subnodes) {
+    subnode.style.spacingBefore = 5;
+    subnode.style.spacingAfter = 5;
+  }
+
+  // 3 lines, each line has 3 items, each item has a size of {50, 50}
+  // width is 190px, enough to fit 3 items into a line
+  // Test that interitem spacing is reset on new lines. Otherwise, lines after the 1st line would have only 2 items.
+  // See: https://github.com/TextureGroup/Texture/pull/472
+  static ASSizeRange kSize = {{190, 300}, {190, 300}};
+  [self testStackLayoutSpecWithStyle:style sizeRange:kSize subnodes:subnodes identifier:nil];
+}
+
 #pragma mark - Content alignment tests
 
 - (void)testAlignContentUnderflow
@@ -1282,4 +1363,33 @@ - (void)testAlignContentStretchAndOtherAlignments
   [self testStackLayoutSpecWithStyle:style sizeRange:kSize subnodes:subnodes identifier:nil];
 }
 
+#pragma mark - Line spacing tests
+
+- (void)testAlignContentAndLineSpacingUnderflow
+{
+  // 3 lines, each line has 2 items, each item has a size of {50, 50}
+  // 10px between lines
+  // width is 110px. It's 10px bigger than the required width of each line (110px vs 100px) to test that items are still correctly collected into lines
+  static ASSizeRange kSize = {{110, 320}, {110, 320}};
+  [self testStackLayoutSpecWithAlignContent:ASStackLayoutAlignContentStart lineSpacing:10 sizeRange:kSize identifier:@"alignContentStart"];
+  [self testStackLayoutSpecWithAlignContent:ASStackLayoutAlignContentCenter lineSpacing:10 sizeRange:kSize identifier:@"alignContentCenter"];
+  [self testStackLayoutSpecWithAlignContent:ASStackLayoutAlignContentEnd lineSpacing:10 sizeRange:kSize identifier:@"alignContentEnd"];
+  [self testStackLayoutSpecWithAlignContent:ASStackLayoutAlignContentSpaceBetween lineSpacing:10 sizeRange:kSize identifier:@"alignContentSpaceBetween"];
+  [self testStackLayoutSpecWithAlignContent:ASStackLayoutAlignContentSpaceAround lineSpacing:10 sizeRange:kSize identifier:@"alignContentSpaceAround"];
+  [self testStackLayoutSpecWithAlignContent:ASStackLayoutAlignContentStretch lineSpacing:10 sizeRange:kSize identifier:@"alignContentStretch"];
+}
+
+- (void)testAlignContentAndLineSpacingOverflow
+{
+  // 6 lines, each line has 1 item, each item has a size of {50, 50}
+  // 10px between lines
+  // width is 40px. It's 10px smaller than the width of each item (40px vs 50px) to test that items are still correctly collected into lines
+  static ASSizeRange kSize = {{40, 310}, {40, 310}};
+  [self testStackLayoutSpecWithAlignContent:ASStackLayoutAlignContentStart lineSpacing:10 sizeRange:kSize identifier:@"alignContentStart"];
+  [self testStackLayoutSpecWithAlignContent:ASStackLayoutAlignContentCenter lineSpacing:10 sizeRange:kSize identifier:@"alignContentCenter"];
+  [self testStackLayoutSpecWithAlignContent:ASStackLayoutAlignContentEnd lineSpacing:10 sizeRange:kSize identifier:@"alignContentEnd"];
+  [self testStackLayoutSpecWithAlignContent:ASStackLayoutAlignContentSpaceBetween lineSpacing:10 sizeRange:kSize identifier:@"alignContentSpaceBetween"];
+  [self testStackLayoutSpecWithAlignContent:ASStackLayoutAlignContentSpaceAround lineSpacing:10 sizeRange:kSize identifier:@"alignContentSpaceAround"];
+}
+
 @end
diff --git a/...cSnapshotTests/testAlignContentAndLineSpacingOverflow_alignContentCenter@2x.png b/...cSnapshotTests/testAlignContentAndLineSpacingOverflow_alignContentCenter@2x.png
diff --git a/...SpecSnapshotTests/testAlignContentAndLineSpacingOverflow_alignContentEnd@2x.png b/...SpecSnapshotTests/testAlignContentAndLineSpacingOverflow_alignContentEnd@2x.png
diff --git a/...shotTests/testAlignContentAndLineSpacingOverflow_alignContentSpaceAround@2x.png b/...shotTests/testAlignContentAndLineSpacingOverflow_alignContentSpaceAround@2x.png
diff --git a/...hotTests/testAlignContentAndLineSpacingOverflow_alignContentSpaceBetween@2x.png b/...hotTests/testAlignContentAndLineSpacingOverflow_alignContentSpaceBetween@2x.png
diff --git a/...ecSnapshotTests/testAlignContentAndLineSpacingOverflow_alignContentStart@2x.png b/...ecSnapshotTests/testAlignContentAndLineSpacingOverflow_alignContentStart@2x.png
diff --git a/...SnapshotTests/testAlignContentAndLineSpacingUnderflow_alignContentCenter@2x.png b/...SnapshotTests/testAlignContentAndLineSpacingUnderflow_alignContentCenter@2x.png
diff --git a/...pecSnapshotTests/testAlignContentAndLineSpacingUnderflow_alignContentEnd@2x.png b/...pecSnapshotTests/testAlignContentAndLineSpacingUnderflow_alignContentEnd@2x.png
diff --git a/...hotTests/testAlignContentAndLineSpacingUnderflow_alignContentSpaceAround@2x.png b/...hotTests/testAlignContentAndLineSpacingUnderflow_alignContentSpaceAround@2x.png
diff --git a/...otTests/testAlignContentAndLineSpacingUnderflow_alignContentSpaceBetween@2x.png b/...otTests/testAlignContentAndLineSpacingUnderflow_alignContentSpaceBetween@2x.png
diff --git a/...cSnapshotTests/testAlignContentAndLineSpacingUnderflow_alignContentStart@2x.png b/...cSnapshotTests/testAlignContentAndLineSpacingUnderflow_alignContentStart@2x.png
diff --git a/...napshotTests/testAlignContentAndLineSpacingUnderflow_alignContentStretch@2x.png b/...napshotTests/testAlignContentAndLineSpacingUnderflow_alignContentStretch@2x.png
diff --git a/...nceImages_64/ASStackLayoutSpecSnapshotTests/testFlexWrapWithItemSpacings@2x.png b/...nceImages_64/ASStackLayoutSpecSnapshotTests/testFlexWrapWithItemSpacings@2x.png
diff --git a/...LayoutSpecSnapshotTests/testFlexWrapWithItemSpacingsBeingResetOnNewLines@2x.png b/...LayoutSpecSnapshotTests/testFlexWrapWithItemSpacingsBeingResetOnNewLines@2x.png