DaddyFrosty/yoga
1
0
Fork 0
Code Issues 96 Pull Requests 31 Actions Packages Projects Releases 33 Wiki Activity

Fix bugs introduced with YogaKit improvements #328

Closed
hartbit wants to merge 8 commits from yogakit-edge-bugs into master
pull from: yogakit-edge-bugs
merge into: DaddyFrosty:master
Conversation 31 Commits 8 Files Changed 4 +443 -95
hartbit commented 2017-01-07 10:23:31 -08:00 (Migrated from github.com)
Copy Link

I'm trying to fix some bugs I introduced in my latest PR, but while writing the Unit Tests for them, I saw a really weird behaviour. The following exact piece of code WORKS inside a Yoga C++ unit test, but fails from a Objective-C unit test. I had me completely confused and blocked me in my progression. Any ideas?

From C

TEST(YogaTest, stupid_test) {
  const YGNodeRef node = YGNodeNew();
  YGNodeStyleSetPosition(node, YGEdgeLeft, 1);

  ASSERT_FLOAT_EQ(1, YGNodeStyleGetPosition(node, YGEdgeLeft).value);
  ASSERT_EQ(YGUnitPixel, YGNodeStyleGetPosition(node, YGEdgeLeft).unit);

  YGNodeFree(node);
}

From Objective-C

- (void)testPositionalPropertiesWork
{
  YGNodeRef node = YGNodeNew();
  YGNodeStyleSetPosition(node, YGEdgeLeft, 1);

  XCTAssertEqual(1, YGNodeStyleGetPosition(node, YGEdgeLeft).value);
  XCTAssertEqual(YGUnitPixel, YGNodeStyleGetPosition(node, YGEdgeLeft).unit);

  YGNodeFree(node);
}
I'm trying to fix some bugs I introduced in my latest PR, but while writing the Unit Tests for them, I saw a really weird behaviour. The following exact piece of code WORKS inside a Yoga C++ unit test, but fails from a Objective-C unit test. I had me completely confused and blocked me in my progression. Any ideas? ## From C ``` TEST(YogaTest, stupid_test) { const YGNodeRef node = YGNodeNew(); YGNodeStyleSetPosition(node, YGEdgeLeft, 1); ASSERT_FLOAT_EQ(1, YGNodeStyleGetPosition(node, YGEdgeLeft).value); ASSERT_EQ(YGUnitPixel, YGNodeStyleGetPosition(node, YGEdgeLeft).unit); YGNodeFree(node); } ``` ## From Objective-C ``` - (void)testPositionalPropertiesWork { YGNodeRef node = YGNodeNew(); YGNodeStyleSetPosition(node, YGEdgeLeft, 1); XCTAssertEqual(1, YGNodeStyleGetPosition(node, YGEdgeLeft).value); XCTAssertEqual(YGUnitPixel, YGNodeStyleGetPosition(node, YGEdgeLeft).unit); YGNodeFree(node); } ```
hartbit commented 2017-01-07 10:25:08 -08:00 (Migrated from github.com)
Copy Link

Btw, while stepping through the debugger, the Yoga YGNodeStyleGetPosition method seems to calculate the right value, but somehow, in between the value being returned and saved on the stack of the Objective-C method, the YGValue struct is filled with nonsense.

I committed the Objective-C tests if anybody wants to play with this @dshahidehpour @emilsjolander

Btw, while stepping through the debugger, the Yoga `YGNodeStyleGetPosition` method seems to calculate the right value, but somehow, in between the value being returned and saved on the stack of the Objective-C method, the `YGValue` struct is filled with nonsense. I committed the Objective-C tests if anybody wants to play with this @dshahidehpour @emilsjolander
d16r commented 2017-01-07 10:35:45 -08:00 (Migrated from github.com)
Copy Link

@hartbit I'm patching onto my machine right now to take a look. Did you try running the tests via BUCK and Xcode? It should make absolutely no difference, just curious.

@hartbit I'm patching onto my machine right now to take a look. Did you try running the tests via BUCK and Xcode? It should make absolutely no difference, just curious.
hartbit commented 2017-01-07 10:41:34 -08:00 (Migrated from github.com)
Copy Link

