Skip to content
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

maxHeight incorrectly includes margins in the calculation #466

Closed
rigdern opened this issue Mar 8, 2017 · 2 comments
Closed

maxHeight incorrectly includes margins in the calculation #466

rigdern opened this issue Mar 8, 2017 · 2 comments

Comments

@rigdern
Copy link

rigdern commented Mar 8, 2017

When a node has both a max height and a vertical margin, it appears that Yoga incorrectly considers the margins to take up part of the max height.

Repro Steps

Here's some HTML representing a case that repros the bug:

<div style="width: 250px; height: 250px;">
  <div style="width: 100px; height: 100px; max-height: 100px; margin-top: 20px;"></div>
</div>

Here's the equivalent C code:

  const YGNodeRef root = YGNodeNew();
  YGNodeStyleSetFlexDirection(root, YGFlexDirectionRow);
  YGNodeStyleSetWidth(root, 250);
  YGNodeStyleSetHeight(root, 250);
  
  const YGNodeRef root_child0 = YGNodeNew();
  YGNodeStyleSetWidth(root_child0, 100);
  YGNodeStyleSetHeight(root_child0, 100);
  YGNodeStyleSetMaxHeight(root_child0, 100);
  YGNodeStyleSetMargin(root_child0, YGEdgeTop, 20);
  YGNodeInsertChild(root, root_child0, 0);
  
  YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
  YGNodePrint(root, YGPrintOptionsLayout | YGPrintOptionsChildren);

Expected Output

Child's height is 100:

{layout: {width: 250, height: 250, top: 0, left: 0}, children: [
  {layout: {width: 100, height: 100, top: 20, left: 0}, },
]},

This JSFiddle illustrates the expected behavior: https://jsfiddle.net/4mr6tbqq/

Actual Output

Child's height is 80:

{layout: {width: 250, height: 250, top: 0, left: 0}, children: [
  {layout: {width: 100, height: 80, top: 20, left: 0}, },
]},
@woehrl01
Copy link
Contributor

woehrl01 commented Mar 8, 2017

I got this, fix is coming in a few minutes. Thank you for the great description. 👍

@rigdern
Copy link
Author

rigdern commented Mar 8, 2017

@woehrl01 Thanks for the quick turnaround!

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

No branches or pull requests

2 participants