maxHeight incorrectly includes margins in the calculation #466

Closed
opened 2017-03-08 02:19:00 -08:00 by rigdern · 2 comments
rigdern commented 2017-03-08 02:19:00 -08:00 (Migrated from github.com)

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}, },
]},
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: ```html <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: ```c 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 commented 2017-03-08 09:23:46 -08:00 (Migrated from github.com)

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

I got this, fix is coming in a few minutes. Thank you for the great description. 👍
rigdern commented 2017-03-08 11:41:25 -08:00 (Migrated from github.com)

@woehrl01 Thanks for the quick turnaround!

@woehrl01 Thanks for the quick turnaround!
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#466
No description provided.