@dshahidehpour I tried both with BUCK and Xcode and got the same results. But as a side note, some other tests do fail when run through Xcode, for two reasons I think:

  1. Xcode runs some tests in parallel, which cause YGNodeGetInstanceCount to return more instances than those of the test calling the function.

  2. Xcode defaults to running the tests on a retina simulator, which causes some of the UILabels to return slightly different sizes when sizeThatFits is called. Those tests should probably be written to not depend on precise values of UILabel sizes.

David.

@dshahidehpour I tried both with BUCK and Xcode and got the same results. But as a side note, some other tests do fail when run through Xcode, for two reasons I think: 1) Xcode runs some tests in parallel, which cause `YGNodeGetInstanceCount` to return more instances than those of the test calling the function. 2) Xcode defaults to running the tests on a retina simulator, which causes some of the UILabels to return slightly different sizes when `sizeThatFits` is called. Those tests should probably be written to not depend on precise values of UILabel sizes. David.
d16r commented 2017-01-07 10:57:10 -08:00 (Migrated from github.com)
Copy Link

Not totally helpful, but, I tried running this test before the Swift API diff landed, and the test passes, not sure if this tells us anything helpful. I have to go offline, but I'll revisit ASAP!

Not **totally** helpful, but, I tried running this test before the Swift API diff landed, and the test passes, not sure if this tells us anything helpful. I have to go offline, but I'll revisit ASAP!
hartbit commented 2017-01-07 11:47:51 -08:00 (Migrated from github.com)
Copy Link

Travis failed initially because I forgot to commit some minor changes. Lets see what it says now...

Travis failed initially because I forgot to commit some minor changes. Lets see what it says now...
hartbit commented 2017-01-07 12:00:03 -08:00 (Migrated from github.com)
Copy Link

Travis won't run because my previous PR was reverted. Anyway, this is the command I run to test YogaKit and which fails on this branch:

buck test //YogaKit:YogaKitTests --config cxx.default_platform=iphonesimulator-x86_64 --config cxx.cflags=-DTRAVIS_CI
Travis won't run because my previous PR was reverted. Anyway, this is the command I run to test YogaKit and which fails on this branch: ``` buck test //YogaKit:YogaKitTests --config cxx.default_platform=iphonesimulator-x86_64 --config cxx.cflags=-DTRAVIS_CI ```
d16r commented 2017-01-09 09:09:13 -08:00 (Migrated from github.com)
Copy Link

@hartbit The diff has relanded, are you still seeing this issue after rebasing?

@hartbit The diff has relanded, are you still seeing this issue after rebasing?
hartbit commented 2017-01-09 14:00:06 -08:00 (Migrated from github.com)
Copy Link

@dshahidehpour The diff fixed the problems I was having and I was able to finish this PR and fix the remaining bugs and add the required tests.

In doing so, I found a problem: the getters for the horizontal, vertical and all properties crashes because the Yoga C API throws an assert when attempting to get those values back, as if it was not expected. I fixed it by removing the assert in the C API and documenting the change by writing C unit tests that demonstrate what the output is supposed to me.

I'm open to discussion.

@dshahidehpour The diff fixed the problems I was having and I was able to finish this PR and fix the remaining bugs and add the required tests. In doing so, I found a problem: the getters for the `horizontal`, `vertical` and `all` properties crashes because the Yoga C API throws an assert when attempting to get those values back, as if it was not expected. I *fixed* it by removing the assert in the C API and documenting the change by writing C unit tests that demonstrate what the output is supposed to me. I'm open to discussion.
d16r (Migrated from github.com) reviewed 2017-01-09 14:40:20 -08:00
yoga/Yoga.c
d16r (Migrated from github.com) commented 2017-01-09 14:40:20 -08:00
Copy Link

@emilsjolander any idea why this is needed?

@emilsjolander any idea why this is needed?
d16r (Migrated from github.com) reviewed 2017-01-09 14:42:53 -08:00
YogaKit/Tests/YogaKitTests.m
@@ -387,0 +408,4 @@
view.yoga.bottom = 4;
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeBottom).value, 4);
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeBottom).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.bottom, 4);
d16r (Migrated from github.com) commented 2017-01-09 14:42:30 -08:00
Copy Link

I think we can remove the verification that YGNodeRef's setters are working, that shouldn't be our responsibility.

