[YogaKit] Improved the objective-c and swift api by hartbit · Pull Request #322 · react/yoga

hartbit · 2017-01-05T23:23:39Z

Compared to what was planned, I added the overflow value which seemed missing. I had to modify the implementation a bit for all values which are backed by a YGValue, but we should probably enable the pixel dimensions in Objective-C and Swift somehow later.

emilsjolander

Just a few comments. Mostly this looks very good and I'm pretty confident we can merge this later today. Thanks!

emilsjolander · 2017-01-06T09:27:00Z

+
+@interface YGLayout ()
+
+@property (nonatomic, weak, readonly) UIView* view;


UIView *view

emilsjolander · 2017-01-06T09:36:08Z

 YG_EXTERN_C_BEGIN

 #define YGFlexDirectionCount 4
+#ifndef NS_ENUM


Could we make this generate something short (less duplicated) ? Maybe by creating our own YG_ENUM macro which expands to one of the two alternatives.

emilsjolander · 2017-01-06T09:38:07Z

+#import <yoga/Yoga.h>
+#import <UIKit/UIKit.h>
+
+extern YGValue YGValueMake(CGFloat value, YGUnit unit) {


This function seems unused. Could you remove it?

emilsjolander · 2017-01-06T09:38:21Z

+
+#import <UIKit/UIKit.h>
+#import <yoga/YGEnums.h>
+#import <yoga/YGValue.h>


This import seems unused, could you remove it?

emilsjolander · 2017-01-06T09:39:07Z

@@ -0,0 +1,19 @@
+/**


Pulling this out into a separate file seems to not be needed for this PR. Could you revert this change?

hartbit · 2017-01-06T14:08:34Z

I implemented all your suggestions 😄 Thanks 👍

d16r · 2017-01-06T14:14:52Z

- Mark that a view's layout needs to be recalculated. Only works for leaf views.
- */
- (void)yg_markDirty;
+@property (nonatomic, readonly, retain) YGLayout *yoga;


We should change this to strong

http://nshipster.com/associated-objects/

facebook-github-bot · 2017-01-06T14:15:24Z

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

d16r · 2017-01-06T14:17:47Z

+
+- (instancetype)initWithView:(UIView*)view
+{
+  if ([super init]) {


Nit, we should be assigning this to self, i.e.

self = [super init]

I know this isn't technically required, but I think it's good to keep the standard. More about that here: https://www.cocoawithlove.com/2009/04/what-does-it-mean-when-you-assign-super.html

emilsjolander · 2017-01-06T14:30:32Z

@hartbit Don't worry about it. I'll fix this while mergin

d16r · 2017-01-06T14:47:36Z

Awesome job @hartbit!! I'm so excited to land this!

Align C# implementation with YogaKit react#322

Summary: Compared to what was planned, I added the `overflow` value which seemed missing. I had to modify the implementation a bit for all values which are backed by a `YGValue`, but we should probably enable the pixel dimensions in Objective-C and Swift somehow later. Closes react/yoga#322 Reviewed By: dshahidehpour Differential Revision: D4386906 Pulled By: emilsjolander fbshipit-source-id: 05ac0e571ef3a8ff0be31469e449a7b23f102218

Summary: Align C# implementation with YogaKit #322 Closes #327 Reviewed By: emilsjolander Differential Revision: D4390687 Pulled By: splhack fbshipit-source-id: 28c87a45898fcd958a422d5e254ead0ec00d3562

Summary: Compared to what was planned, I added the `overflow` value which seemed missing. I had to modify the implementation a bit for all values which are backed by a `YGValue`, but we should probably enable the pixel dimensions in Objective-C and Swift somehow later. Closes react/yoga#322 Reviewed By: cosmin1123 Differential Revision: D4391434 Pulled By: emilsjolander fbshipit-source-id: e33f6f7b2bbaad29553100b7a5bb424496372110

Summary: Compared to what was planned, I added the `overflow` value which seemed missing. I had to modify the implementation a bit for all values which are backed by a `YGValue`, but we should probably enable the pixel dimensions in Objective-C and Swift somehow later. Closes #322 Reviewed By: cosmin1123 Differential Revision: D4391434 Pulled By: emilsjolander fbshipit-source-id: e33f6f7b2bbaad29553100b7a5bb424496372110

ericvicenti · 2017-01-09T10:45:20Z

It looks like the import of this commit into react-native causes CI tests to fail. Can somebody help fix this ASAP?

https://travis-ci.org/facebook/react-native/builds/190054212

emilsjolander · 2017-01-09T10:53:07Z

@ericvicenti looking now

emilsjolander · 2017-01-09T12:06:40Z

@ericvicenti Found issue. Pushing fix within next few hours

hartbit · 2017-01-09T12:32:18Z

@emilsjolander What was the problem?

emilsjolander · 2017-01-09T16:46:01Z

0acb620

hartbit · 2017-01-09T17:06:21Z

Ah, of course!

Summary: Compared to what was planned, I added the `overflow` value which seemed missing. I had to modify the implementation a bit for all values which are backed by a `YGValue`, but we should probably enable the pixel dimensions in Objective-C and Swift somehow later. Closes react/yoga#322 Reviewed By: dshahidehpour Differential Revision: D4386906 Pulled By: emilsjolander fbshipit-source-id: 05ac0e571ef3a8ff0be31469e449a7b23f102218

Summary: Compared to what was planned, I added the `overflow` value which seemed missing. I had to modify the implementation a bit for all values which are backed by a `YGValue`, but we should probably enable the pixel dimensions in Objective-C and Swift somehow later. Closes react/yoga#322 Reviewed By: cosmin1123 Differential Revision: D4391434 Pulled By: emilsjolander fbshipit-source-id: e33f6f7b2bbaad29553100b7a5bb424496372110

improved the objective-c and swift api

35ceb97

facebook-github-bot added the CLA Signed label Jan 5, 2017

hartbit mentioned this pull request Jan 5, 2017

WIP: Improve the Objective-C and Swift bridge #289

Closed

emilsjolander suggested changes Jan 6, 2017

View reviewed changes

review improvements

ec28cac

emilsjolander approved these changes Jan 6, 2017

View reviewed changes

d16r reviewed Jan 6, 2017

View reviewed changes

splhack added a commit to splhack/css-layout that referenced this pull request Jan 7, 2017

[C#] Add spacing properties

dc11197

Align C# implementation with YogaKit react#322

This was referenced Jan 7, 2017

[C#] Add spacing properties #327

Closed

Xamarin iOS Support #324

Closed

facebook-github-bot closed this in 8d320ce Jan 7, 2017

hartbit deleted the improve-yogakit-api branch January 7, 2017 16:31

guidomb mentioned this pull request Jan 9, 2017

Carthage and CocoaPods support #309

Closed

guidomb mentioned this pull request Jan 26, 2017

Add Carthage & CocoaPods support. #352

Closed

8 tasks


		@interface YGLayout ()

		@property (nonatomic, weak, readonly) UIView* view;

Conversation

hartbit commented Jan 5, 2017

Uh oh!

emilsjolander left a comment

Choose a reason for hiding this comment

Uh oh!

emilsjolander Jan 6, 2017

Choose a reason for hiding this comment

Uh oh!

emilsjolander Jan 6, 2017

Choose a reason for hiding this comment

Uh oh!

emilsjolander Jan 6, 2017

Choose a reason for hiding this comment

Uh oh!

emilsjolander Jan 6, 2017

Choose a reason for hiding this comment

Uh oh!

emilsjolander Jan 6, 2017

Choose a reason for hiding this comment

Uh oh!

hartbit commented Jan 6, 2017

Uh oh!

d16r Jan 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jan 6, 2017

Uh oh!

d16r Jan 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emilsjolander commented Jan 6, 2017

Uh oh!

d16r commented Jan 6, 2017

Uh oh!

ericvicenti commented Jan 9, 2017

Uh oh!

emilsjolander commented Jan 9, 2017

Uh oh!

emilsjolander commented Jan 9, 2017

Uh oh!

hartbit commented Jan 9, 2017

Uh oh!

emilsjolander commented Jan 9, 2017

Uh oh!

hartbit commented Jan 9, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

d16r Jan 6, 2017 •

edited

Loading

d16r Jan 6, 2017 •

edited

Loading