-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Measure nodes which have margin: auto and align-item: stretch #645
Conversation
ba9020c
to
65cccd3
Compare
65cccd3
to
c0c2616
Compare
@woehrl01 Can you ensure we have a test case to catch the case where we have auto + stretch without a custom measure? Both when node has explicit and non-explcit size set. |
If I remember correctly we already have that in place for non custom measure nodes. This change is only needed for custom measure nodes. I added those when I initially added the auto margin support. |
I can't find any test in https://github.com/facebook/yoga/blob/master/tests/YGMarginTest.cpp which sets auto margin without defined width/height (thus relying on stretch behaviour). Could you add one? |
Sure 👍 I'll have a look in the next couple of hours. I'm currently out of office. |
👍 |
@emilsjolander I hope these are the kind of tests you are looking for. 🔬 |
Looks good! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@emilsjolander is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: If you have a measurable node and set ```marign-left: auto``` + ```align-item:stretch``` on it, it won't get measured and they get a width/height of ```-(nan)```. This change fixes that behaviour. Fixes #644. Closes #645 Differential Revision: D6413512 Pulled By: emilsjolander fbshipit-source-id: 755febeb33bb0d4520ca6b3c28d56ac333e4a14d
Summary: If you have a measurable node and set ```marign-left: auto``` + ```align-item:stretch``` on it, it won't get measured and they get a width/height of ```-(nan)```. This change fixes that behaviour. Fixes #644. Closes facebook/yoga#645 Differential Revision: D6413512 Pulled By: emilsjolander fbshipit-source-id: 755febeb33bb0d4520ca6b3c28d56ac333e4a14d
Summary: If you have a measurable node and set ```marign-left: auto``` + ```align-item:stretch``` on it, it won't get measured and they get a width/height of ```-(nan)```. This change fixes that behaviour. Fixes facebook#644. Closes facebook/yoga#645 Differential Revision: D6413512 Pulled By: emilsjolander fbshipit-source-id: 755febeb33bb0d4520ca6b3c28d56ac333e4a14d
Summary: If you have a measurable node and set ```marign-left: auto``` + ```align-item:stretch``` on it, it won't get measured and they get a width/height of ```-(nan)```. This change fixes that behaviour. Fixes facebook#644. Closes facebook/yoga#645 Differential Revision: D6413512 Pulled By: emilsjolander fbshipit-source-id: 755febeb33bb0d4520ca6b3c28d56ac333e4a14d
Summary: If you have a measurable node and set ```marign-left: auto``` + ```align-item:stretch``` on it, it won't get measured and they get a width/height of ```-(nan)```. This change fixes that behaviour. Fixes #644. Closes facebook/yoga#645 Differential Revision: D6413512 Pulled By: emilsjolander fbshipit-source-id: 755febeb33bb0d4520ca6b3c28d56ac333e4a14d
If you have a measurable node and set
marign-left: auto
+align-item:stretch
on it, it won't get measured and they get a width/height of-(nan)
. This change fixes that behaviour. Fixes #644.