-
Notifications
You must be signed in to change notification settings - Fork 377
Properly handle in-app messages dismissed by back press #2328
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
Conversation
997a21d
to
222c20b
Compare
* By extending PopupWindow, we can mark it as dismissed by us, and we can add a custom listener to the dismiss event and receive this flag. * In InAppMessageView which owns the popup window, it will listen to the popup window's dismissal event and trigger the post-dismissal flow. It also sets the manual flag when `removeAllViews()` is invoked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested on Android 9 (API level 28) and if I press the back button it does dismiss the IAM, however if I background the app and resume the app it comes back.
- If instead I tap the "X", the IAM doesn't come back, as I would expect.
- I didn't see this issue when I tested on Android 14 (API level 34)
After more testing, this doesn't seem related, and also doesn't consistently happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes look good. The only thing I would like to see is testing a few more versions of Android and including them in the PR.
@jkasten2 - I included the Android versions I was able to test. Some I ran into issues testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for testing all those Android versions, things look good now!
I noticed the Android 5 and 6 display issues are pre-existing as well, so thing can be done for them in this PR.
Properly handle in-app messages dismissed by back press
Description
One Line Summary
Properly handle and dismiss in-app messages that are dismissed by back press.
Details
Motivation
Fixes poor behavior: When back button is pressed, the system automatically dismisses the in-app message, without the SDK's awareness. This leads to poor user experience as the SDK does not know of this message's dismissal and will inappropriately redisplay it.
Scope
InAppMessageView
. A single instance ofInAppMessageView
can have multiple popupWindow instances such as when orientation changes. So, we need to hook into individual popup windows instead of the View. We mark the popup window as manually dismissed in the methodremoveAllViews()
. This is sufficient as this method is called by every flow in the SDK that dismisses an IAM (clicking to close, activity change, drag to dismiss, etc). In our listener, if we believe the popup window was dismissed by the system bypassing OneSignal, we triggermessageController?.onMessageWasDismissed()
. This call is appropriate as this is the root method that triggers the post-dismissal flows for an IAM. We considered doing more logic in the dismiss event but we cannot generalize here as some dismiss events should not trigger the post-dismissal flow, such as on orientation changes.Example of system triggering popup window dismissal
Testing
Unit testing
None
Manual testing
Android emulator API 33
Types of IAMs tested:
Scenarios tested:
Devices tested:
GET /iams
made after user is createdAffected code checklist
Checklist
Overview
Testing
Final pass
This change is