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

Add Share module #5904

Closed

deminoth wants to merge 49 commits into facebook:master from dobbit:share-module

Contributor

deminoth commented Feb 13, 2016 •

edited

Loading

Share

Open a dialog to share text content.

In iOS, Returns a Promise which will be invoked an object containing action, activityType.
If the user dismissed the dialog, the Promise will still be resolved with action being Share.dismissedAction
and all the other keys being undefined.

In Android, Returns a Promise which always be resolved with action being Share.sharedAction.

Content

message - a message to share
title - title of the message

iOS

url - an URL to share

At least one of URL and message is required.

Options

iOS

excludedActivityTypes
tintColor

Android

dialogTitle


          Add Share module

a76c6c0

Contributor

facebook-github-bot commented Feb 13, 2016

By analyzing the blame information on this pull request, we identified @andreicoman11, @mkonicek and @nicklockwood to be potential reviewers.

facebook-github-bot added GH Review: review-needed CLA Signed labels

deminoth mentioned this pull request

IntentAndroid.openChooserWithOptions implements #5476

Closed

eslint-bot reviewed

View reviewed changes

Examples/UIExplorer/ShareExample.js

+                Text,
+                TextInput,
+                TouchableHighlight,
+                Share,

eslint-bot Feb 13, 2016

property Share Property not found in Object.create

Contributor

satya164 Feb 13, 2016

You need to add it to the react-native.js.flow file.


          change the way handling promise reject

424a665

Contributor

facebook-github-bot commented Feb 13, 2016

@deminoth updated the pull request.

Contributor

bestander commented Feb 13, 2016

FYI BUCK build is failing in circle CI.
Could you edit BUCK file to make it building?


          buck file added

5da16c7

Contributor

facebook-github-bot commented Feb 13, 2016

@deminoth updated the pull request.


          added buck target

8c3bece

Contributor

facebook-github-bot commented Feb 13, 2016

@deminoth updated the pull request.

deminoth added 2 commits

February 14, 2016 02:50


          add share module to flow file

1f5f0c0


          Merge branch 'master' into share-module

fb5f6ea

Contributor

facebook-github-bot commented Feb 13, 2016

@deminoth updated the pull request.

Contributor Author

deminoth commented Feb 13, 2016

@satya164 @bestander Thanks :)


          options handling fixed

f21d709

Contributor

facebook-github-bot commented Feb 13, 2016

@deminoth updated the pull request.


          Keyword "if" must be followed by whitespace.

Contributor

facebook-github-bot commented Feb 14, 2016

@deminoth updated the pull request.


          Merge branch 'master' into share-module

767f914

Contributor

satya164 commented Feb 16, 2016

cc @nicklockwood @dmmiller

ghost commented Apr 3, 2016

@ericvicenti would you mind taking a look at this pull request? It's been a while since the last commit was reviewed.

Contributor Author

deminoth commented Apr 4, 2016

Now I think it's not a good idea making a new module... Maybe this feature should included in the Linking Module?

Contributor

satya164 commented Apr 4, 2016

Sorry this got neglected. Will review it soon. Why do you think this should be part of linking?

satya164 self-assigned this

Contributor Author

deminoth commented Apr 4, 2016

@satya164 Nevermind. I thought Linking is interact with outer apps but it's just interact with app links.

andreicoman11 reviewed

View reviewed changes

ReactAndroid/src/main/java/com/facebook/react/modules/share/ShareModule.java

+                    throw new JSApplicationIllegalArgumentException("Invalid contents");
+                  }
+                  mPromise = promise;

Contributor

andreicoman11 Apr 4, 2016

Why is this needed?

satya164 reviewed

View reviewed changes

ReactAndroid/src/main/java/com/facebook/react/modules/share/ShareModule.java

+                    }
+                    if (content.hasKey("url")) {
+                      intent.putExtra(Intent.EXTRA_TEXT, content.getString("url")); // this will overwrite message

Contributor

satya164 Jul 22, 2016

Let's ignore this prop on Android.

Contributor

satya164 commented Jul 22, 2016

Thanks a lot @deminoth . Overall looks good. Just address the few things I mentioned and we can merge,

satya164 added GH Review: needs-revision and removed GH Review: review-needed labels

Contributor

satya164 commented Jul 22, 2016

Also would be nice to update the PR text with the new API. It'll be useful when someone looks at the commit to see how to use this.

ghost added the CLA Signed label

deminoth added 2 commits

July 25, 2016 18:18


          do not log the error if we're already rejecting

dc1c57c


          url in content prop is iOS only now

94023f9

ghost added GH Review: review-needed and removed GH Review: needs-revision labels

Contributor

satya164 commented Jul 25, 2016

@facebook-github-bot shipit

ghost added CLA Signed GH Review: accepted Import Started and removed GH Review: review-needed labels

ghost commented Jul 25, 2016

Thanks for importing. If you are an FB employee go to Phabricator to review internal test results.

ghost added the CLA Signed label

ghost closed this in

3b35732

Contributor

andreicoman11 commented Jul 25, 2016

Thanks @deminoth for sticking with this and seeing it through.

rozele pushed a commit to microsoft/react-native-windows that referenced this pull request


          Add Share module

a6ad011

Summary:
revision of facebook/react-native#5476

It has only one method `shareTextContent` and next will be`shareBinaryContent`.

In Android, Promise can't receive a result, because `startActivityForResult` is not working with `Intent.ACTION_SEND`. Maybe we can use `createChooser(Intent target, CharSequence title, IntentSender sender)` which requires API level 22.
Closes facebook/react-native#5904

Differential Revision: D3612889

fbshipit-source-id: 0e7aaf34b076a99089cc76bd649e6da067d9a760

mpretty-cyro pushed a commit to HomePass/react-native that referenced this pull request


          Add Share module

bfc7d41

Summary:
revision of facebook#5476

It has only one method `shareTextContent` and next will be`shareBinaryContent`.

In Android, Promise can't receive a result, because `startActivityForResult` is not working with `Intent.ACTION_SEND`. Maybe we can use `createChooser(Intent target, CharSequence title, IntentSender sender)` which requires API level 22.
Closes facebook#5904

Differential Revision: D3612889

fbshipit-source-id: 0e7aaf34b076a99089cc76bd649e6da067d9a760

bucketclan commented May 13, 2017 •

edited

Loading

I still face this issue.
RN - 40
react - 15.4.1
iOS - 10.2
device - iPhone 7 Plus(Simulator)

Contributor Author

deminoth commented May 15, 2017

@Themarsguy Can I get some more info?

Contributor

IjzerenHein commented Dec 18, 2017

Hi, just curious, was there a reason why specifying an URL/image was not implemented on Android? Were there any hurdles with this?
I'm asking, because I was thinking of adding this.
Cheers

Contributor Author

deminoth commented Dec 19, 2017

@IjzerenHein As I remember, the app chooser didn't support Uri and some extras well at that time. Maybe you can do better with recent Android apis :)

Contributor

IjzerenHein commented Dec 19, 2017

Alright, thanks for the feedback @deminoth 👍
I've already made the code changes, however I'm having a hard time building react-native from source in order to test the contribution..
Any tips on that? :)

Contributor Author

deminoth commented Dec 19, 2017

@IjzerenHein Read documents and see https://github.com/facebook/react-native/tree/master/RNTester (was UIExplorer when I wrote this module)

Contributor

IjzerenHein commented Dec 19, 2017

Alright, thanks for the tip @deminoth !

This pull request was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Import Started