Margin auto allows negative values #978

New Issue

2020-03-02T05:11:01-08:00

jacobp100 commented

2020-03-02 05:11:01 -08:00

(Migrated from github.com)

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, ...)

# Report - [x] I have searched [existing issues](https://github.com/facebook/yoga/issues) and this is not a duplicate # Issues and Steps to Reproduce Create a setup like follows ```js <View style={{ height: 500 }}> <View style={{ flex: 0, height: 300 }} /> <View style={{ flex: 0, marginTop: 'auto', height: 300 }} /> </View> ``` [Or use playground link](https://yogalayout.com/playground?eyJ3aWR0aCI6NTAwLCJoZWlnaHQiOjUwMCwibWluV2lkdGgiOm51bGwsIm1pbkhlaWdodCI6bnVsbCwibWF4V2lkdGgiOm51bGwsIm1heEhlaWdodCI6bnVsbCwiZmxleERpcmVjdGlvbiI6MCwicG9zaXRpb24iOnsidG9wIjpudWxsLCJyaWdodCI6bnVsbCwiYm90dG9tIjpudWxsLCJsZWZ0IjpudWxsfSwiY2hpbGRyZW4iOlt7IndpZHRoIjoiIiwiaGVpZ2h0IjoiMzAwIiwibWluV2lkdGgiOm51bGwsIm1pbkhlaWdodCI6bnVsbCwibWF4V2lkdGgiOm51bGwsIm1heEhlaWdodCI6bnVsbCwicG9zaXRpb24iOnsidG9wIjpudWxsLCJyaWdodCI6bnVsbCwiYm90dG9tIjpudWxsLCJsZWZ0IjpudWxsfSwiZmxleFNocmluayI6IjAifSx7IndpZHRoIjoiIiwiaGVpZ2h0IjoiMzAwIiwibWluV2lkdGgiOm51bGwsIm1pbkhlaWdodCI6bnVsbCwibWF4V2lkdGgiOm51bGwsIm1heEhlaWdodCI6bnVsbCwibWFyZ2luIjp7InRvcCI6ImF1dG8ifSwicG9zaXRpb24iOnsidG9wIjpudWxsLCJyaWdodCI6bnVsbCwiYm90dG9tIjpudWxsLCJsZWZ0IjpudWxsfSwiZmxleFNocmluayI6IjAifV19) # 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](https://github.com/facebook/yoga/blob/master/yoga/Yoga.cpp#L2601-L2605) 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 commented

2020-06-09 04:11:52 -07:00

(Migrated from github.com)

Hi @jacobp100

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

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

❤️ 1

jacobp100 commented

2020-06-09 04:46:28 -07:00

(Migrated from github.com)

Amazing. Thanks so much!

jacobp100 commented

2024-03-21 15:35:42 -07:00

(Migrated from github.com)

@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 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 commented

2024-03-21 16:34:15 -07:00

(Migrated from github.com)

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 commented

2024-03-22 02:30:17 -07:00

(Migrated from github.com)

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)

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)`

👍 2

NickGerleman commented

2024-04-18 16:00:26 -07:00

(Migrated from github.com)

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.

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.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#978