-
Notifications
You must be signed in to change notification settings - Fork 23
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
chore: improve reliability dismissing modal messages on page rule change #732
chore: improve reliability dismissing modal messages on page rule change #732
Conversation
Part of: https://linear.app/customerio/issue/MBL-355/ios-fix-in-app-message-displayed-on-wrong-page There is a scenario where a modal message can be displayed on the wrong screen due to a timing issue. Although the time window where this could happen is small, it was encountered during QA testing. Test case to reproduce: * In-app message with page rule "Dashboard" sent to device. User on "Dashboard" screen. In-app message begins to render. * Similar to what is explained [here](https://customerio.slack.com/archives/C06H7MMLH6C/p1716391939537599), if the user of the app clicks a button to begin navigating to another screen in the app and the modal message finishes rendering during the screen transition animation, the modal will be displayed and will remain displayed after the animation is complete and the next screen is shown. Solution: Dismiss the modal when the screen transition animation ends and a new page rule is triggered. Ensure that when a Modal message is shown, it does not trigger an automatic screenview event. This allows the SDK to recognize user navigation to a new screen accurately. Testing: * Automated tests added to verify auto screenview tracking. * QA test verified that when a Gist Modal message shown, an automatic screenview event not tracked.
Sample app builds 📱Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.
|
SDK binary size reports 📊SDK binary size of this PR
SDK binary size diff report between this PR and the main branch
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## levi/inapp-strict-pagerules #732 +/- ##
===============================================================
+ Coverage 60.29% 60.50% +0.21%
===============================================================
Files 140 140
Lines 3926 3922 -4
===============================================================
+ Hits 2367 2373 +6
+ Misses 1559 1549 -10 ☔ View full report in Codecov by Sentry. |
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.
👍
Part of: https://linear.app/customerio/issue/MBL-355/ios-fix-in-app-message-displayed-on-wrong-page After this PR, #732, if you close a modal message, it will not load the next one in the local queue. This PR brings this feature back by checking the local queue after a message is closed. Testing * Added automated tests. However, the added tests fail because of asynchronous code needing to run. I rebased this commit on top of PR (#738) and tests all ran and passed as expected. To not block this PR from getting merged and shipped, the automated tests are skipped in this PR. Once we merge in the synchronous changes, we can re-enable the tests.
Part of: https://linear.app/customerio/issue/MBL-355/ios-fix-in-app-message-displayed-on-wrong-page
There is a scenario where a modal message can be displayed on the wrong screen due to a timing issue. Although the time window where this could happen is small, it was encountered during QA testing.
Test case to reproduce:
Solution:
Dismiss the modal when the screen transition animation ends and a new page rule is triggered. Ensure that when a Modal message is shown, it does not trigger an automatic screenview event. This allows the SDK to recognize user navigation to a new screen accurately.
Testing:
Based on PR #731
Full chain of PRs as of 2024-06-14
levi/ignore-cio-autoscreenviews
➔levi/inapp-strict-pagerules
levi/inapp-strict-pagerules
➔main