Skip to content

Add Share module - #5904

Closed
deminoth wants to merge 49 commits into
react:masterfrom
dobbit:share-module
Closed

Add Share module#5904
deminoth wants to merge 49 commits into
react:masterfrom
dobbit:share-module

Conversation

@deminoth

@deminothdeminoth commented Feb 13, 2016•
edited
Loading

Copy link
Copy Markdown
Contributor

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

@facebook-github-bot

facebook-github-bot commented Feb 13, 2016

Copy link
Copy Markdown
Contributor

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

@facebook-github-botfacebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Feb 13, 2016
@deminothdeminoth mentioned this pull request Feb 13, 2016
Closed
eslint-bot
eslint-bot reviewed Feb 13, 2016
View reviewed changes
Comment threadExamples/UIExplorer/ShareExample.js
Text,
TextInput,
TouchableHighlight,
Share,

@eslint-boteslint-botFeb 13, 2016

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

property Share Property not found in Object.create

@satya164satya164Feb 13, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

@facebook-github-bot

facebook-github-bot commented Feb 13, 2016

Copy link
Copy Markdown
Contributor

@deminoth updated the pull request.

@bestander

bestander commented Feb 13, 2016

Copy link
Copy Markdown
Contributor

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

@facebook-github-bot

facebook-github-bot commented Feb 13, 2016

Copy link
Copy Markdown
Contributor

@deminoth updated the pull request.

@facebook-github-bot

facebook-github-bot commented Feb 13, 2016

Copy link
Copy Markdown
Contributor

@deminoth updated the pull request.

@facebook-github-bot

facebook-github-bot commented Feb 13, 2016

Copy link
Copy Markdown
Contributor

@deminoth updated the pull request.

@deminoth

deminoth commented Feb 13, 2016

Copy link
Copy Markdown
ContributorAuthor

@satya164@bestander Thanks :)

@facebook-github-bot

facebook-github-bot commented Feb 13, 2016

Copy link
Copy Markdown
Contributor

@deminoth updated the pull request.

@facebook-github-bot

facebook-github-bot commented Feb 14, 2016

Copy link
Copy Markdown
Contributor

@deminoth updated the pull request.

@satya164

satya164 commented Feb 16, 2016

Copy link
Copy Markdown
Contributor

cc @nicklockwood@dmmiller

@ghost

ghost commented Apr 3, 2016

Copy link
Copy Markdown

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

@deminoth

deminoth commented Apr 4, 2016

Copy link
Copy Markdown
ContributorAuthor

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

@satya164

satya164 commented Apr 4, 2016

Copy link
Copy Markdown
Contributor

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

@satya164satya164 self-assigned this Apr 4, 2016
@deminoth

deminoth commented Apr 4, 2016

Copy link
Copy Markdown
ContributorAuthor

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

andreicoman11
andreicoman11 reviewed Apr 4, 2016
View reviewed changes
Comment threadReactAndroid/src/main/java/com/facebook/react/modules/share/ShareModule.java Outdated
throw new JSApplicationIllegalArgumentException("Invalid contents");
}

mPromise = promise;

@andreicoman11andreicoman11Apr 4, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

satya164
satya164 reviewed Jul 22, 2016
View reviewed changes
Comment threadReactAndroid/src/main/java/com/facebook/react/modules/share/ShareModule.java Outdated
}

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

@satya164satya164Jul 22, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's ignore this prop on Android.

@satya164

satya164 commented Jul 22, 2016

Copy link
Copy Markdown
Contributor

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

@satya164

satya164 commented Jul 22, 2016

Copy link
Copy Markdown
Contributor

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.

@ghostghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 22, 2016
@satya164

satya164 commented Jul 25, 2016

Copy link
Copy Markdown
Contributor

@facebook-github-bot shipit

@ghostghost added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Jul 25, 2016
@ghost

ghost commented Jul 25, 2016

Copy link
Copy Markdown

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

@ghostghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 25, 2016
@ghostghost closed this in 3b35732Jul 25, 2016
@andreicoman11

andreicoman11 commented Jul 25, 2016

Copy link
Copy Markdown
Contributor

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

rozele pushed a commit to microsoft/react-native-windows that referenced this pull request Jul 26, 2016
@deminothFacebook Github Bot 6
Add Share module
a6ad011
Summary:
revision of react/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.
Closesreact/react-native#5904
Differential Revision: D3612889
fbshipit-source-id: 0e7aaf34b076a99089cc76bd649e6da067d9a760
@bucketclan

bucketclan commented May 13, 2017•
edited
Loading

Copy link
Copy Markdown

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

@deminoth

deminoth commented May 15, 2017

Copy link
Copy Markdown
ContributorAuthor

@Themarsguy Can I get some more info?

@IjzerenHein

IjzerenHein commented Dec 18, 2017

Copy link
Copy Markdown
Contributor

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

@deminoth

deminoth commented Dec 19, 2017

Copy link
Copy Markdown
ContributorAuthor

@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 :)

@IjzerenHein

IjzerenHein commented Dec 19, 2017

Copy link
Copy Markdown
Contributor

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? :)

@deminoth

deminoth commented Dec 19, 2017

Copy link
Copy Markdown
ContributorAuthor

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

@IjzerenHein

IjzerenHein commented Dec 19, 2017

Copy link
Copy Markdown
Contributor

Alright, thanks for the tip @deminoth !

This pull request was closed.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@satya164satya164

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.Import StartedThis pull request has been imported. This does not imply the PR has been approved.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

11 participants

@deminoth@facebook-github-bot@bestander@satya164@mkonicek@grabbou@ericvicenti@andreicoman11@bucketclan@IjzerenHein@eslint-bot