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
[TIMOB-24593] Update Facebook Android SDK + iOS SDK to 4.25.0, support Graph v2.9 + FBSDKPlacesKit API, refactor iOS-project #85
Conversation
…into update-android-fb-sdk
…book into update-android-fb-sdk
@garymathews I finished moving the "old" jar-library to our new AAR-handling and updated to the latest FB-SDK. We would probably need to bump the minimum version of Ti.Facebook to 6.1.0 then, is that okay? |
@hansemannn Yes, the older module can be used if pre-6.1.0 compatibility is needed 👍 I'll review this very soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR: PASS
ios/Classes/FacebookModule.m
Outdated
|
||
if (description != nil) { | ||
NSLog(@"Ti.Facebook.presentMessengerDialog.description has been deprecated in Ti.Facebook 5.5.0 as of the Graph v2.9 changes."); | ||
content.contentDescription = [params objectForKey:@"description"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per FBSDKShareLinkContent.h -
@Property (nonatomic, readonly) NSString *contentDescription
DEPRECATED_MSG_ATTRIBUTE("contentDescription
is deprecated from Graph API 2.9");
contentDescription has changed to read only, but you are assigning to it. It is giving build error. Please remove it.
ios/Classes/FacebookModule.m
Outdated
|
||
if (title != nil) { | ||
NSLog(@"Ti.Facebook.presentMessengerDialog.title has been deprecated in Ti.Facebook 5.5.0 as of the Graph v2.9 changes."); | ||
content.contentTitle = title; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Property (nonatomic, readonly) NSString *contentTitle
DEPRECATED_MSG_ATTRIBUTE("contentTitle
is deprecated from Graph API 2.9");
contentTitle has changed to read only, but you are assigning to it. It is giving build error. Please remove it.
ios/Classes/FacebookModule.m
Outdated
|
||
if (picture != nil) { | ||
NSLog(@"Ti.Facebook.presentMessengerDialog.picture has been deprecated in Ti.Facebook 5.5.0 as of the Graph v2.9 changes."); | ||
content.imageURL = [NSURL URLWithString:picture]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per FBSDKShareLinkContent.h -
@Property (nonatomic, readonly) NSURL *imageURL
DEPRECATED_MSG_ATTRIBUTE("imageURL
is deprecated from Graph API 2.9");
imageURL has changed to read only, but you are assigning to it. It is giving build error. Please remove it.
ios/Classes/FacebookModule.m
Outdated
|
||
if (description != nil) { | ||
NSLog(@"Ti.Facebook.presentShareDialog.description has been deprecated in Ti.Facebook 5.5.0 as of the Graph v2.9 changes."); | ||
content.contentDescription = [params objectForKey:@"description"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per FBSDKShareLinkContent.h -
@Property (nonatomic, readonly) NSString *contentDescription
DEPRECATED_MSG_ATTRIBUTE("contentDescription is deprecated from Graph API 2.9");
contentDescription has changed to read only, but you are assigning to it. It is giving build error. Please remove it.
@vijaysingh-axway Thanks for the feedback, I've eliminated all of them! |
Hey @vijaysingh-axway, this unintentionally ended in basically rewriting the whole module. This includes:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR passed.
@garymathews Double-checking: Can we merge this? iOS is ready, Android seems to be approved by you as well. Thanksss! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR: PASS
@ewieberappc Can we do a FT before merging here? I feel we are more save to test the packaged modules before. It's provided above! |
Side-note: CI fails because it tries with master, which requires the V8 upgrade for Android, so not related to this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Node Version: 6.10.3
NPM Version: 3.10.10
Mac OS: 10.12.4
Appc CLI: 6.2.3
Appc CLI NPM: 4.2.9
Titanium SDK version: 6.2.0.v20170823150008
Appcelerator Studio, build: 4.9.1.201707200100
Xcode 8.3.2
Ti Facebook Module 6.3.0 for Android
FR passed for Android. I used the latest module, and was able to use the module with a Facebook app that I created.
JIRA: https://jira.appcelerator.org/browse/TIMOB-24593
Android: facebook-android-6.3.0.zip
iOS: facebook-iphone-5.5.0.zip