Skip to content

Fix initial notification data lost issue and empty foreground notification issue #496

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

Merged
merged 1 commit into from
Apr 5, 2020

Conversation

patrickyin
Copy link
Contributor

@patrickyin patrickyin commented Feb 13, 2020

This PR fixes:

  • Empty foreground notification content if your notification payload has title and body files under notification field rather than data.
  • Get undefined initial notification data if your notification payload has title and body files under notification field rather than data.

Details of issue and notification payloads: #456 (comment)
Related issues: #456 , #492

@puremana
Copy link
Contributor

Can confirm this fixed the empty foreground notification issue for me. Thanks!

@trinhak
Copy link

trinhak commented Feb 18, 2020

i have improved your fork. but it's not change anything

@rnnyrk
Copy link

rnnyrk commented Feb 18, 2020

Experiencing the same issue. Would be really nice if this can be merged :)

@rnnyrk
Copy link

rnnyrk commented Feb 19, 2020

I did try to implement the changes of this PR, but afterwards the push notifications didn't work anymore. Did you tested this properly @patrickyin?

@simonasdev
Copy link

works for me

@FadiAboMsalam
Copy link

when this will be merged ?

@matt-way
Copy link

matt-way commented Mar 2, 2020

I am using one signal with this library, and had to make different (but similar) changes to the props file. I think a better solution than this PR would be the ability to set the key name to find the body and title inside javascript (defaulting to body/title). That way you could get it working with any payload format.

@alexparlett
Copy link

+1

@luisfuertes
Copy link

How can i install this? Can copy package.json line please?

@doodlemoonch
Copy link

@luisfuertes I used patch-package, example patch and it's worked well

@luisfuertes
Copy link

luisfuertes commented Mar 5, 2020

Thx for answer @doodlemoonch , i didnt know that component!

Finally i install with:

yarn add https://github.com/racv-home/react-native-notifications.git

Now in foreground, notification title and body are shown. But notification icon continue broken.
How i can hide notifications with app in foreground?

EDIT: To fix icon i only have to renamed icon png file to 'notification_icon'

@boyuanfang
Copy link

please have this merged

@patrickyin
Copy link
Contributor Author

Hmm, since I don't have permission to merge this PR, it might need code owner's help. @yogevbd

@williamcardozo
Copy link

it worked for me too. Can you merge this @yogevbd ?

@kleinbruno
Copy link

kleinbruno commented Apr 4, 2020

yeah, it worked for me! @yogevbd

@andreialecu
Copy link

I'm running react-native-notifications 3.2.2 which should have this PR but I still need to supply the data property in the payload when sending notifications, otherwise they show up blank.

The issue is also reported here: #615

Any ideas what could be missing?

@andreialecu
Copy link

andreialecu commented May 26, 2020

Interestingly I've been sending notifications using the AWS Pinpoint console, using the built in "Campaigns" functionality.

I noticed that in the adb logs the body and title appeared in pinpoint.notification.title and pinpoint.notification.body when sent using the default UI. There is a raw mode in the console where a different format can be entered.

I wonder if this is normal and if the proper body and title could be used automatically somehow for all sorts of notifications, regardless of how they're sent. Granted, I'm not very familiar with Android notifications, but on iOS this issue doesn't exist.

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.