Skip to content

Make 'stretch' aligned children fill cross-axis in containers of undefined size in the cross-axis #171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

jsendros
Copy link

Summary: Fetched and rebased
#145 and updated it to do 2
things:

  1. Make 'stretch' aligned children grow to fill their containers in the
    cross-axis if the cross-axis size is undefined. The added test might be
    the best explanation of this.
  2. Make sure this doesn’t decrease the frequency of the
    “simpleStackCross” optimization in a relatively complex sample
    environment.

Test Plan: Added a test: Container with 3 row directioned children and
no explicit height set. One child is of height 0, one of 0 height with
stretch alignment, and one of 150 height. Expected the 0 height child
remains of height 0, stretch grows to 150, and 150 stays the same.

@lucasr
Copy link
Contributor

lucasr commented Feb 22, 2016

Looks good, just needs the suggested cleanups. Please communicate about these changes here in #145 as this is based on his work.

@jsendros
Copy link
Author

The reason all the tests change in LayoutCachingTest.java is because they all have roots which are all default styled (flexDirection: column, alignItems: stretch). The width of the root changes, so the width of the leaves must change.

@lucasr
Copy link
Contributor

lucasr commented Feb 23, 2016

Looks good but you should also land the generated code for each language in the repo.

@lucasr
Copy link
Contributor

lucasr commented Feb 23, 2016

Never mind, the commit code does have the generated code. One C# test is failing though.

@jsendros jsendros force-pushed the stretch-cross-axis branch 2 times, most recently from 639bd7b to f29af8a Compare February 23, 2016 17:34
…fined size in the cross-axis

Summary: Fetched and rebased
facebook#145 and updated it to do 2
things:
1) Make 'stretch' aligned children grow to fill their containers in the
cross-axis if the cross-axis size is undefined. The added test might be
the best explanation of this.
2) Make sure this doesn’t decrease the frequency of the
“simpleStackCross” optimization in a relatively complex sample
environment.

Test Plan: Added a test: Container with 3 row directioned children and
no explicit height set. One child is of height 0, one of 0 height with
stretch alignment, and one of 150 height. Expected the 0 height child
remains of height 0, stretch grows to 150, and 150 stays the same.
@@ -155,7 +155,7 @@ public void testInvalidatesFullTreeWhenParentWidthChanges()
root.calculateLayout();
markLayoutAppliedForTree(root);

c0.Height = 200;
c0.Width = 200;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This right here... This is why I wish there wasn't code(or test) duplication.

@lucasr
Copy link
Contributor

lucasr commented Mar 1, 2016

Landed in #145

@lucasr lucasr closed this Mar 1, 2016
child->layout.position[trailing[crossAxis]] -= getTrailingMargin(child, crossAxis) +
getRelativePosition(child, crossAxis);

layoutNode(child, maxWidth, maxHeight, direction);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes us to call layoutNode recursively a second time, which has side-effects. The bug we're seeing because of this is that items that are centered get their leading dimension incremented twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants