Skip to content
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

Add YGLayoutGetMargin #335

Closed
wants to merge 4 commits into from
Closed

Add YGLayoutGetMargin #335

wants to merge 4 commits into from

Conversation

arcanis
Copy link
Contributor

@arcanis arcanis commented Jan 12, 2017

Fix #326. I'll open another PR once this one gets accepted to add support for YGLayoutGetBorder 👌

@emilsjolander
Copy link
Contributor

Could you add the c# and java bindings for this as well? Should be fairly straight forward as it will look exactly like the padding ones.

@arcanis
Copy link
Contributor Author

arcanis commented Jan 12, 2017

Eh eh, I was working on them, they've just been pushed :) I haven't been able to test them yet, will Travis run the tests for those interfaces as well ?

@emilsjolander
Copy link
Contributor

Yup it will :) thanks!

@arcanis
Copy link
Contributor Author

arcanis commented Jan 12, 2017

Seems good, tests are now passing

@arcanis
Copy link
Contributor Author

arcanis commented Jan 12, 2017

@emilsjolander I started looking at how I can implement YGNodeLayoutGetBorder, and actually there will be a bit more work than I thought, since such a function would need to go through the same EDGE_START / EDGE_END check than its two siblings. Would you mind if I implemented it in this PR, since the code will be quite similar?

@emilsjolander
Copy link
Contributor

@arcanis I would rather it was a PR based on top of this one. Trying to move to smaller pull requests where possible as it makes it much easier to grok the code. Thanks 👍

@arcanis
Copy link
Contributor Author

arcanis commented Jan 12, 2017

Ok 👍

Copy link
Contributor

@emilsjolander emilsjolander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests to YogaNodeTest.cs and YogaNodeTest.java. These will be similar to the tests already there for layoutPadding.

@@ -333,6 +333,12 @@ public void SetPosition(YogaEdge edge, YogaValue value)
}
}

[Obsolete("use LayoutMargin properties")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't add an Obsolete property :p all these will be removed soon enough.

@emilsjolander
Copy link
Contributor

@arcanis Looks good now, thanks!

@facebook-github-bot
Copy link
Contributor

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

facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Jan 15, 2017
Summary:
Fix #326. I'll open another PR once this one gets accepted to add support for `YGLayoutGetBorder` 👌
Closes facebook/yoga#335

Reviewed By: gkassabli

Differential Revision: D4409399

Pulled By: emilsjolander

fbshipit-source-id: 8153f6701cab60b55a485f6d2e0b9f7767481090
nicktate pushed a commit to nicktate/react-native that referenced this pull request Jan 19, 2017
Summary:
Fix facebook#326. I'll open another PR once this one gets accepted to add support for `YGLayoutGetBorder` 👌
Closes facebook/yoga#335

Reviewed By: gkassabli

Differential Revision: D4409399

Pulled By: emilsjolander

fbshipit-source-id: 8153f6701cab60b55a485f6d2e0b9f7767481090
facebook-github-bot pushed a commit that referenced this pull request Jan 26, 2017
Summary:
Followup of #335, fix #326. This commit add the `YGLayoutGetBorder(node, edge)` function, which correctly takes RTL/LTR into account when resolving `EDGE_START` & `EDGE_END`.
Closes #344

Reviewed By: dshahidehpour

Differential Revision: D4459950

Pulled By: emilsjolander

fbshipit-source-id: b57eb7a5b1c181a364913c3200a3794a2b7b31a6
vincentriemer pushed a commit to vincentriemer/yoga-dom that referenced this pull request May 9, 2018
Summary:
Fix #326. I'll open another PR once this one gets accepted to add support for `YGLayoutGetBorder` 👌
Closes facebook/yoga#335

Reviewed By: gkassabli

Differential Revision: D4409399

Pulled By: emilsjolander

fbshipit-source-id: 8153f6701cab60b55a485f6d2e0b9f7767481090
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants