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

Expose content-available APS key for iOS silent push #14584

Closed
wants to merge 7 commits into from

Conversation

blankg
Copy link

@blankg blankg commented Jun 18, 2017

Thanks for submitting a PR! Please read these instructions carefully:
  • Explain the motivation for making this change.
  • Provide a test plan demonstrating that the code is solid.
  • Match the code formatting of the rest of the codebase.
  • Target the master branch, NOT a "stable" branch.

Please read the Contribution Guidelines to learn more about contributing to React Native.

Motivation (required)

_What existing problem does the pull request solve?
In iOS when sending a silent push notification you need to configure the 'content-available' APS key to the value of 1 (When this key is present, the system wakes up your app in the background and delivers the notification to its app delegate, see apple docs).

This PR exposes this property to the notification event handler so app code can handle silent push scenario specifically. Currently this property is not available.

Test Plan (required)

I've updated the PushNotificationIOSExample in the RNTester.

  1. Open RNTester in xcode
  2. Enable the push notifications capability
  3. run on device
  4. Go to PushNotificationIOS
  5. click on "send fake notification"
  6. verify alert message contains 'content-available' with a value of 1.

@facebook-github-bot facebook-github-bot 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: review-needed labels Jun 18, 2017
@@ -72,11 +72,13 @@ class NotificationExample extends React.Component {

_sendNotification() {
require('RCTDeviceEventEmitter').emit('remoteNotificationReceived', {
remote: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Author

Choose a reason for hiding this comment

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

This property is added by the iOS native code for remote notification (handleRemoteNotificationReceived) and the PushNotificationIOS constructor looks for it otherwise it will not look for the aps object.

@@ -115,9 +117,15 @@ class NotificationExample extends React.Component {
}

_onRemoteNotification(notification) {
const result = `Alert message: ${notification.getMessage()}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Indention.

Copy link
Author

Choose a reason for hiding this comment

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

will fix

@@ -485,6 +487,13 @@ class PushNotificationIOS {
}

/**
* Gets the content-available number from the `aps` object
*/
getContentAvailable(): ?number {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the returning type is different from _contentAvailable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the only possible values are 1 and null, right?

Copy link
Author

Choose a reason for hiding this comment

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

yes, it can be 1 or null/undefined so this is why it is optional. I aligned _contentAvailable member with the other private members in this class which add the optional flag only in the getter.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we declare special type type ContentAvailable = 1; (https://flow.org/en/docs/types/unions/) and then use it as ?ContentAvailable? (I am not sure.) Or maybe we can create special already optional type?

Copy link
Author

Choose a reason for hiding this comment

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

Create a ContentAvailable type which is already optional

@facebook-github-bot facebook-github-bot added 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 Jun 19, 2017
@facebook-github-bot
Copy link
Contributor

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

sayar pushed a commit to microsoft/react-native-windows that referenced this pull request Jul 26, 2017
Summary:
<details>
  Thanks for submitting a PR! Please read these instructions carefully:

  - [ ] Explain the **motivation** for making this change.
  - [ ] Provide a **test plan** demonstrating that the code is solid.
  - [ ] Match the **code formatting** of the rest of the codebase.
  - [ ] Target the `master` branch, NOT a "stable" branch.

  Please read the [Contribution Guidelines](https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md) to learn more about contributing to React Native.
</details>

_What existing problem does the pull request solve?
In iOS when sending a silent push notification you need to configure the 'content-available' APS key to the value of 1 (When this key is present, the system wakes up your app in the background and delivers the notification to its app delegate, see [apple docs](https://developer.apple.com/library/content/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/PayloadKeyReference.html#//apple_ref/doc/uid/TP40008194-CH17-SW1)).

This PR exposes this property to the notification event handler so app code can handle silent push scenario specifically. Currently this property is not available.

I've updated the PushNotificationIOSExample in the RNTester.

1. Open RNTester in xcode
2. Enable the  push notifications capability
3. run on device
4. Go to PushNotificationIOS
5. click on "send fake notification"
6. verify alert message contains 'content-available' with a value of 1.
Closes facebook/react-native#14584

Differential Revision: D5279181

Pulled By: shergin

fbshipit-source-id: d2288e147d89ba267f54265d819aa0a9969095e7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants