From b8316413b310643ea6555a015b3903f4fde1104e Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Tue, 7 Apr 2015 10:52:15 +0800 Subject: [PATCH] Ensure that flex children adopt their parent's cross-axis min dimension. --- src/Layout.c | 8 +-- src/Layout.js | 8 +-- src/__tests__/Layout-test.c | 67 +++++++++++++++++ src/__tests__/Layout-test.js | 13 +++- .../com/facebook/csslayout/LayoutEngine.java | 8 +-- .../facebook/csslayout/LayoutEngineTest.java | 71 +++++++++++++++++++ 6 files changed, 162 insertions(+), 13 deletions(-) diff --git a/src/Layout.c b/src/Layout.c index 002b7952..5fbbd594 100644 --- 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 index 69d06b31..652dc891 100755 --- 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 index 9b9e3008..99c23a52 100644 --- 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 index 67a96dc4..53f02106 100755 --- 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 index 6169fbcf..2518bb00 100644 --- a/src/java/src/com/facebook/csslayout/LayoutEngine.java +++ b/src/java/src/com/facebook/csslayout/LayoutEngine.java @@ -446,9 +446,9 @@ public class LayoutEngine { 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 @@ public class LayoutEngine { 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 index 9942017d..3754ce4b 100644 --- a/src/java/tests/com/facebook/csslayout/LayoutEngineTest.java +++ b/src/java/tests/com/facebook/csslayout/LayoutEngineTest.java @@ -5002,5 +5002,76 @@ public class LayoutEngineTest { 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 **/ }