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 +525 -812
50 changed files with 525 additions and 812 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 468191e160 - Show all commits

168
YogaKit/Tests/YogaKitTests.m
View File

@@ -9,6 +9,7 @@
#import <XCTest/XCTest.h> #import <XCTest/XCTest.h>
#import <YogaKit/UIView+Yoga.h> #import <YogaKit/UIView+Yoga.h>
#import <YogaKit/YGLayout+Private.h>
#import <yoga/Yoga.h> #import <yoga/Yoga.h>
@interface YogaKitTests : XCTestCase @interface YogaKitTests : XCTestCase
@@ -386,13 +387,170 @@
- (void)testPositionalPropertiesWork - (void)testPositionalPropertiesWork
{ {
YGNodeRef node = YGNodeNew(); UIView *view = [[UIView alloc] initWithFrame:CGRectZero];
YGNodeStyleSetPosition(node, YGEdgeLeft, 1);
XCTAssertEqual(1, YGNodeStyleGetPosition(node, YGEdgeLeft).value); view.yoga.left = 1;
XCTAssertEqual(YGUnitPixel, YGNodeStyleGetPosition(node, YGEdgeLeft).unit); XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeLeft).value, 1);
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeLeft).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.left, 1);
YGNodeFree(node); view.yoga.right = 2;
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeRight).value, 2);
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeRight).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.right, 2);
view.yoga.top = 3;
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeTop).value, 3);
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeTop).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.top, 3);
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 commented 2017-01-09 14:42:30 -08:00 (Migrated from github.com)
Review
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 commented 2017-01-09 14:53:05 -08:00 (Migrated from github.com)
Review
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.
d16r commented 2017-01-10 06:13:09 -08:00 (Migrated from github.com)
Review
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!
hartbit commented 2017-01-10 11:46:01 -08:00 (Migrated from github.com)
Review
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 :)
view.yoga.start = 5;
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeStart).value, 5);
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeStart).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.start, 5);
view.yoga.end = 6;
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeEnd).value, 6);
XCTAssertEqual(YGNodeStyleGetPosition(view.yoga.node, YGEdgeEnd).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.end, 6);
}
- (void)testMarginPropertiesWork
{
UIView *view = [[UIView alloc] initWithFrame:CGRectZero];
view.yoga.marginLeft = 1;
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeLeft).value, 1);
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeLeft).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.marginLeft, 1);
view.yoga.marginRight = 2;
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeRight).value, 2);
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeRight).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.marginRight, 2);
view.yoga.marginTop = 3;
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeTop).value, 3);
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeTop).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.marginTop, 3);
view.yoga.marginBottom = 4;
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeBottom).value, 4);
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeBottom).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.marginBottom, 4);
view.yoga.marginStart = 5;
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeStart).value, 5);
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeStart).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.marginStart, 5);
view.yoga.marginEnd = 6;
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeEnd).value, 6);
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeEnd).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.marginEnd, 6);
view.yoga.marginHorizontal = 7;
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeHorizontal).value, 7);
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeHorizontal).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.marginHorizontal, 7);
view.yoga.marginVertical = 8;
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeVertical).value, 8);
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeVertical).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.marginVertical, 8);
view.yoga.margin = 9;
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeAll).value, 9);
XCTAssertEqual(YGNodeStyleGetMargin(view.yoga.node, YGEdgeAll).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.margin, 9);
}
- (void)testPaddingPropertiesWork
{
UIView *view = [[UIView alloc] initWithFrame:CGRectZero];
view.yoga.paddingLeft = 1;
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeLeft).value, 1);
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeLeft).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.paddingLeft, 1);
view.yoga.paddingRight = 2;
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeRight).value, 2);
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeRight).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.paddingRight, 2);
view.yoga.paddingTop = 3;
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeTop).value, 3);
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeTop).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.paddingTop, 3);
view.yoga.paddingBottom = 4;
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeBottom).value, 4);
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeBottom).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.paddingBottom, 4);
view.yoga.paddingStart = 5;
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeStart).value, 5);
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeStart).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.paddingStart, 5);
view.yoga.paddingEnd = 6;
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeEnd).value, 6);
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeEnd).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.paddingEnd, 6);
view.yoga.paddingHorizontal = 7;
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeHorizontal).value, 7);
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeHorizontal).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.paddingHorizontal, 7);
view.yoga.paddingVertical = 8;
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeVertical).value, 8);
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeVertical).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.paddingVertical, 8);
view.yoga.padding = 9;
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeAll).value, 9);
XCTAssertEqual(YGNodeStyleGetPadding(view.yoga.node, YGEdgeAll).unit, YGUnitPixel);
XCTAssertEqual(view.yoga.padding, 9);
}
- (void)testBorderWidthPropertiesWork
{
UIView *view = [[UIView alloc] initWithFrame:CGRectZero];
view.yoga.borderLeftWidth = 1;
XCTAssertEqual(YGNodeStyleGetBorder(view.yoga.node, YGEdgeLeft), 1);
XCTAssertEqual(view.yoga.borderLeftWidth, 1);
view.yoga.borderRightWidth = 2;
XCTAssertEqual(YGNodeStyleGetBorder(view.yoga.node, YGEdgeRight), 2);
XCTAssertEqual(view.yoga.borderRightWidth, 2);
view.yoga.borderTopWidth = 3;
XCTAssertEqual(YGNodeStyleGetBorder(view.yoga.node, YGEdgeTop), 3);
XCTAssertEqual(view.yoga.borderTopWidth, 3);
view.yoga.borderBottomWidth = 4;
XCTAssertEqual(YGNodeStyleGetBorder(view.yoga.node, YGEdgeBottom), 4);
XCTAssertEqual(view.yoga.borderBottomWidth, 4);
view.yoga.borderStartWidth = 5;
XCTAssertEqual(YGNodeStyleGetBorder(view.yoga.node, YGEdgeStart), 5);
XCTAssertEqual(view.yoga.borderStartWidth, 5);
view.yoga.borderEndWidth = 6;
XCTAssertEqual(YGNodeStyleGetBorder(view.yoga.node, YGEdgeEnd), 6);
XCTAssertEqual(view.yoga.borderEndWidth, 6);
view.yoga.borderWidth = 7;
XCTAssertEqual(YGNodeStyleGetBorder(view.yoga.node, YGEdgeAll), 7);
XCTAssertEqual(view.yoga.borderWidth, 7);
} }
@end @end

