Ensure that flex children adopt their parent's cross-axis min dimension.

CharlieGreenman · Apr 7, 2015 · b831641 · b831641
1 parent b912acf
commit b831641
Show file tree

Hide file tree

Showing 6 changed files with 162 additions and 13 deletions.
diff --git a/src/Layout.c b/src/Layout.c
@@ -501,9 +501,9 @@ static void layoutNodeImpl(css_node_t *node, float parentMaxWidth) {
       child = node->get_child(node->context, i);
       float nextContentDim = 0;
 
-      // It only makes sense to consider a child flexible if we have a computed
-      // dimension for the node->
-      if (!isUndefined(node->layout.dimensions[dim[mainAxis]]) && isFlex(child)) {
+      // If it's a flexible child, accumulate the size that the child potentially
+      // contributes to the row
+      if (isFlex(child)) {
         flexibleChildrenCount++;
         totalFlexible += getFlex(child);
 
@@ -569,7 +569,7 @@ static void layoutNodeImpl(css_node_t *node, float parentMaxWidth) {
     if (!isUndefined(node->layout.dimensions[dim[mainAxis]])) {
       remainingMainDim = definedMainDim - mainContentDim;
     } else {
-      remainingMainDim = fmaxf(mainContentDim, 0) - mainContentDim;
+      remainingMainDim = boundAxis(node, mainAxis, fmaxf(mainContentDim, 0)) - mainContentDim;
     }
 
     // If there are flexible children in the mix, they are going to fill the

diff --git a/src/Layout.js b/src/Layout.js
@@ -392,9 +392,9 @@ var computeLayout = (function() {
         child = node.children[i];
         var/*float*/ nextContentDim = 0;
 
-        // It only makes sense to consider a child flexible if we have a computed
-        // dimension for the node.
-        if (!isUndefined(node.layout[dim[mainAxis]]) && isFlex(child)) {
+        // If it's a flexible child, accumulate the size that the child potentially
+        // contributes to the row
+        if (isFlex(child)) {
           flexibleChildrenCount++;
           totalFlexible += getFlex(child);
 
@@ -460,7 +460,7 @@ var computeLayout = (function() {
       if (!isUndefined(node.layout[dim[mainAxis]])) {
         remainingMainDim = definedMainDim - mainContentDim;
       } else {
-        remainingMainDim = fmaxf(mainContentDim, 0) - mainContentDim;
+        remainingMainDim = boundAxis(node, mainAxis, fmaxf(mainContentDim, 0)) - mainContentDim;
       }
 
       // If there are flexible children in the mix, they are going to fill the

diff --git a/src/__tests__/Layout-test.c b/src/__tests__/Layout-test.c
@@ -4687,6 +4687,73 @@ int main()
 
     test("should layout node with position absolute, top and left and min bounds", root_node, root_layout);
   }
+
+  {
+    css_node_t *root_node = new_test_css_node();
+    {
+      css_node_t *node_0 = root_node;
+      node_0->style.minDimensions[CSS_HEIGHT] = 800;
+      init_css_node_children(node_0, 1);
+      {
+        css_node_t *node_1;
+        node_1 = node_0->get_child(node_0->context, 0);
+        node_1->style.flex = 1;
+      }
+    }
+
+    css_node_t *root_layout = new_test_css_node();
+    {
+      css_node_t *node_0 = root_layout;
+      node_0->layout.position[CSS_TOP] = 0;
+      node_0->layout.position[CSS_LEFT] = 0;
+      node_0->layout.dimensions[CSS_WIDTH] = 0;
+      node_0->layout.dimensions[CSS_HEIGHT] = 800;
+      init_css_node_children(node_0, 1);
+      {
+        css_node_t *node_1;
+        node_1 = node_0->get_child(node_0->context, 0);
+        node_1->layout.position[CSS_TOP] = 0;
+        node_1->layout.position[CSS_LEFT] = 0;
+        node_1->layout.dimensions[CSS_WIDTH] = 0;
+        node_1->layout.dimensions[CSS_HEIGHT] = 800;
+      }
+    }
+
+    test("should layout minHeight with a flex child", root_node, root_layout);
+  }
+
+  {
+    css_node_t *root_node = new_test_css_node();
+    {
+      css_node_t *node_0 = root_node;
+      node_0->style.minDimensions[CSS_HEIGHT] = 800;
+      init_css_node_children(node_0, 1);
+      {
+        css_node_t *node_1;
+        node_1 = node_0->get_child(node_0->context, 0);
+      }
+    }
+
+    css_node_t *root_layout = new_test_css_node();
+    {
+      css_node_t *node_0 = root_layout;
+      node_0->layout.position[CSS_TOP] = 0;
+      node_0->layout.position[CSS_LEFT] = 0;
+      node_0->layout.dimensions[CSS_WIDTH] = 0;
+      node_0->layout.dimensions[CSS_HEIGHT] = 800;
+      init_css_node_children(node_0, 1);
+      {
+        css_node_t *node_1;
+        node_1 = node_0->get_child(node_0->context, 0);
+        node_1->layout.position[CSS_TOP] = 0;
+        node_1->layout.position[CSS_LEFT] = 0;
+        node_1->layout.dimensions[CSS_WIDTH] = 0;
+        node_1->layout.dimensions[CSS_HEIGHT] = 0;
+      }
+    }
+
+    test("should layout minHeight without a flex child", root_node, root_layout);
+  }
   /** END_GENERATED **/
   return tests_finished();
 }
diff --git a/src/__tests__/Layout-test.js b/src/__tests__/Layout-test.js
@@ -1513,7 +1513,7 @@ describe('Layout', function() {
     );
   });
 
-  xit('should layout minHeight with a flex child', function() {
+  it('should layout minHeight with a flex child', function() {
     testLayout(
       {style: {minHeight: 800}, children: [
         {style: {flex: 1}}
@@ -1524,6 +1524,17 @@ describe('Layout', function() {
     );
   });
 
+  it('should layout minHeight without a flex child', function() {
+    testLayout(
+      {style: {minHeight: 800}, children: [
+        {style: {}}
+      ]},
+      {width: 0, height: 800, top: 0, left: 0, children: [
+        {width: 0, height: 0, top: 0, left: 0}
+      ]}
+    );
+  });
+
   xit('should layout node with a nested sibling child with width', function() {
     testLayout(
       {style: {}, children: [

diff --git a/src/java/src/com/facebook/csslayout/LayoutEngine.java b/src/java/src/com/facebook/csslayout/LayoutEngine.java
@@ -446,9 +446,9 @@ private static void layoutNodeImpl(
         child = node.getChildAt(i);
         float nextContentDim = 0;
 
-        // It only makes sense to consider a child flexible if we have a computed
-        // dimension for the node.
-        if (!CSSConstants.isUndefined(getLayoutDimension(node, getDim(mainAxis))) && isFlex(child)) {
+        // If it's a flexible child, accumulate the size that the child potentially
+        // contributes to the row
+        if (isFlex(child)) {
           flexibleChildrenCount++;
           totalFlexible = totalFlexible + getFlex(child);
 
@@ -514,7 +514,7 @@ private static void layoutNodeImpl(
       if (!CSSConstants.isUndefined(getLayoutDimension(node, getDim(mainAxis)))) {
         remainingMainDim = definedMainDim - mainContentDim;
       } else {
-        remainingMainDim = Math.max(mainContentDim, 0) - mainContentDim;
+        remainingMainDim = boundAxis(node, mainAxis, Math.max(mainContentDim, 0)) - mainContentDim;
       }
 
       // If there are flexible children in the mix, they are going to fill the

diff --git a/src/java/tests/com/facebook/csslayout/LayoutEngineTest.java b/src/java/tests/com/facebook/csslayout/LayoutEngineTest.java
@@ -5002,5 +5002,76 @@ public void testCase119()
 
     test("should layout node with position absolute, top and left and min bounds", root_node, root_layout);
   }
+
+  @Test
+  public void testCase120()
+  {
+    TestCSSNode root_node = new TestCSSNode();
+    {
+      TestCSSNode node_0 = root_node;
+      node_0.style.minHeight = 800;
+      addChildren(node_0, 1);
+      {
+        TestCSSNode node_1;
+        node_1 = node_0.getChildAt(0);
+        node_1.style.flex = 1;
+      }
+    }
+
+    TestCSSNode root_layout = new TestCSSNode();
+    {
+      TestCSSNode node_0 = root_layout;
+      node_0.layout.y = 0;
+      node_0.layout.x = 0;
+      node_0.layout.width = 0;
+      node_0.layout.height = 800;
+      addChildren(node_0, 1);
+      {
+        TestCSSNode node_1;
+        node_1 = node_0.getChildAt(0);
+        node_1.layout.y = 0;
+        node_1.layout.x = 0;
+        node_1.layout.width = 0;
+        node_1.layout.height = 800;
+      }
+    }
+
+    test("should layout minHeight with a flex child", root_node, root_layout);
+  }
+
+  @Test
+  public void testCase121()
+  {
+    TestCSSNode root_node = new TestCSSNode();
+    {
+      TestCSSNode node_0 = root_node;
+      node_0.style.minHeight = 800;
+      addChildren(node_0, 1);
+      {
+        TestCSSNode node_1;
+        node_1 = node_0.getChildAt(0);
+      }
+    }
+
+    TestCSSNode root_layout = new TestCSSNode();
+    {
+      TestCSSNode node_0 = root_layout;
+      node_0.layout.y = 0;
+      node_0.layout.x = 0;
+      node_0.layout.width = 0;
+      node_0.layout.height = 800;
+      addChildren(node_0, 1);
+      {
+        TestCSSNode node_1;
+        node_1 = node_0.getChildAt(0);
+        node_1.layout.y = 0;
+        node_1.layout.x = 0;
+        node_1.layout.width = 0;
+        node_1.layout.height = 0;
+      }
+    }
+
+    test("should layout minHeight without a flex child", root_node, root_layout);
+  }
   /** END_GENERATED **/
 }