I do think it's valuable to verify that they are set to Pixel values, though.

I do think it's valuable to verify that getter for view.yoga.* works, and that it is a YGUnitPixel would be more than enough. Any chance we can condense these tests so that we simply test that different

I think we can remove the verification that `YGNodeRef`'s setters are working, that shouldn't be our responsibility. I do think it's valuable to verify that they are set to `Pixel` values, though. I do think it's valuable to verify that getter for `view.yoga.*` works, and that it is a `YGUnitPixel` would be more than enough. Any chance we can condense these tests so that we simply test that different
hartbit (Migrated from github.com) reviewed 2017-01-09 14:53:05 -08:00
YogaKit/Tests/YogaKitTests.m
@@ -387,0 +408,4 @@
view.yoga.bottom = 4;
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeBottom).value, 4);
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeBottom).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.bottom, 4);
hartbit (Migrated from github.com) commented 2017-01-09 14:53:05 -08:00
Copy Link

I think those tests are important. They were failing because a bug in the macros I wrote was causing the getters/setters to not be defined and Objective-C would therefore auto-synthesise them, causing the YGNodeSet functions to never be called. The tests make sure the macros are defined and called correctly to both (1) define the correct getters and setters and (2) have the setters call YGNodeSet. Its so easy to mess up the macros and never see it.

I think those tests are important. They were failing because a bug in the macros I wrote was causing the getters/setters to not be defined and Objective-C would therefore auto-synthesise them, causing the `YGNodeSet` functions to never be called. The tests make sure the macros are defined and called correctly to both (1) define the correct getters and setters and (2) have the setters call `YGNodeSet`. Its so easy to mess up the macros and never see it.
emilsjolander (Migrated from github.com) reviewed 2017-01-10 03:34:52 -08:00
yoga/Yoga.c
emilsjolander (Migrated from github.com) commented 2017-01-10 03:34:52 -08:00
Copy Link

It is not needed i guess. but getting the computed value of "all" or "horizontal edge just does not make sense. e.g.

node.marginHorizontal = 10;
node.marginLeft = 20;
node.marginRight = 30;

At this point resolving marginHorizontal does not make much sense. I don't think this should be fixed by removing the assert. If we want to return the value set then we should instead have another function to do so.

It is not needed i guess. but getting the computed value of "all" or "horizontal edge just does not make sense. e.g. ``` node.marginHorizontal = 10; node.marginLeft = 20; node.marginRight = 30; ``` At this point resolving `marginHorizontal` does not make much sense. I don't think this should be fixed by removing the assert. If we want to return the value set then we should instead have another function to do so.
d16r (Migrated from github.com) reviewed 2017-01-10 06:13:09 -08:00
YogaKit/Tests/YogaKitTests.m
@@ -387,0 +408,4 @@
view.yoga.bottom = 4;
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeBottom).value, 4);
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeBottom).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.bottom, 4);
d16r (Migrated from github.com) commented 2017-01-10 06:13:09 -08:00
Copy Link

Thanks for clarifying!

I was just thinking that if we had a unit test that tested layout while using margin/paddng/border, etc, it would become clear that the NodeRef's weren't being properly set because the layout would be in correct. Since this accomplishes the same thing (yet in a slightly more verbose way) I won't argue the point anymore :) Thanks for the test cases!

Thanks for clarifying! I was just thinking that if we had a unit test that tested layout while using margin/paddng/border, etc, it would become clear that the `NodeRef`'s weren't being properly set because the layout would be in correct. Since this accomplishes the same thing (yet in a slightly more verbose way) I won't argue the point anymore :) Thanks for the test cases!
d16r (Migrated from github.com) reviewed 2017-01-10 06:20:57 -08:00
yoga/Yoga.c
d16r (Migrated from github.com) commented 2017-01-10 06:20:57 -08:00
Copy Link

@emilsjolander So If I'm understanding you right we should:

  1. Add the ASSERT back.
  2. Change all the tests where we call a getter on YGEdgeAll, YGEdgeHorizontal, YGEdgeVertical to access their respective representations.
  3. Remove the new YGEdgeTests since they are testing functionality we don't support.