51
tests/YGEdgeTest.cpp
View File

@@ -161,3 +161,54 @@ TEST(YogaTest, all_overridden) {
YGNodeFreeRecursive(root); YGNodeFreeRecursive(root);
} }
TEST(YogaTest, get_style_horizontal) {
const YGNodeRef node = YGNodeNew();
YGNodeStyleSetMargin(node, YGEdgeLeft, 10);
YGNodeStyleSetMargin(node, YGEdgeRight, 10);
YGNodeStyleSetMargin(node, YGEdgeStart, 10);
YGNodeStyleSetMargin(node, YGEdgeEnd, 10);
ASSERT_FLOAT_EQ(0, YGNodeStyleGetMargin(node, YGEdgeHorizontal).value);
YGNodeStyleSetMargin(node, YGEdgeAll, 15);
ASSERT_FLOAT_EQ(15, YGNodeStyleGetMargin(node, YGEdgeHorizontal).value);
YGNodeStyleSetMargin(node, YGEdgeHorizontal, 20);
ASSERT_FLOAT_EQ(20, YGNodeStyleGetMargin(node, YGEdgeHorizontal).value);
YGNodeFreeRecursive(node);
}
TEST(YogaTest, get_style_vertical) {
const YGNodeRef node = YGNodeNew();
YGNodeStyleSetMargin(node, YGEdgeTop, 10);
YGNodeStyleSetMargin(node, YGEdgeBottom, 10);
ASSERT_FLOAT_EQ(0, YGNodeStyleGetMargin(node, YGEdgeVertical).value);
YGNodeStyleSetMargin(node, YGEdgeAll, 15);
ASSERT_FLOAT_EQ(15, YGNodeStyleGetMargin(node, YGEdgeVertical).value);
YGNodeStyleSetMargin(node, YGEdgeVertical, 20);
ASSERT_FLOAT_EQ(20, YGNodeStyleGetMargin(node, YGEdgeVertical).value);
YGNodeFreeRecursive(node);
}
TEST(YogaTest, get_style_all) {
const YGNodeRef node = YGNodeNew();
YGNodeStyleSetMargin(node, YGEdgeLeft, 10);
YGNodeStyleSetMargin(node, YGEdgeRight, 10);
YGNodeStyleSetMargin(node, YGEdgeStart, 10);
YGNodeStyleSetMargin(node, YGEdgeEnd, 10);
YGNodeStyleSetMargin(node, YGEdgeTop, 10);
YGNodeStyleSetMargin(node, YGEdgeBottom, 10);
ASSERT_FLOAT_EQ(0, YGNodeStyleGetMargin(node, YGEdgeAll).value);
YGNodeStyleSetMargin(node, YGEdgeAll, 15);
ASSERT_FLOAT_EQ(15, YGNodeStyleGetMargin(node, YGEdgeAll).value);
YGNodeFreeRecursive(node);
}

2
yoga/Yoga.c
View File

@@ -229,8 +229,6 @@ static YGLogger gLogger = &YGDefaultLog;
static inline const YGValue *YGComputedEdgeValue(const YGValue edges[YGEdgeCount], static inline const YGValue *YGComputedEdgeValue(const YGValue edges[YGEdgeCount],
d16r commented 2017-01-09 14:40:20 -08:00 (Migrated from github.com)
Review
Copy Link

@emilsjolander any idea why this is needed?

@emilsjolander any idea why this is needed?
emilsjolander commented 2017-01-10 03:34:52 -08:00 (Migrated from github.com)
Review
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 commented 2017-01-10 06:20:57 -08:00 (Migrated from github.com)
Review
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 commented 2017-01-10 06:47:16 -08:00 (Migrated from github.com)
Review
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 commented 2017-01-10 11:51:46 -08:00 (Migrated from github.com)
Review
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 commented 2017-01-10 11:56:31 -08:00 (Migrated from github.com)
Review
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 commented 2017-01-10 12:12:26 -08:00 (Migrated from github.com)
Review
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 commented 2017-01-10 13:05:13 -08:00 (Migrated from github.com)
Review
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 commented 2017-01-10 13:08:25 -08:00 (Migrated from github.com)
Review
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 commented 2017-01-10 13:44:01 -08:00 (Migrated from github.com)
Review
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-13 06:48:57 -08:00 (Migrated from github.com)
Review
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?
const YGEdge edge, const YGEdge edge,
const YGValue *const defaultValue) { const YGValue *const defaultValue) {
YG_ASSERT(edge <= YGEdgeEnd, "Cannot get computed value of multi-edge shorthands");
if (edges[edge].unit != YGUnitUndefined) { if (edges[edge].unit != YGUnitUndefined) {
return &edges[edge]; return &edges[edge];
} }