readme updates #228
readme updates #228
Conversation
| ## In App Forms | ||
|
|
||
| ### Prerequisites | ||
| - A published "in-app" form in the Klaviyo forms portal |
There was a problem hiding this comment.
@ajaysubra should we include something like this?
There was a problem hiding this comment.
yeah I don't think this is needed. for instance we don't say having a push campaign is a prereq for setting up push. The audience here are other mobile devs so this prereq could just be what are the things needed for setting up IAFs on an android app.
| - Using version 3.2.0 or higher | ||
|
|
||
| ### Setup | ||
| The Klaviyo Push SDK for Android works as an extension of the Klaviyo object, and can be chained with your initialization code to allow for form display. |
There was a problem hiding this comment.
Do you mean the klaviyo in-app forms sdk?
There was a problem hiding this comment.
Thoughts on avoid internal details like The Klaviyo Push SDK for Android works as an extension of the Klaviyo object? Just calling out the setup steps, for instance this line could be,
registerForInAppFormscan be chained with your initialization code to allow for form display.
|
|
||
|
|
||
| Once you call the `registerForInAppForms` function, the web view will persist in the background until a form is ready to be shown, or a timeout occurs. | ||
| Consider how often you want to check form forms when you call this, for example: |
There was a problem hiding this comment.
By this you mean registerForInappForms right?
| Consider how often you want to check form forms when you call this, for example: | ||
| * `Activity.onCreate()` -> If you only want forms to appear over a specific activity, checks every time this activity is created. | ||
| * `Application.onResume()` -> Anytime the app is foregrounded, check for forms and show if available. | ||
| * `Activity.onDestroy()` -> If you need the forms to display only once an activity is removed from the view (ex. splash screen). |
There was a problem hiding this comment.
Is that a normal pattern calling on onDestroy? Will it latch onto the right activity?
| .initialize("KLAVIYO_PUBLIC_API_KEY", applicationContext) | ||
| .registerForInAppForms() | ||
| ``` | ||
| You can also call it publicly anywhere you can access the Klaviyo forms module: |
There was a problem hiding this comment.
Curious what you intended when you say "publicly" here?
| @@ -465,6 +467,37 @@ extensions such as `import com.klaviyo.pushFcm.KlaviyoRemoteMessage.body` to acc | |||
| We also provide an `Intent.appendKlaviyoExtras(RemoteMessage)` extension method, which attaches the data to your | |||
| notification intent that the Klaviyo SDK requires in order to track opens when you call `Klaviyo.handlePush(intent)`. | |||
|
|
|||
| ## In App Forms | |||
|
|
|||
| ### Prerequisites | |||
There was a problem hiding this comment.
Should we be clear on which SDK modules are required for in-app forms?
| - Using version 3.2.0 or higher | ||
|
|
||
| ### Setup | ||
| The Klaviyo Push SDK for Android works as an extension of the Klaviyo object, and can be chained with your initialization code to allow for form display. |
There was a problem hiding this comment.
Did you mean The Klaviyo Forms SDK?
|
|
||
|
|
||
| Once you call the `registerForInAppForms` function, the web view will persist in the background until a form is ready to be shown, or a timeout occurs. | ||
| Consider how often you want to check form forms when you call this, for example: |
There was a problem hiding this comment.
form forms
Looks like you got a little bit of duplication in there
There was a problem hiding this comment.
I guess what you intended was for forms
|
|
||
|
|
||
| Once you call the `registerForInAppForms` function, the web view will persist in the background until a form is ready to be shown, or a timeout occurs. | ||
| Consider how often you want to check form forms when you call this, for example: |
There was a problem hiding this comment.
nit: I feel like we could be a little clearer here in how we communicate that where registerForInAppForms is called will affect the behaviour. Just being super explicit on communication so there's no room for user error
| ### Behavior | ||
|
|
||
| The web view will listen for forms to display with the same timeout as other requests (10 seconds). | ||
| If no form is available to show, the web view will be removed from memory and there will need to be another call to `registerForInAppForms` |
There was a problem hiding this comment.
Can you clarify this a little bit? It sort of reads as a contradiction to the sentence you had above: Once you call the registerForInAppFormsfunction, the web view will persist in the background until a form is ready to be shown, or a timeout occurs.
There was a problem hiding this comment.
Actually re-reading everything I had overlooked part of that sentence. "Contradiction" is harsh, but maybe we could better summarize the behaviour of registerForInAppForms by leaving out notes on behaviour in the Setup section and letting this Behaviour section talk to it in one place?
Description
Check List
Changelog / Code Overview
Test Plan
Related Issues/Tickets