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

[YogaKit] Improved the objective-c and swift api #322

Closed
hartbit wants to merge 2 commits from improve-yogakit-api into master
pull from: improve-yogakit-api
merge into: DaddyFrosty:master
Conversation 14 Commits 2 Files Changed 13 +769 -761
hartbit commented 2017-01-05 15:23:39 -08:00 (Migrated from github.com)
Copy Link

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.

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 (Migrated from github.com) requested changes 2017-01-06 01:39:58 -08:00
emilsjolander (Migrated from github.com) left a comment
Copy Link

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

Just a few comments. Mostly this looks very good and I'm pretty confident we can merge this later today. Thanks!
YogaKit/YGLayout.h
@@ -0,0 +1,114 @@
/**
emilsjolander (Migrated from github.com) commented 2017-01-06 01:38:21 -08:00
Copy Link

This import seems unused, could you remove it?

This import seems unused, could you remove it?
YogaKit/YGLayout.m
@@ -0,0 +1,380 @@
/**
emilsjolander (Migrated from github.com) commented 2017-01-06 01:27:00 -08:00
Copy Link

UIView *view

UIView *view
emilsjolander (Migrated from github.com) commented 2017-01-06 01:38:07 -08:00
Copy Link

This function seems unused. Could you remove it?

This function seems unused. Could you remove it?
yoga/YGEnums.h
@@ -14,29 +14,29 @@
YG_EXTERN_C_BEGIN
emilsjolander (Migrated from github.com) commented 2017-01-06 01:36:08 -08:00
Copy Link

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.

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.
yoga/YGValue.h
emilsjolander (Migrated from github.com) commented 2017-01-06 01:39:07 -08:00
Copy Link

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

Pulling this out into a separate file seems to not be needed for this PR. Could you revert this change?
hartbit commented 2017-01-06 06:08:34 -08:00 (Migrated from github.com)
Copy Link

I implemented all your suggestions 😄 Thanks 👍

I implemented all your suggestions 😄 Thanks 👍
emilsjolander (Migrated from github.com) approved these changes 2017-01-06 06:10:06 -08:00
d16r (Migrated from github.com) reviewed 2017-01-06 06:14:52 -08:00
YogaKit/UIView+Yoga.h
d16r (Migrated from github.com) commented 2017-01-06 06:14:52 -08:00
Copy Link

We should change this to strong

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

We should change this to `strong` http://nshipster.com/associated-objects/
facebook-github-bot commented 2017-01-06 06:15:24 -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/D4386906).
d16r (Migrated from github.com) reviewed 2017-01-06 06:17:47 -08:00
YogaKit/YGLayout.m
@@ -0,0 +93,4 @@
- (instancetype)initWithView:(UIView*)view
{
if ([super init]) {
d16r (Migrated from github.com) commented 2017-01-06 06:17:47 -08:00
Copy Link

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

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 commented 2017-01-06 06:30:32 -08:00 (Migrated from github.com)
Copy Link

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

@hartbit Don't worry about it. I'll fix this while mergin
d16r commented 2017-01-06 06:47:36 -08:00 (Migrated from github.com)
Copy Link

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

Awesome job @hartbit!! I'm so excited to land this!
🎉 1
ericvicenti commented 2017-01-09 02:45:20 -08:00 (Migrated from github.com)
Copy Link

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

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 commented 2017-01-09 02:53:07 -08:00 (Migrated from github.com)
Copy Link

@ericvicenti looking now

@ericvicenti looking now
emilsjolander commented 2017-01-09 04:06:40 -08:00 (Migrated from github.com)
Copy Link

@ericvicenti Found issue. Pushing fix within next few hours

@ericvicenti Found issue. Pushing fix within next few hours
hartbit commented 2017-01-09 04:32:18 -08:00 (Migrated from github.com)
Copy Link

@emilsjolander What was the problem?

@emilsjolander What was the problem?
emilsjolander commented 2017-01-09 08:46:01 -08:00 (Migrated from github.com)
Copy Link

0acb620159

https://github.com/facebook/yoga/commit/0acb6201594739d069d8973ee8391dea9bfaf9c7
hartbit commented 2017-01-09 09:06:21 -08:00 (Migrated from github.com)
Copy Link

Ah, of course!

Ah, of course!

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
emilsjolander
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#322
No description provided.
Delete Branch "improve-yogakit-api"

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?