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.

Sign up for GitHub

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

Jump to bottom

Add missing onBackgroundPushReceived method #426

Closed
wants to merge 3 commits into from
Closed

Add missing onBackgroundPushReceived method #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d3e4916
handle notifications from a cold start
MaximBelov Apr 19, 2024
5446425
Add onBackgroundPushReceived method
MaximBelov Apr 19, 2024
93ebe85
Revert "handle notifications from a cold start"
MaximBelov Apr 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Example/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
console.log("onPushReceived: " + JSON.stringify(event))
})

Airship.push.onBackgroundPushReceived(function(event) {
console.log("onBackgroundPushReceived: " + JSON.stringify(event))
})

Airship.push.onNotificationResponse(function(event) {
console.log("onNotificationResponse: " + JSON.stringify(event))
})
Expand Down
10 changes: 10 additions & 0 deletions cordova-airship/types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,16 @@ export interface AirshipPush {
callback: (event: PushReceivedEvent) => void
): Cancellable

/**
* Background push received listener.
*
* @param callback The callback.
* @return A cancellable that can be used to cancel the listener.
*/
onBackgroundPushReceived(
callback: (event: PushReceivedEvent) => void
): Cancellable

/**
* Notification response listener.
*
Expand Down
5 changes: 5 additions & 0 deletions cordova-airship/www/Airship.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,11 @@ airship.push.onPushReceived = function (callback) {
return registerListener("airship.event.push_received", callback)
}

airship.push.onBackgroundPushReceived = function (callback) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@MaximBelov MaximBelov Apr 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@rlepinski rlepinski Apr 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

You need this exposed in the app?

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 you will get a foreground on the response but not currently the push received event. We could add it though

Copy link
Contributor

Choose a reason for hiding this comment

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

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

argscheck.checkArgs('F', 'Airship.push.onBackgroundPushReceived', arguments)
return registerListener("airship.event.background_push_received", callback)
}

airship.push.onNotificationResponse = function (callback) {
argscheck.checkArgs('F', 'Airship.push.onNotificationResponse', arguments)
return registerListener("airship.event.notification_response", callback)
Expand Down