WalletConnect
Make the notification a data notification
You have much more control over a data notification. E.g. this way you can localize the text for the user. Also this way you do not need to go through the launch activity but can directly launch the correct action. It was this way before - but with the unfortunate migration to javascript it became a message notification.
Please see: https://firebase.google.com/docs/cloud-messaging/concept-options
@pedrouid perhaps this was also the problem you had with the kotlin version? These messages have to be handled differently and remembering the problems you described this could explain it. So this means we can go back to kotlin here then?-)
@pedrouid perhaps this was also the problem you had with the kotlin version? These messages have to be handled differently and remembering the problems you described this could explain it. So this means we can go back to kotlin here then?-)
Ahah maybe that was the issue. I need to detail the tech spec more thoroughly for client-side as well. Most specification revolves around the Bridge server which is not as relevant
Happy to move back to Kotlin!
Wait a second, I just realised this was a PR and not an issue.
On this PR, you are removing the notification body. We need that to tell the user what this notification is about if the app shuts down. Otherwise we could've down simply through websockets.
No - removing the notification body makes it to a data notification. Please see: https://firebase.google.com/docs/cloud-messaging/concept-options
push
btw.: this seems to be related: https://discuss.walletconnect.org/t/dappname-in-create-call/116
Hey, I've followed up on the discourse about the dappName, being included in the push notification. This actually has been part of the design from the beginning. We can make it optional however.
Also about the data notifications, they assume the client app is actively running on the background which might not always be the case. Making it not possible for the operating system to push these notifications to re-open the app.
Hey, I've followed up on the discourse about the dappName, being included in the push notification. This actually has been part of the design from the beginning. We can make it optional however.
IMHO it should be completely removed
Also about the data notifications, they assume the client app is actively running on the background which might not always be the case. Making it not possible for the operating system to push these notifications to re-open the app.
no - this is not true - please read the documentation I linked
No worries, I will give it a try right now with the walletconnect-developer-app
Confirmed! This was the same issue that I got from the Kotlin codebase before. However we need to figure out why does my app require a notification object.
Also about the data notifications, they assume the client app is actively running on the background which might not always be the case. Making it not possible for the operating system to push these notifications to re-open the app.
no - this is not true - please read the documentation I linked
I've read the documentation again and I still get the same conclusion: Notification Message - FCM automatically displays the message to end-user devices on behalf of the client app. Data Message - Client app is responsible for processing data messages. Data messages have only custom key-value pairs.
The misunderstanding is solely because of the way the push notifications are handled in our own apps. I want the user receive notifications even in when the app is not open or even running on the background. This is a requirement for the Data Messages which completely removes the point of why we created the Push Server in the first place. We are implementing long-polling and websockets soon enough that would replace the whole purpose of Data Messages. However Notification Messages require the Push Server to exist and the idea of WalletConnect is to exist in the absence of the Push Server but provide this extra layer for improved UX
I've read the documentation again and I still get the same conclusion: Notification Message - FCM automatically displays the message to end-user devices on behalf of the client app. Data Message - Client app is responsible for processing data messages. Data messages have only custom key-value pairs.
correct - and we want "Data Message" as you have more control over them (e.g. you can display them in the language of the user, specify the correct notification-channel, ...)
The misunderstanding is solely because of the way the push notifications are handled in our own apps. I want the user receive notifications even in when the app is not open or even running on the background. This is a requirement for the Data Messages which completely removes the point of why we created the Push Server in the first place. We are implementing long-polling and websockets soon enough that would replace the whole purpose of Data Messages. However Notification Messages require the Push Server to exist and the idea of WalletConnect is to exist in the absence of the Push Server but provide this extra layer for improved UX
I just tried the following to confirm that I was reading the documentation correctly: I restarted the phone and received a Data Notification afterwards without starting the app before this. Worked perfectly - and this app is not using any ACTION_BOOT_COMPLETED or things like this.
So can we merge this now?
I'm sorry Ligi but there is still a misunderstanding. For iPhone this won't work. I requires Notification Messages to display to the user when the app is on the background or closed. Merging this PR would just make it work for Android.
I will push the development of WalletConnect long-polling / websockets faster and then we can re-visit this issue. There won't be any use for Data Messages then and we can leave the Notification Messages for only Wallet providers that want it to add to their apps.
There won't be any use for Data Messages
I see a lot of use for data messages ..