@emilsjolander So If I'm understanding you right we should: 1. Add the `ASSERT` back. 2. Change all the tests where we call a getter on `YGEdgeAll`, `YGEdgeHorizontal`, `YGEdgeVertical` to access their respective representations. 3. Remove the new `YGEdgeTests` since they are testing functionality we don't support.
emilsjolander (Migrated from github.com) reviewed 2017-01-10 06:47:21 -08:00
emilsjolander (Migrated from github.com) left a comment
Copy Link
  • nit: test*PropertiesWork -> test*Properties
- nit: `test*PropertiesWork` -> `test*Properties`
YogaKit/Tests/YogaKitTests.m
emilsjolander (Migrated from github.com) commented 2017-01-10 06:42:59 -08:00
Copy Link

revert

revert
YogaKit/YGLayout+Private.h
@@ -11,2 +12,3 @@
@interface YGLayout (Private)
@interface YGLayout ()
emilsjolander (Migrated from github.com) commented 2017-01-10 06:45:00 -08:00
Copy Link

Why did you remove the (Private)?

Why did you remove the `(Private)`?
yoga/Yoga.c
emilsjolander (Migrated from github.com) commented 2017-01-10 06:47:16 -08:00
Copy Link

The tests should test that setting node.marginHorizontal = 10 results in a a node with marginLeft and marginRight set. However there are already tests for this in Yoga so not sure what the value is in adding tests for this in yogakit as well.

The tests should test that setting `node.marginHorizontal = 10` results in a a node with `marginLeft` and `marginRight` set. However there are already tests for this in Yoga so not sure what the value is in adding tests for this in yogakit as well.
hartbit (Migrated from github.com) reviewed 2017-01-10 11:46:01 -08:00
YogaKit/Tests/YogaKitTests.m
@@ -387,0 +408,4 @@
view.yoga.bottom = 4;
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeBottom).value, 4);
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeBottom).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.bottom, 4);
hartbit (Migrated from github.com) commented 2017-01-10 11:46:01 -08:00
Copy Link

I started with tests which tested the layout, but it felt like a duplication of tests in the C++ Yoga tests. I felt better with tests which read like what they are testing. But if you feel strongly about it, I don't mind updating the tests :)

I started with tests which tested the layout, but it felt like a duplication of tests in the C++ Yoga tests. I felt better with tests which read like what they are testing. But if you feel strongly about it, I don't mind updating the tests :)
hartbit (Migrated from github.com) reviewed 2017-01-10 11:51:46 -08:00
yoga/Yoga.c
hartbit (Migrated from github.com) commented 2017-01-10 11:51:46 -08:00
Copy Link
  1. Change all the tests where we call a getter on YGEdgeAll, YGEdgeHorizontal, YGEdgeVertical to access their respective representations.

But what would the getter of marginHorizontal do? Right now, I'm not aware of any function in Yoga which I can call which returns the non-computed value. Should I create it? But what should its name be? I wrote these tests as a result of removing the assert because with the assert, the getters of multi-edge shorthands crashed.

> 2. Change all the tests where we call a getter on YGEdgeAll, YGEdgeHorizontal, YGEdgeVertical to access their respective representations. But what would the getter of `marginHorizontal` do? Right now, I'm not aware of any function in Yoga which I can call which returns the non-computed value. Should I create it? But what should its name be? I wrote these tests as a result of removing the assert because with the assert, the getters of multi-edge shorthands crashed.
emilsjolander (Migrated from github.com) reviewed 2017-01-10 11:56:31 -08:00
yoga/Yoga.c
emilsjolander (Migrated from github.com) commented 2017-01-10 11:56:31 -08:00
Copy Link

Let's just not have a getter for marginHorizontal and similar for now. We don't allow this is any other versions anyways so if we were to change how those work it should not be part of this PR anyways.

Let's just not have a getter for `marginHorizontal` and similar for now. We don't allow this is any other versions anyways so if we were to change how those work it should not be part of this PR anyways.
hartbit (Migrated from github.com) reviewed 2017-01-10 12:12:26 -08:00
yoga/Yoga.c
hartbit (Migrated from github.com) commented 2017-01-10 12:12:26 -08:00
Copy Link

I think that would negatively affect the API in Swift and for users of Objective-C which want to use dot-notation. The whole point of the PR I wrote which was recently merged was to make the APIs cleaner by using properties. Reverting that for multi-edge shorthands looks really weird to me:

