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

Remove authorization request for notifications from app app delegate #203

Conversation

josh-
Copy link
Contributor

@josh- josh- commented May 27, 2024

Currently when using the Intercom React Native SDK on iOS, users are prompted to enable notification permissions as soon as a the app is launched, as a result of the SDK injecting a call to requestAuthorizationWithOptions (doc here) in the app delegate.

This contravenes the recommendation in the README to manually request permissions from the user:

Note: You should request user permission to display push notifications. e.g. react-native-permissions


As such, this PR removes the call to requestAuthorizationWithOptions, so SDK consumers can instead request permissions at the time that is appropriate to their app (eg. in an onboarding carousel)

Copy link
Contributor

@Br1an-Boyle Br1an-Boyle left a comment

Choose a reason for hiding this comment

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

Thanks @josh-

@Br1an-Boyle
Copy link
Contributor

@josh- you'll need to update to the latest master branch in order to merge this PR

@josh- josh- force-pushed the ios-app-delegate-remove-request-notification-permission branch from a3db412 to 5be2b53 Compare May 29, 2024 03:39
@josh-
Copy link
Contributor Author

josh- commented May 29, 2024

Done - thanks @Br1an-Boyle

@liutingdu liutingdu enabled auto-merge (squash) May 29, 2024 12:36
auto-merge was automatically disabled May 29, 2024 23:29

Head branch was pushed to by a user without write access

@josh- josh- force-pushed the ios-app-delegate-remove-request-notification-permission branch from 5be2b53 to 84c8a94 Compare May 29, 2024 23:29
@josh-
Copy link
Contributor Author

josh- commented May 29, 2024

@liutingdu just rebased but looks like that disabled auto merge

@liutingdu liutingdu merged commit c2d0a56 into intercom:main May 30, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants