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

iOS 13: Resolve promise if ShareSheet is manually dismissed #607

Merged

Conversation

tomtargosz
Copy link
Contributor

@tomtargosz tomtargosz commented Oct 15, 2019

Overview

Fixes #601.

This PR fixes an issue on iOS 13 where the ShareSheet's promise isn't resolved if the user closes the sheet without completing any actions introduced in #584.

This PR changes the if (completed) check in completionWithItemsHandler 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.

Test Plan

  • Ensure promise is resolved when the ShareSheet is closed manually
  • Ensure the promise is not resolved when a different modal is dismissed (photos, add to contact, reminders, etc)

@tomtargosz
Copy link
Contributor Author

Similar PR is open in react-native: facebook/react-native#26842

Copy link
Member

@jgcmarins jgcmarins left a comment

Choose a reason for hiding this comment

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

nice one, thanks

@jgcmarins jgcmarins merged commit 736a8ac into react-native-share:master Oct 16, 2019
cuonghuynhpycogroup pushed a commit to cuonghuynhpycogroup/react-native-share that referenced this pull request Dec 25, 2019
* master-base: (215 commits)
  Update react-native to 0.61.4 (react-native-share#637)
  Update and fix Gradle setup (react-native-share#588)
  set useNativeDriver explicitly for Animations (react-native-share#633)
  correctly type image source (react-native-share#632)
  ci(fix): adapt to new workflow (react-native-share#626)
  fix: added missing "v" to `source` field in podspec (react-native-share#619)
  chore(readme): mocking with Jest example (react-native-share#610)
  iOS 13: Resolve promise if ShareSheet is manually dismissed (react-native-share#607)
  Update README.md
  fix: remove uncessary tools replace on build gradle
  chore(lint): fixing lint errors
  Makes sharing with Snapchat work on Android (react-native-share#464)
  Fixed validate error
  Removed react-native Share dependency
  Only call successCallback when completed is true
  Change readme.md
  Update Gradle wrapper to 5.6.2
  build(deps): bump eslint-utils from 1.4.0 to 1.4.2 (react-native-share#580)
  Send email to predefined email address - Example
  Send email to predefined email address
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iOS 13 New UI close button doesn't reject promise
2 participants