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

[chsarp] Add YogaKit Shared and iOS version #336

Closed
wants to merge 4 commits into from

Conversation

rmarinho
Copy link
Contributor

@rmarinho rmarinho commented Jan 12, 2017

Using shared code for reuse in other platforms based on iOS native implementation.

Adds YogaKit sample.

Adds YogaKit tests (same as objc). (3 tests failing)

YogaKitTest : 80 ms
Facebook.YogaKit.iOS.Tests.exe : 81 ms
Tests run: 11 Passed: 8 Inconclusive: 0 Failed: 3 Ignored: 1

Since we don't have extension properties we need to go with a extension method to get access to the YogaLayout .

I m also not sure this is leak free yet, would love some help with testing and feedback about view/node lifecycle

…ne. Using shared code for reuse in other platforms. Adds YogaKit sample
@rmarinho rmarinho changed the title [chsarp] Add YogaKit shared and Xamarin.iOS [chsarp] Add YogaKit Shared and iOS version Jan 12, 2017
@emilsjolander
Copy link
Contributor

I'm worried about this copying a lot of logic from YogaKit. Can Xamarin interop with the obj-c code in YogaKit instead?

@rmarinho
Copy link
Contributor Author

This is so we can share YogaKit implementation to other platforms like android and uwp.

I could do binding for the the YogaKit lib ... but that will only work for iOS.

@emilsjolander
Copy link
Contributor

Aren't you tying yourself to iOS by doing [DllImport("/usr/lib/libobjc.dylib")] though?

@rmarinho
Copy link
Contributor Author

That file only lives in the YogaKit iOS project, it's a partial class , so the methods are only added when you are running in that platform.

@emilsjolander
Copy link
Contributor

Ok, makes sense. Thanks for taking the time to explain 👍 I'll let @splhack give the final ok on the c# code but it looks good to me.

@splhack
Copy link
Contributor

splhack commented Jan 12, 2017

I feel maybe we can have another approach for this. For example, this line from test.

root.Yoga().AlignItems = YogaAlign.Center;

It would be like next line if we can use YogaNode class directly.

root.Yoga.AlignItems = YogaAlign.Center;

I'll try to hack around YogaKit based on this PR later.

@rmarinho
Copy link
Contributor Author

rmarinho commented Jan 12, 2017

@splhack yeah maybe C# 7 or 8 will give us Extension Properties .

But i agree with you and open to a solution :)

@rmarinho
Copy link
Contributor Author

Added tests

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

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.

4 participants