Margin auto allows negative values

[jacobp100]

Report

• I have searched existing issues and this is not a duplicate

Issues and Steps to Reproduce

Create a setup like follows

<View style={{ height: 500 }}>
  <View style={{ flex: 0, height: 300 }} />
  <View style={{ flex: 0, marginTop: 'auto', height: 300 }} />
</View>

Or use playground link

Expected Behavior

The bottom view should overflow the parent (as happens on the web)

Actual Behavior

The bottom view gets a negative top margin applied, and overlaps the top view. Nothing overflows the parent

Link to Code

https://yogalayout.com/playground?eyJ3aWR0aCI6NTAwLCJoZWlnaHQiOjUwMCwibWluV2lkdGgiOm51bGwsIm1pbkhlaWdodCI6bnVsbCwibWF4V2lkdGgiOm51bGwsIm1heEhlaWdodCI6bnVsbCwiZmxleERpcmVjdGlvbiI6MCwicG9zaXRpb24iOnsidG9wIjpudWxsLCJyaWdodCI6bnVsbCwiYm90dG9tIjpudWxsLCJsZWZ0IjpudWxsfSwiY2hpbGRyZW4iOlt7IndpZHRoIjoiIiwiaGVpZ2h0IjoiMzAwIiwibWluV2lkdGgiOm51bGwsIm1pbkhlaWdodCI6bnVsbCwibWF4V2lkdGgiOm51bGwsIm1heEhlaWdodCI6bnVsbCwicG9zaXRpb24iOnsidG9wIjpudWxsLCJyaWdodCI6bnVsbCwiYm90dG9tIjpudWxsLCJsZWZ0IjpudWxsfSwiZmxleFNocmluayI6IjAifSx7IndpZHRoIjoiIiwiaGVpZ2h0IjoiMzAwIiwibWluV2lkdGgiOm51bGwsIm1pbkhlaWdodCI6bnVsbCwibWF4V2lkdGgiOm51bGwsIm1heEhlaWdodCI6bnVsbCwibWFyZ2luIjp7InRvcCI6ImF1dG8ifSwicG9zaXRpb24iOnsidG9wIjpudWxsLCJyaWdodCI6bnVsbCwiYm90dG9tIjpudWxsLCJsZWZ0IjpudWxsfSwiZmxleFNocmluayI6IjAifV19

Notes

If you remove marginTop: auto you get the correct behaviour

I did find these few lines

I'm not sure what the logic of remainingFreeSpace is - if it should allow negative values or not. If it does allow them, it might just need a max(0, ...)

[woehrl01]

Hi @jacobp100

Thank you for reporting this. I just pushed a PR for that bug fix.

[jacobp100]

Amazing. Thanks so much!

[jacobp100]

@NickGerleman is this something you'd be willing to accept a PR for Yoga 3? Or is it unlikely to get accepted if it'll break things at Meta

https://www.yogalayout.dev/playground?code=DwGQhgng9grgLgAgMZQHYDMCWBzAvAb3xgGcBTAdVICMARU9MGAGzmIC4EGmyBfHgPgBQCBMAByUACakExOBCakC+AO6ZJcABYcAzAFYADABoEm0jk1xdhkwAcwkyZlTYOAJgN8hIkeKky5BSVCNQ1tBB0DY1NzbEsOABYokwBbMAAnbGcAISg4OCgUjgByRgLivgQAem8fP2lZeUVlUK1dZJiLKwQARijKmuFRKolpIWAq8Gh4fiA

[NickGerleman]

I’d be willing to give it another go. I’ve been leaning a bit on merging things where existing coverage doesn’t show lots of product changes, and this one might not be too bad.

[jacobp100]

I think it's literally just these two lines

https://github.com/facebook/yoga/blob/main/yoga/algorithm/CalculateLayout.cpp#L1046
https://github.com/facebook/yoga/blob/main/yoga/algorithm/CalculateLayout.cpp#L1062

Need a max(flexLine.layout.remainingFreeSpace, 0)

[NickGerleman]

I think the change will stick, but fixing justify-content overflow behavior in particular did subtlety break a couple real-world UIs at Meta. Namely, there were cases where main axis would overflow, and justification would happen to remove space between items to make it not noticeable. Should be rare though I think.