Add missing onBackgroundPushReceived method #426
Conversation
MaximBelov
changed the title
| @@ -377,6 +377,11 @@ airship.push.onPushReceived = function (callback) { | |||
| return registerListener("airship.event.push_received", callback) | |||
| } | |||
|
|
|||
| airship.push.onBackgroundPushReceived = function (callback) { | |||
There was a problem hiding this comment.
This is only different on Android because on the other frameworks you have to do some special hoops to get background push working. On cordova I am not sure I see a point of separating them. What if we have both airship.event.background_push_received and airship.event.push_received go to the same listener?
There was a problem hiding this comment.
depending on the state of the application in the background only one will work
I added listeners for events
airship.event.push_received - foreground, ios and android
airship.event.notification_response - background, ios only
airship.event.background_push_received - background android only - background or cold start
my initial testing with positive results
it works correctly without any problems
There was a problem hiding this comment.
But do you need it separated? The event map, I think we can just map the background event to airship.event.push_received on Android to have a single onPushReceived. I am not sure if there is a reason to have it broken out on Android.
I might need to modify the proxy to put them all under the same event queue that way the events stay in order https://github.com/urbanairship/airship-mobile-framework-proxy/blob/main/android/airship-framework-proxy/src/main/java/com/urbanairship/android/framework/proxy/events/EventEmitter.kt#L21
I am thinking ill add a config flag to the proxy to treat all push received as the same thing on Android then we can ignore the background_push_received events.
There was a problem hiding this comment.
we can combine this into one event listener, but we must add new properties to the response to understand exactly what type of event received and where it came from
There was a problem hiding this comment.
You need this exposed in the app?
There was a problem hiding this comment.
Looks like you will get a foreground on the response but not currently the push received event. We could add it though
There was a problem hiding this comment.
Adds isForeground and orders events - urbanairship/airship-mobile-framework-proxy#39
Going to open a new PR that combines pushReceived and notificationResponses into the same listener
|
#428 still need the proxy to be reviewed + released before the event order is correct and we get the |
|
Fixed |
What do these changes do?
Add missing onBackgroundPushReceived method
Why are these changes necessary?
How did you verify these changes?
Added method to Example/index,html
Verification Screenshots:
Anything else a reviewer should know?