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.

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

Fix Share dialog not resolving promise when dismissed on iOS #26842

Closed

Conversation

v-fernandez
Copy link
Contributor

@v-fernandez v-fernandez commented Oct 13, 2019

Summary

On iOS the promised returned by Share.share(content, options) isn't resolved if the user dismisses the dialog by either pressing "Cancel" or pressing outside the shared dialog. This PR fixes this issue.

This fixes #26809.

Changelog

[iOS] [Fixed] - Fix promised returned by Share.share(content, options) not resolving if share dialog dismissed

Test Plan

  1. on iOS, open a share dialog with:
const onShare = async () => {
  const result = await Share.share({ message: 'example message' });
}
  1. Dismiss the opened share dialog and the returned promised should resolve.

@facebook-github-bot facebook-github-bot 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 Oct 13, 2019
@tomtargosz
Copy link
Contributor

So it looks like this is because of my PR (#26429). I added this check because we need to have some sort of guard against the success callback being called multiple times. Is there a way we can know that the ActionSheet was closed so we can do if (completed || dismissed)?

@tomtargosz
Copy link
Contributor

@v-fernandez so I think we can change the check to:
if (completed || activityType == nil)

It looks like in completionWithItemsHandler the activityType will only be nil if the user closed the ShareSheet themselves, like so:

Screen Shot 2019-10-14 at 1 41 22 PM

Here's when they close the reminders modal:
Screen Shot 2019-10-14 at 1 43 02 PM

And the photos modal:
Screen Shot 2019-10-14 at 1 47 03 PM

This should fix the current issue without reintroducing the crash.

@v-fernandez
Copy link
Contributor Author

@tomtargosz makes sense. I'll update the PR with this. Thanks for the help!

Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

Nice, thank you!

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @v-fernandez in 7468a6c.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 28, 2019
@franciscomorais
Copy link

When is this PR going to be picked?

@dpolkovnikov
Copy link

When is this PR going to be picked?

https://github.com/facebook/react-native/releases/tag/v0.62.0-rc.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API: Share Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Promise returned by share dialog won't resolve if cancelled on iOS
7 participants