chore: rename message variables to closer represent their purpose in SDK #730
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 PRSDK binary size diff report between this PR and the main branch |
Part of: https://linear.app/customerio/issue/MBL-310/create-a-uikit-uiview-that-displays-an-inline-in-app-message-sent-from Create a WebView to display inline in-app message and display in the Inline UIView. To do this, we can re-use a lot of the same logic that Modal messages use to create WebViews and display them. Testing: There are some automated tests added in this commit. Reviewer notes: This commit will not show an in-app message if you were to add it to a sample app. Autolayout constraints need to be added to have Views resize and that will come in a future commit. commit-id:025c7bf5
…UIKit app Part of: https://linear.app/customerio/issue/MBL-310/create-a-uikit-uiview-that-displays-an-inline-in-app-message-sent-from This commit implements UIKit AutoLayout constraints to the Inline View and the WebView. Adding these constraints make inline messages visible inside of UIKit apps! The Inline View will start at height 0 and then after the webview content is loaded, it will instantly change the height to display the full in-app message. Testing: All testing is done manually in sample apps. AutoLayout constraints change the visuals of Views so viewing how the View looks in an app is required to test. If you want to test this commit in a sample app build, follow these instructions: * Open the UIKit app on device. * Immediately after the app opens, send yourself a test in-app message with element ID "test". * Wait 10 seconds. In this time, the in-app SDK will fetch the test in-app message and then will display it inline on the screen. commit-id:5ab2c403
Part of: https://linear.app/customerio/issue/MBL-310/create-a-uikit-uiview-that-displays-an-inline-in-app-message-sent-from Fetching of in-app message is an async operation running in the background. There is a chance that in-app messages are not available when the inline View is constructed but become available after. This commit gets notified when a fetch is complete so it can check if there are in-app messages to display after the View has already been constructed. Testing: * Automated tests are added. * Verified that messages display in sample app. If you want to test this commit in a sample app build, follow these instructions: 1. Kill the app on device. 2. Send yourself a test inline message from Fly. 3. Open app. After you wait a few seconds, the inline message you sent yourself will display. commit-id:aea90dff
… View Part of: https://linear.app/customerio/issue/MBL-311/provide-positive-ux-when-inline-messages-displayed-by-animating-height Fetching of in-app message is an async operation running in the background. There is a chance that an inline in-app message is rendered and ready to display on the screen currently in the foreground. We want to display the inline message when it's ready to show, and we also want to display these messages without causing a negative UX. This commit does the following: 1. Hides the inline View when the View is constructed by setting the height to 0. 2. When the in-app message web content is rendered, the inline View is given a height that matches the web content aspect ratio. The inline View animates it's height from 0 to the new height to make the inline View visible. Testing: This change is heavy on UI so testing is done mostly in UIKit sample app QA testing. If you want to test this commit in a sample app build, follow these instructions: 1. Open the app on device. 2. Send yourself a test inline message from Fly to one of the inline Views on Dashboard screen. 3. After you wait a few seconds, you will see the inline message animate into visibility. Notes for reviewer: * During development, I encountered difficulties with getting the animation to work when the inline View is embedded in a StackView. Because of this, I modified the Dashboard screen's UI to nest the inline Views inside of the StackView to make sure we QA test this scenario.
levibostian
force-pushed
the
levi/inline-animate-open
branch
from
fbc5274 to
434d587
Compare
Part of: https://linear.app/customerio/issue/MBL-310/create-a-uikit-uiview-that-displays-an-inline-in-app-message-sent-from As a follow-up to PR: #716, this change renames 2 Message properties to better explain the purpose of the properties in the SDK. Notes to reviewer: * `InAppMessage.messageId` is part of the public API. This change keeps backwards compatibility by not renaming this property.
levibostian
force-pushed
the
levi/inline-rename-variables
branch
from
0d5aa73 to
c177a65
Compare
The logic is actually backwards. I modified the test with the expected behavior.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature-inline-inapp #730 +/- ##
=======================================================
Coverage ? 58.81%
=======================================================
Files ? 143
Lines ? 4018
Branches ? 0
=======================================================
Hits ? 2363
Misses ? 1655
Partials ? 0 ☔ View full report in Codecov by Sentry. |
Base automatically changed from
levi/inline-animate-open
to
feature-inline-inapp
June 12, 2024 13:00
Shahroz16
approved these changes
Jun 12, 2024
| } | ||
|
|
||
| // Used to construct instance from API response | ||
| init(queueId: String? = nil, priority: Int? = nil, messageId: String, properties: [String: Any]?) { |
There was a problem hiding this comment.
Suggested change
| init(queueId: String? = nil, priority: Int? = nil, messageId: String, properties: [String: Any]?) { | |
| init(queueId: String? = nil, priority: Int? = nil, templateId: String, properties: [String: Any]?) { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of: https://linear.app/customerio/issue/MBL-310/create-a-uikit-uiview-that-displays-an-inline-in-app-message-sent-from
As a follow-up to PR: #716, this change renames 2 Message properties to better explain the purpose of the properties in the SDK.
Notes to reviewer:
InAppMessage.messageIdis part of the public API. This change keeps backwards compatibility by not renaming this property.Based on PR #724
Full chain of PRs as of 2024-05-30
levi/inline-rename-variables➔levi/inline-animate-openlevi/inline-animate-open➔levi/display-inline-uiview-afterfetchlevi/display-inline-uiview-afterfetch➔spr/main/5ab2c403spr/main/5ab2c403➔spr/main/025c7bf5spr/main/025c7bf5➔spr/main/60639329spr/main/60639329➔feature-inline-inapp