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

Receive sound and color from JSON payload #225

Merged
merged 1 commit into from
Nov 11, 2016

Conversation

jgarber
Copy link
Contributor

@jgarber jgarber commented Oct 8, 2016

Sound and color should be passed from the JSON payload to the notification listener.

Sound and color should be passed from the [JSON payload](https://developers.google.com/cloud-messaging/http-server-ref#notification-payload-support) to the notification listener.
@varungupta85
Copy link
Contributor

@jgarber I think this feature is already present in the current implementation. I am not using color in my code yet but sound plays as configured in the remote notification if configured in the bundle itself. According to your changes, you are setting the color and sound from the data section of the bundle but I think if those properties are configured at the top level in the bundle, you should see the expected results.

@jgarber
Copy link
Contributor Author

jgarber commented Oct 8, 2016

@varungupta85 I'm afraid not. Put in a few logging statements and try it yourself.

JSON payload:

{"channels": ["rtAvn8ayx3y"], "color": "#FF0000", "sound": "alarm.wav", "data": {"title": "Test title", "alert": "Your alarm is sounding", "color": "#FF0000", "sound": "alarm.wav"}}

adb logcat:

0-08 06:00:30.538 17321 17664 D RNPushNotification: Not parsing sound/color from data
10-08 06:00:30.543 17321 17495 I ReactNativeJS: 'NOTIFICATION:', { foreground: false,
10-08 06:00:30.543 17321 17495 I ReactNativeJS:   'google.sent_time': 1475920829803,
10-08 06:00:30.543 17321 17495 I ReactNativeJS:   userInteraction: false,
10-08 06:00:30.543 17321 17495 I ReactNativeJS:   push_id: 'x423DEnffO',
10-08 06:00:30.543 17321 17495 I ReactNativeJS:   id: '776927272',
10-08 06:00:30.543 17321 17495 I ReactNativeJS:   data:
10-08 06:00:30.543 17321 17495 I ReactNativeJS:    { title: 'Test title',
10-08 06:00:30.543 17321 17495 I ReactNativeJS:      alert: 'Your alarm is sounding',
10-08 06:00:30.543 17321 17495 I ReactNativeJS:      color: '#FF0000',
10-08 06:00:30.543 17321 17495 I ReactNativeJS:      sound: 'alarm.wav' },
10-08 06:00:30.543 17321 17495 I ReactNativeJS:   time: '2016-10-08T10:00:29.299Z',
10-08 06:00:30.543 17321 17495 I ReactNativeJS:   title: 'Test title',
10-08 06:00:30.543 17321 17495 I ReactNativeJS:   'google.message_id': '0:1475920829812308%b0cc2c21f9fd7ecd',
10-08 06:00:30.543 17321 17495 I ReactNativeJS:   message: 'Your alarm is sounding',
10-08 06:00:30.543 17321 17495 I ReactNativeJS:   collapse_key: 'do_not_collapse' }
10-08 06:00:30.544 17321 17664 D RNPushNotification: Bundle being passed to RNPushNotificationHelper sendNotification: Bundle[{google.sent_time=1475920829803, userInteraction=false, push_id=x423DEnffO, id=776927272, data={"title":"Test title","alert":"Your alarm is sounding","color":"#FF0000","sound":"alarm.wav"}, time=2016-10-08T10:00:29.299Z, title=Test title, google.message_id=0:1475920829812308%b0cc2c21f9fd7ecd, message=Your alarm is sounding, collapse_key=do_not_collapse, foreground=false}]
10-08 06:00:30.548 17321 17664 D RNPushNotification: soundName: null
10-08 06:00:30.548 17321 17664 D RNPushNotification: color: null

...but thank you so much @varungupta85 for adding sounds! You got me 99% of the way there! 👏

@jgarber
Copy link
Contributor Author

jgarber commented Oct 8, 2016

...In which case there might be other things you want to collect from the payload. I just needed sound and color was easy too. Feel free to address it differently!

@jgarber
Copy link
Contributor Author

jgarber commented Oct 8, 2016

The project I'm using this on has Parse Server on the backend. With sound in the data hash and this patch for Android, the alarm sound works properly on both platforms through Parse's push notifications.

@npomfret
Copy link
Contributor

npomfret commented Oct 9, 2016

This project already supports sound and colour. It sounds like the parse backend isn't sending the data in the correct shape maybe? The solution is to send a silent notification that contains a payload, and then have your app send a local-notification in the JS code that puts an alert in the notification centre. With local-notifications you can control sound, vibrate, color, different text sizes - you have much more control over the ui basically.

@jgarber
Copy link
Contributor Author

jgarber commented Oct 11, 2016

@the Parse backed is sending data in the correct shape for iOS and I just assumed it should be the same for Android since RN is all about cross platform as much as possible. But if you say that's not how it's supposed to work, that's fine--feel free to close this PR. I don't think it does any harm, but on the other hand, I've been an OSS maintainer who accepted too much and got the project loaded up with everybody's crap. 😁

Thanks for setting giving me some direction on the proper implementation!

@npomfret npomfret merged commit d9cd728 into zo0r:master Nov 11, 2016
@npomfret
Copy link
Contributor

I agree actually - this project already synthesises a local notification from a data payload, so all this does is add to that functionality

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.

None yet

3 participants