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.

Sign up for GitHub

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

Jump to bottom

[FEAT] Added custom share text option. #99

Merged
merged 1 commit into from
Apr 16, 2016
Merged

[FEAT] Added custom share text option. #99

merged 1 commit into from
Apr 16, 2016

Conversation

renesansz
Copy link
Contributor

Added option for customizing share text.

@renesansz renesansz mentioned this pull request Apr 13, 2016
Closed
@renesansz renesansz force-pushed the feat/added-custom-share-text-option branch from b776746 to e01e5d6 Compare April 13, 2016 06:36
aaustin
aaustin reviewed Apr 13, 2016
View reviewed changes
src/android/io/branch/BranchSDK.java
{

Log.d(LCAT, "start showShareSheet()");

ShareSheetStyle shareSheetStyle = new ShareSheetStyle(this.activity, "Check this out!", "This stuff is awesome: ")
ShareSheetStyle shareSheetStyle = new ShareSheetStyle(this.activity, "Check this out!", shareText)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should expose the title as well instead of hard coding it to "Check this out!"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I supposed, however iOS doesn't have a functionality to modify the share title text according to @jestoniyap . So atm I disregarded the modification of the title, though I can still add the functionality for Android but not on iOS. If it's okay with you.

@aaustin
Copy link
Contributor

aaustin commented Apr 13, 2016

Can you add this argument to iOS as well?

@renesansz
Copy link
Contributor Author

Hey @aaustin ,
This is already have been implemented in iOS, just have to let the javascript accept additional parameter for the custom text.
https://github.com/BranchMetrics/Cordova-Ionic-PhoneGap-Deferred-Deep-Linking-SDK/blob/master/src/ios/BranchSDK.m#L473

@aaustin
Copy link
Contributor

aaustin commented Apr 14, 2016

Sounds good. See my other note though?

@renesansz
Copy link
Contributor Author

Yes, however I also consulted @jestoniyap in regards to the iOS.
He said that there's no implementation for iOS in order to modify the share sheet title.
That's why I disregarded this at the moment.

@aaustin
Copy link
Contributor

aaustin commented Apr 15, 2016

Ok sounds good. Thanks. Can you fix the merge conflicts first?

@renesansz renesansz force-pushed the feat/added-custom-share-text-option branch from e01e5d6 to ad5c2a5 Compare April 15, 2016 08:16
@renesansz
Copy link
Contributor Author

@aaustin fixed merge conflict.

@aaustin
Copy link
Contributor

aaustin commented Apr 16, 2016

👍

@aaustin aaustin merged commit 6702d55 into master Apr 16, 2016
@aaustin aaustin deleted the feat/added-custom-share-text-option branch April 16, 2016 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@renesansz @aaustin