let view = UIView()
view.yoga.isEnabled = true
view.yoga.marginLeft = 10
view.yoga.setMarginVertical(20)
view.yoga.paddingTop = 15
view.yoga.paddingBottom = 10
view.yoga.setPaddingHorizontal(10)
I think that would negatively affect the API in Swift and for users of Objective-C which want to use dot-notation. The whole point of the PR I wrote which was recently merged was to make the APIs cleaner by using properties. Reverting that for multi-edge shorthands looks really weird to me: ``` let view = UIView() view.yoga.isEnabled = true view.yoga.marginLeft = 10 view.yoga.setMarginVertical(20) view.yoga.paddingTop = 15 view.yoga.paddingBottom = 10 view.yoga.setPaddingHorizontal(10) ```
d16r (Migrated from github.com) reviewed 2017-01-10 13:05:13 -08:00
yoga/Yoga.c
d16r (Migrated from github.com) commented 2017-01-10 13:05:13 -08:00
Copy Link

view.yoga.marginVertical = 20 is just syntax-sugar [view.yoga setMarginVertical:20]. So (for ObjC) we can add - (void)setMarginVertical: and still use the dot-syntax.

Not sure how it translates for the Swift API, though.

`view.yoga.marginVertical = 20` is just syntax-sugar `[view.yoga setMarginVertical:20]`. So (for ObjC) we can add `- (void)setMarginVertical:` and still use the dot-syntax. Not sure how it translates for the Swift API, though.
d16r (Migrated from github.com) reviewed 2017-01-10 13:08:25 -08:00
yoga/Yoga.c
d16r (Migrated from github.com) commented 2017-01-10 13:08:25 -08:00
Copy Link

I also wouldn't be against removing the computed edges from YogaKit

I also wouldn't be against removing the computed edges from `YogaKit`
hartbit (Migrated from github.com) reviewed 2017-01-10 13:44:01 -08:00
yoga/Yoga.c
hartbit (Migrated from github.com) commented 2017-01-10 13:44:01 -08:00
Copy Link

view.yoga.marginVertical = 20 is just syntax-sugar [view.yoga setMarginVertical:20]. So (for ObjC) we can add - (void)setMarginVertical: and still use the dot-syntax.

It does work, but (1) we loose auto-completion and (2) it's kind of an anti-pattern to use dot-notation on methods.

Not sure how it translates for the Swift API, though.

The code I provided above is how it translates to Swift: we are forced to use the method syntax.

Sorry if I come across as vindicative. I'm just trying to express that I feel this would be a net-loss for the Objective-C and more importantly the Swift API. Can we try to think of solutions which would allow us the retain the getters? Here are some ideas to help me/us think about it:

We could implement the getters to return a value when the short-hand edges have the same value, and return undefined instead:

let view = UIView()
view.yoga.marginHorizontal = 10
print(view.yoga.marginHorizontal) // 10
view.yoga.marginStart = 10
print(view.yoga.marginHorizontal) // YGUndefined
view.yoga.marginEnd = 10
print(view.yoga.marginHorizontal) // 10
view.yoga.marginStart = 15
print(view.yoga.marginHorizontal) // YGUndefined

Any other ideas?

> view.yoga.marginVertical = 20 is just syntax-sugar [view.yoga setMarginVertical:20]. So (for ObjC) we can add - (void)setMarginVertical: and still use the dot-syntax. It does work, but (1) we loose auto-completion and (2) it's kind of an anti-pattern to use dot-notation on methods. > Not sure how it translates for the Swift API, though. The code I provided above is how it translates to Swift: we are forced to use the method syntax. Sorry if I come across as vindicative. I'm just trying to express that I feel this would be a net-loss for the Objective-C and more importantly the Swift API. Can we try to think of solutions which would allow us the retain the getters? Here are some ideas to help me/us think about it: We could implement the getters to return a value when the short-hand edges have the same value, and return undefined instead: ``` let view = UIView() view.yoga.marginHorizontal = 10 print(view.yoga.marginHorizontal) // 10 view.yoga.marginStart = 10 print(view.yoga.marginHorizontal) // YGUndefined view.yoga.marginEnd = 10 print(view.yoga.marginHorizontal) // 10 view.yoga.marginStart = 15 print(view.yoga.marginHorizontal) // YGUndefined ``` Any other ideas?
emilsjolander commented 2017-01-12 02:12:39 -08:00 (Migrated from github.com)
Copy Link

@hartbit What is the status of this?

@hartbit What is the status of this?
❤️ 1
hartbit commented 2017-01-12 12:03:25 -08:00 (Migrated from github.com)
Copy Link

@emilsjolander I am waiting for your feedback on my reply on the comment above to see what we do about the getters :)

@emilsjolander I am waiting for your feedback on my reply on the comment above to see what we do about the getters :)
emilsjolander (Migrated from github.com) reviewed 2017-01-13 06:48:57 -08:00
yoga/Yoga.c
emilsjolander (Migrated from github.com) commented 2017-01-13 06:48:57 -08:00
Copy Link

@hartbit I agree we should keep these as being properties. I like the suggestion of returning YGUndefined when the semantics are not defined. I suggest for this pull request to always return YGUndefined for the property getters of YGEdgeAll/Horizontal/Vertical. That way we are able to land the rest of this PR. In a follow up issue / PR we can discuss improving this by implementing what you suggested above. How does that sound?

@hartbit I agree we should keep these as being properties. I like the suggestion of returning `YGUndefined` when the semantics are not defined. I suggest for this pull request to always return `YGUndefined` for the property getters of YGEdgeAll/Horizontal/Vertical. That way we are able to land the rest of this PR. In a follow up issue / PR we can discuss improving this by implementing what you suggested above. How does that sound?
hartbit (Migrated from github.com) reviewed 2017-01-13 08:09:16 -08:00
YogaKit/Tests/YogaKitTests.m
hartbit (Migrated from github.com) commented 2017-01-13 08:09:16 -08:00
Copy Link

Done

Done
hartbit (Migrated from github.com) reviewed 2017-01-13 16:16:55 -08:00
YogaKit/YGLayout+Private.h
@@ -11,2 +12,3 @@
@interface YGLayout (Private)
@interface YGLayout ()
hartbit (Migrated from github.com) commented 2017-01-13 16:16:55 -08:00
Copy Link

That's required because the private header now contains a property (node, which needs to be accessed in the tests) and which isn't synthesised if declared in a category, compared to a private interface definition.

That's required because the private header now contains a property (`node`, which needs to be accessed in the tests) and which isn't synthesised if declared in a category, compared to a private interface definition.
hartbit commented 2017-01-13 16:17:23 -08:00 (Migrated from github.com)
Copy Link

@emilsjolander All done for me. I did as you suggested and made all shorthand edge getters return YGUndefined for now :)

@emilsjolander All done for me. I did as you suggested and made all shorthand edge getters return `YGUndefined` for now :)
emilsjolander commented 2017-01-15 14:03:02 -08:00 (Migrated from github.com)
Copy Link

@hartbit travis is failing

@hartbit travis is failing
hartbit commented 2017-01-15 23:54:27 -08:00 (Migrated from github.com)
Copy Link

@emilsjolander My bad. It's fixed now. Had forgotten to remove the tests I had added to prove that the removable of the ASSERT was behaving as expected. The tests are not necessary now that I put the ASSERT back.

@emilsjolander My bad. It's fixed now. Had forgotten to remove the tests I had added to prove that the removable of the ASSERT was behaving as expected. The tests are not necessary now that I put the ASSERT back.
facebook-github-bot commented 2017-01-16 10:13:48 -08:00 (Migrated from github.com)
Copy Link

@emilsjolander has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@emilsjolander has imported this pull request. If you are a Facebook employee, you can view this diff [on Phabricator](https://phabricator.intern.facebook.com/D4421504).

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
Algorithm
Build and Infrastructure
C#
C++ Enhancement
CLA Signed
Discussion
Documentation
Feature Request
Java
JavaScript
Merged
Question
Reverted
Shared with Meta
Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
YogaKit
YogaLayout ViewGroup
bug
dependencies
Pull requests that update a dependency file
duplicate
enhancement
fb-exported
fh:direct-merge-enabled
invalid
ruby
Pull requests that update Ruby code
wontfix
No Label
CLA Signed
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
DaddyFrosty
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#328
No description provided.
Delete Branch "yogakit-edge-bugs"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?