feat: Add form lifecycle event bridge for in-app forms (#73)#75
Merged
Conversation
evan-masseau
commented
Apr 10, 2026
| } | ||
|
|
||
| def klaviyoSdkVersion = "4.3.1" | ||
| def klaviyoSdkVersion = "0a48d79980dc3f276422cf9c42ff145eb39f364a" |
Contributor
Author
There was a problem hiding this comment.
Will need to update this with a proper SDK version once released obvs
dan-peluso
reviewed
Apr 10, 2026
dan-peluso
reviewed
Apr 10, 2026
dan-peluso
reviewed
Apr 10, 2026
dan-peluso
reviewed
Apr 10, 2026
dan-peluso
reviewed
Apr 10, 2026
dan-peluso
reviewed
Apr 10, 2026
dan-peluso
reviewed
Apr 10, 2026
dan-peluso
reviewed
Apr 10, 2026
9 tasks
evan-masseau
added a commit
that referenced
this pull request
Apr 10, 2026
Addresses Dan's review feedback on PR #75 about cleaning up the lifecycle handler when the Flutter app stops. On Android, we now unregister in onDetachedFromEngine; on iOS, in onCancel (stream handler teardown). Both are best-effort to avoid crashing during plugin teardown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
evan-masseau
added a commit
that referenced
this pull request
Apr 14, 2026
* fix(forms): throw on missing required fields in bridge message parsing FormLifecycleEvent.fromMap now validates that required fields (formId, formName, event type, and buttonLabel for CTA events) are present and non-empty, throwing ArgumentError instead of silently falling back to empty strings. The call site in KlaviyoSdk wraps parsing in try/catch to log and drop malformed events rather than crashing the stream. Also makes deepLinkUrl nullable on FormCtaClicked since it is genuinely optional — a CTA may not have a deep link configured. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: use expand() for cleaner null-free stream filtering Replace map+where+cast chain with expand() which naturally emits 0 or 1 elements, eliminating the nullable intermediate type. Also fix test using empty formName in equality assertion for consistency with validation rules. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: unregister form lifecycle handler on engine detach Addresses Dan's review feedback on PR #75 about cleaning up the lifecycle handler when the Flutter app stops. On Android, we now unregister in onDetachedFromEngine; on iOS, in onCancel (stream handler teardown). Both are best-effort to avoid crashing during plugin teardown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): revert deepLinkUrl to required — native SDKs only dispatch CTA events when URL exists Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * revert: remove unnecessary lifecycle handler cleanup on engine detach The unregisterFormLifecycleHandler() calls in onDetachedFromEngine (Android) and onCancel (iOS) are unnecessary — when the engine detaches the event sink is already null so lifecycle events would no-op. Reaching into the native SDK during teardown is fragile and not standard Flutter plugin cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): allow empty buttonLabel on CTA events Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * docs(forms): remove internal implementation details from public API docs Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(forms): default buttonLabel to empty string instead of rejecting null Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dan-peluso
reviewed
Apr 15, 2026
dan-peluso
reviewed
Apr 15, 2026
dan-peluso
reviewed
Apr 15, 2026
| // safe singleton; its constructor only creates stream controllers, not platform | ||
| // channel calls. This allows onFormLifecycleEvent (and other streams) to be | ||
| // subscribed before initialize() is called, matching native SDK behavior. | ||
| final KlaviyoNativeWrapper _nativeWrapper = KlaviyoNativeWrapper(); |
Contributor
There was a problem hiding this comment.
This is fine as it stands, since that wrapper is already a singleton and there's no side effects happening - but I think we should maybe add a comment somewhere that specifies if we add logic to the klaviyo native wrapper constructor then we have to revisit this
dan-peluso
approved these changes
Apr 15, 2026
evan-masseau
added a commit
that referenced
this pull request
Apr 16, 2026
* fix(forms): throw on missing required fields in bridge message parsing FormLifecycleEvent.fromMap now validates that required fields (formId, formName, event type, and buttonLabel for CTA events) are present and non-empty, throwing ArgumentError instead of silently falling back to empty strings. The call site in KlaviyoSdk wraps parsing in try/catch to log and drop malformed events rather than crashing the stream. Also makes deepLinkUrl nullable on FormCtaClicked since it is genuinely optional — a CTA may not have a deep link configured. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: use expand() for cleaner null-free stream filtering Replace map+where+cast chain with expand() which naturally emits 0 or 1 elements, eliminating the nullable intermediate type. Also fix test using empty formName in equality assertion for consistency with validation rules. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: unregister form lifecycle handler on engine detach Addresses Dan's review feedback on PR #75 about cleaning up the lifecycle handler when the Flutter app stops. On Android, we now unregister in onDetachedFromEngine; on iOS, in onCancel (stream handler teardown). Both are best-effort to avoid crashing during plugin teardown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): revert deepLinkUrl to required — native SDKs only dispatch CTA events when URL exists Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * revert: remove unnecessary lifecycle handler cleanup on engine detach The unregisterFormLifecycleHandler() calls in onDetachedFromEngine (Android) and onCancel (iOS) are unnecessary — when the engine detaches the event sink is already null so lifecycle events would no-op. Reaching into the native SDK during teardown is fragile and not standard Flutter plugin cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): allow empty buttonLabel on CTA events Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * docs(forms): remove internal implementation details from public API docs Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(forms): default buttonLabel to empty string instead of rejecting null Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
evan-masseau
force-pushed
the
feat/form-lifecycle-hooks
branch
from
April 16, 2026 18:32
fd7ecfb to
bc55e62
Compare
* feat: add form lifecycle event bridge for in-app forms Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: configure deps for form lifecycle hooks testing iOS SDK: feat/form-lifecycle-hooks branch Android SDK: 0a48d79980dc3f276422cf9c42ff145eb39f364a Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: use camelCase for event names across bridge and Dart Align with RN SDK and iOS native eventName convention. Bridge wire format: formShown, formDismissed, formCtaClicked. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(forms): throw on missing required fields in bridge message parsing FormLifecycleEvent.fromMap now validates that required fields (formId, formName, event type, and buttonLabel for CTA events) are present and non-empty, throwing ArgumentError instead of silently falling back to empty strings. The call site in KlaviyoSdk wraps parsing in try/catch to log and drop malformed events rather than crashing the stream. Also makes deepLinkUrl nullable on FormCtaClicked since it is genuinely optional — a CTA may not have a deep link configured. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: use expand() for cleaner null-free stream filtering Replace map+where+cast chain with expand() which naturally emits 0 or 1 elements, eliminating the nullable intermediate type. Also fix test using empty formName in equality assertion for consistency with validation rules. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: unregister form lifecycle handler on engine detach Addresses Dan's review feedback on PR #75 about cleaning up the lifecycle handler when the Flutter app stops. On Android, we now unregister in onDetachedFromEngine; on iOS, in onCancel (stream handler teardown). Both are best-effort to avoid crashing during plugin teardown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): revert deepLinkUrl to required — native SDKs only dispatch CTA events when URL exists Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * revert: remove unnecessary lifecycle handler cleanup on engine detach The unregisterFormLifecycleHandler() calls in onDetachedFromEngine (Android) and onCancel (iOS) are unnecessary — when the engine detaches the event sink is already null so lifecycle events would no-op. Reaching into the native SDK during teardown is fragile and not standard Flutter plugin cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): allow empty buttonLabel on CTA events Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * docs(forms): remove internal implementation details from public API docs Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(forms): default buttonLabel to empty string instead of rejecting null Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…detach
- Android: wrap eventSink.success() in Handler(Looper.getMainLooper()).post {}
to guarantee EventChannel calls happen on the UI thread
- Android: call Klaviyo.unregisterFormLifecycleHandler() in onDetachedFromEngine
to prevent plugin instance being kept alive by the Klaviyo singleton
- iOS: wrap eventSink call in DispatchQueue.main.async for thread safety
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…subscription KlaviyoNativeWrapper is a safe singleton — its constructor only creates stream controllers, not platform channel calls. Making _nativeWrapper a final field instead of late means onFormLifecycleEvent can be subscribed before initialize() is called, matching native SDK behavior on Android/iOS. Reverts unnecessary isInitialized guard added to example app. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Prevent duplicate handler registration by unregistering before re-registering on both Android and iOS - Narrow catch-all in onDetachedFromEngine to catch MissingKlaviyoModule specifically, with a warning log for unexpected exceptions - Allow empty formName (only reject null, matching buttonLabel treatment) - Validate outer 'type' field in FormLifecycleEvent.fromMap - Clarify docs on buttonLabel default and deepLinkUrl presence - Add stream integration tests for filter, error-drop, and ordering Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…encing pattern Add Registry.log calls to all forms-related MissingKlaviyoModule catches for consistency with the geofencing error handling pattern. User-facing errors log at error level; cleanup/teardown paths log at verbose level. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…LifecycleHandler The outer catch already handles MissingKlaviyoModule, and registerForInAppForms is called before the unregister—so if forms is missing, we never reach this code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The entire method body needs the #if canImport(KlaviyoForms) guard since registerFormLifecycleHandler and unregisterFormLifecycleHandler are symbols from KlaviyoForms that must be stripped by the compiler when excluded. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
evan-masseau
force-pushed
the
feat/form-lifecycle-hooks
branch
from
April 16, 2026 18:34
bc55e62 to
c1e42d9
Compare
…/5.3.0) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 92eaf2d. Configure here.
The Logger.klaviyoFlutterSDK extension is @available(iOS 14.0, *), so the call in the #else branch of subscribeToFormLifecycleEvents needs a matching if #available guard to match all other call sites in the file. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ab1470
pushed a commit
that referenced
this pull request
Apr 20, 2026
* fix(forms): throw on missing required fields in bridge message parsing FormLifecycleEvent.fromMap now validates that required fields (formId, formName, event type, and buttonLabel for CTA events) are present and non-empty, throwing ArgumentError instead of silently falling back to empty strings. The call site in KlaviyoSdk wraps parsing in try/catch to log and drop malformed events rather than crashing the stream. Also makes deepLinkUrl nullable on FormCtaClicked since it is genuinely optional — a CTA may not have a deep link configured. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: use expand() for cleaner null-free stream filtering Replace map+where+cast chain with expand() which naturally emits 0 or 1 elements, eliminating the nullable intermediate type. Also fix test using empty formName in equality assertion for consistency with validation rules. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: unregister form lifecycle handler on engine detach Addresses Dan's review feedback on PR #75 about cleaning up the lifecycle handler when the Flutter app stops. On Android, we now unregister in onDetachedFromEngine; on iOS, in onCancel (stream handler teardown). Both are best-effort to avoid crashing during plugin teardown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): revert deepLinkUrl to required — native SDKs only dispatch CTA events when URL exists Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * revert: remove unnecessary lifecycle handler cleanup on engine detach The unregisterFormLifecycleHandler() calls in onDetachedFromEngine (Android) and onCancel (iOS) are unnecessary — when the engine detaches the event sink is already null so lifecycle events would no-op. Reaching into the native SDK during teardown is fragile and not standard Flutter plugin cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): allow empty buttonLabel on CTA events Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * docs(forms): remove internal implementation details from public API docs Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(forms): default buttonLabel to empty string instead of rejecting null Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
evan-masseau
added a commit
that referenced
this pull request
May 1, 2026
* fix(forms): throw on missing required fields in bridge message parsing FormLifecycleEvent.fromMap now validates that required fields (formId, formName, event type, and buttonLabel for CTA events) are present and non-empty, throwing ArgumentError instead of silently falling back to empty strings. The call site in KlaviyoSdk wraps parsing in try/catch to log and drop malformed events rather than crashing the stream. Also makes deepLinkUrl nullable on FormCtaClicked since it is genuinely optional — a CTA may not have a deep link configured. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: use expand() for cleaner null-free stream filtering Replace map+where+cast chain with expand() which naturally emits 0 or 1 elements, eliminating the nullable intermediate type. Also fix test using empty formName in equality assertion for consistency with validation rules. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: unregister form lifecycle handler on engine detach Addresses Dan's review feedback on PR #75 about cleaning up the lifecycle handler when the Flutter app stops. On Android, we now unregister in onDetachedFromEngine; on iOS, in onCancel (stream handler teardown). Both are best-effort to avoid crashing during plugin teardown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): revert deepLinkUrl to required — native SDKs only dispatch CTA events when URL exists Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * revert: remove unnecessary lifecycle handler cleanup on engine detach The unregisterFormLifecycleHandler() calls in onDetachedFromEngine (Android) and onCancel (iOS) are unnecessary — when the engine detaches the event sink is already null so lifecycle events would no-op. Reaching into the native SDK during teardown is fragile and not standard Flutter plugin cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): allow empty buttonLabel on CTA events Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * docs(forms): remove internal implementation details from public API docs Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(forms): default buttonLabel to empty string instead of rejecting null Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
12 tasks
9 tasks
ab1470
added a commit
that referenced
this pull request
May 5, 2026
* chore: bump version to 0.2.0 and remove interactive confirm from bump script Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: allow re-initialization with a new API key (#80) * Fix local SDK setup script and bump AGP version for compatibility (#76) * fix: allow re-initialization with a new API key Remove the early-return guards from both KlaviyoSDK.initialize() and KlaviyoNativeWrapper.initialize() so that calling initialize() again with a different API key forwards the new key to the native layer, matching Android and iOS SDK behavior. The event channel listener is still set up only once to avoid duplicate events. Closes MAGE-507 Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(review): move _apiKey assignment after native call, eagerly init wrapper Address review findings: - _apiKey is now set only after the native initialize() succeeds, so a failed re-init preserves the previous key instead of corrupting state - _nativeWrapper is now a final field (eagerly constructed singleton) instead of being re-assigned on every initialize() call - Added test for failed re-initialization preserving previous apiKey Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix: dart format test file -Claude * fix(bugbot): separate event listener flag, await async test assertion Two BugBot findings: - Use dedicated _eventListenerAttached flag instead of _isInitialized to guard event channel subscription, so a failed first init doesn't leave the flag unset and cause duplicate subscriptions on retry - Fix failed-reinit test to use `await expectLater(future, throwsA(...))` instead of the unawaited `expect(() => future, throwsA(...))` which passed trivially without validating the actual error behavior Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * feat: Add form lifecycle event bridge for in-app forms (#73) * feat: add form lifecycle event bridge for in-app forms Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: configure deps for form lifecycle hooks testing iOS SDK: feat/form-lifecycle-hooks branch Android SDK: 0a48d79980dc3f276422cf9c42ff145eb39f364a Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: use camelCase for event names across bridge and Dart Align with RN SDK and iOS native eventName convention. Bridge wire format: formShown, formDismissed, formCtaClicked. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): validate required bridge message fields (MAGE-484) (#78) * fix(forms): throw on missing required fields in bridge message parsing FormLifecycleEvent.fromMap now validates that required fields (formId, formName, event type, and buttonLabel for CTA events) are present and non-empty, throwing ArgumentError instead of silently falling back to empty strings. The call site in KlaviyoSdk wraps parsing in try/catch to log and drop malformed events rather than crashing the stream. Also makes deepLinkUrl nullable on FormCtaClicked since it is genuinely optional — a CTA may not have a deep link configured. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: use expand() for cleaner null-free stream filtering Replace map+where+cast chain with expand() which naturally emits 0 or 1 elements, eliminating the nullable intermediate type. Also fix test using empty formName in equality assertion for consistency with validation rules. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: unregister form lifecycle handler on engine detach Addresses Dan's review feedback on PR #75 about cleaning up the lifecycle handler when the Flutter app stops. On Android, we now unregister in onDetachedFromEngine; on iOS, in onCancel (stream handler teardown). Both are best-effort to avoid crashing during plugin teardown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): revert deepLinkUrl to required — native SDKs only dispatch CTA events when URL exists Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * revert: remove unnecessary lifecycle handler cleanup on engine detach The unregisterFormLifecycleHandler() calls in onDetachedFromEngine (Android) and onCancel (iOS) are unnecessary — when the engine detaches the event sink is already null so lifecycle events would no-op. Reaching into the native SDK during teardown is fragile and not standard Flutter plugin cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): allow empty buttonLabel on CTA events Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * docs(forms): remove internal implementation details from public API docs Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(forms): default buttonLabel to empty string instead of rejecting null Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): dispatch eventSink on main thread, unregister handler on detach - Android: wrap eventSink.success() in Handler(Looper.getMainLooper()).post {} to guarantee EventChannel calls happen on the UI thread - Android: call Klaviyo.unregisterFormLifecycleHandler() in onDetachedFromEngine to prevent plugin instance being kept alive by the Klaviyo singleton - iOS: wrap eventSink call in DispatchQueue.main.async for thread safety Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(example): guard lifecycle subscription on SDK initialization * fix(sdk): eagerly initialize native wrapper to allow pre-init stream subscription KlaviyoNativeWrapper is a safe singleton — its constructor only creates stream controllers, not platform channel calls. Making _nativeWrapper a final field instead of late means onFormLifecycleEvent can be subscribed before initialize() is called, matching native SDK behavior on Android/iOS. Reverts unnecessary isInitialized guard added to example app. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(forms): guard unregisterFormLifecycleHandler in onDetachedFromEngine * fix(forms): address review findings for form lifecycle hooks - Prevent duplicate handler registration by unregistering before re-registering on both Android and iOS - Narrow catch-all in onDetachedFromEngine to catch MissingKlaviyoModule specifically, with a warning log for unexpected exceptions - Allow empty formName (only reject null, matching buttonLabel treatment) - Validate outer 'type' field in FormLifecycleEvent.fromMap - Clarify docs on buttonLabel default and deepLinkUrl presence - Add stream integration tests for filter, error-drop, and ordering Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(forms): add logging to MissingKlaviyoModule catches matching geofencing pattern Add Registry.log calls to all forms-related MissingKlaviyoModule catches for consistency with the geofencing error handling pattern. User-facing errors log at error level; cleanup/teardown paths log at verbose level. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(forms): remove unnecessary nested try-catch around unregisterFormLifecycleHandler The outer catch already handles MissingKlaviyoModule, and registerForInAppForms is called before the unregister—so if forms is missing, we never reach this code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): wrap subscribeToFormLifecycleEvents in canImport guard The entire method body needs the #if canImport(KlaviyoForms) guard since registerFormLifecycleHandler and unregisterFormLifecycleHandler are symbols from KlaviyoForms that must be stripped by the compiler when excluded. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: target native SDK release branches (Android rel/4.4.0, iOS rel/5.3.0) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(forms): guard Logger call with iOS 14 availability check The Logger.klaviyoFlutterSDK extension is @available(iOS 14.0, *), so the call in the #else branch of subscribeToFormLifecycleEvents needs a matching if #available guard to match all other call sites in the file. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(android): emit push_notification_opened for tapped Klaviyo pushes (#83) * fix(example): defer notification permission to Push tab (#84) The example app's _setupFCM ran firebase_messaging.requestPermission() at app launch, prompting the user for the Android POST_NOTIFICATIONS permission before they had a chance to initialize the SDK with their API key. The Push tab already requests permission via permission_handler at the right time (when the user taps Enable Push Notifications), so the call in _setupFCM was redundant. Both APIs target the same POST_NOTIFICATIONS permission on Android 13+. Token-refresh and foreground-message listeners are unaffected — they don't depend on permission state. Fixes MAGE-547. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(android): emit push_notification_opened for tapped Klaviyo pushes The Klaviyo Android SDK namespaces push-notification extras with `com.klaviyo.` when building the tap PendingIntent (`KlaviyoRemoteMessage.appendKlaviyoExtras`), but `handleIntent` was checking `extras.containsKey("_k")` — the unprefixed key — so the check always failed and the event was dropped silently. Switch the guard to look for `com.klaviyo._k`, and strip the prefix from keys forwarded to Dart so the payload matches the iOS `userInfo` shape. Fixes MAGE-512. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(android): use Constants.PACKAGE_PREFIX / TRACKING_PARAMETER Replace inlined "com.klaviyo." / "_k" literals in handleIntent with references to com.klaviyo.core.Constants, so the Flutter plugin tracks the source of truth in the core SDK instead of duplicating it. Addresses PR #83 review feedback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(android): use Klaviyo.isKlaviyoNotificationIntent for guard Replace the manual Constants.PACKAGE_PREFIX + Constants.TRACKING_PARAMETER composition in handleIntent with the SDK's public extension Klaviyo.isKlaviyoNotificationIntent, which performs the same check and is the documented entry point for "did this intent originate from a Klaviyo push?". The prefix-strip loop still uses Constants.PACKAGE_PREFIX since there's no equivalent SDK helper for stripping keys. Addresses PR #83 review feedback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: consume KlaviyoSwift ~> 5.3.1 and Android 4.4.0 (#88) * consume Swift 5.3.0 and Android 4.4.0 * chore(ios): bump KlaviyoSwift dependency to ~> 5.3.1 5.3.0 has an upstream issue (MAGE-565): KlaviyoSwiftExtension 5.3.0 depends on KlaviyoCore 5.3.0, which uses UIApplication.shared in DeepLinkHandler.swift — that symbol is unavailable in app-extension contexts, so the example app's NotificationServiceExtension target fails to compile. The iOS team is publishing a 5.3.1 patch release to fix this; pin to ~> 5.3.1 now so we pick it up the moment it lands. iOS build verification deferred until 5.3.1 is published. Android side (klaviyoSdkVersion = "4.4.0") was already verified. References MAGE-454, MAGE-565. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(scripts): peg example app version to plugin version (#89) * chore(scripts): peg example app version to plugin version Extend sync_version.sh to also write the plugin version into example/pubspec.yaml (preserving any existing +<buildnumber> suffix), and sync the current value to 0.2.0+1. The pre-commit sync-sdk-version hook already runs this script on root pubspec.yaml changes, so future bumps stay in sync automatically. * chore(scripts): also sync MARKETING_VERSION in example pbxproj The RunnerTests and NotificationServiceExtension targets have hardcoded MARKETING_VERSION values in project.pbxproj (their xcconfig base is the CocoaPods-generated Pods-*.xcconfig, which doesn't include Generated.xcconfig, so they can't reference $(FLUTTER_BUILD_NAME)). Sed-update those literal numeric values to the plugin version. The main Runner target is unchanged — it reads $(FLUTTER_BUILD_NAME) via Info.plist as before. * fix(scripts): support pre-release versions in MARKETING_VERSION sync The previous character class [0-9.] only matched purely numeric versions, so once a pre-release value like 0.3.0-alpha.1 landed in the pbxproj (via bump_version.sh which supports semver pre-release tags), subsequent syncs couldn't find the value to replace and it'd go stale. Broaden to [0-9A-Za-z.-] so SemVer pre-release identifiers round-trip correctly. The class still excludes $/(/)/quotes, so $(FLUTTER_BUILD_NAME) refs on the main Runner target remain untouched. * fix(scripts,ci): strip build suffix from root version, extend drift check - sync_version.sh: strip any +<buildnumber> suffix from the root pubspec.yaml version up front. The plugin convention is to omit it, but if present we'd reassemble "$VERSION+$BUILD_NUMBER" into a malformed "0.3.0+2+1" when writing example/pubspec.yaml. Pre-release identifiers like 0.3.0-alpha.1 are preserved. - version-check.yml: also diff example/pubspec.yaml and example/ios/Runner.xcodeproj/project.pbxproj after running the sync script. Without this, drift in either file would slip past CI if someone bypasses the pre-commit hook with --no-verify. * chore(example): bind Runner target MARKETING_VERSION to FLUTTER_BUILD_NAME The main Runner target had no MARKETING_VERSION build setting, so Xcode's General → Identity Version field rendered blank even though Info.plist's CFBundleShortVersionString correctly resolved to $(FLUTTER_BUILD_NAME) at build time. The Identity panel binds to the build setting specifically, not Info.plist. Add MARKETING_VERSION = "$(FLUTTER_BUILD_NAME)"; to all three Runner configs (Debug, Release, Profile). FLUTTER_BUILD_NAME is in scope via Generated.xcconfig, included by Debug.xcconfig and Release.xcconfig. The sync_version.sh regex deliberately excludes $/(/)/quotes, so these dynamic references are not rewritten — they auto-resolve from pubspec.yaml on every Flutter build. * added privacy descriptions * added app description * chore(scripts): drop build-number suffix from example pubspec The example app is publish_to: 'none' and never ships to a store, so the +1 build number serves no purpose — Flutter defaults FLUTTER_BUILD_NUMBER to 1 when omitted. Drop it for consistency with the plugin's pubspec (which has no suffix) and simplify sync_version.sh by removing the build-number preservation branch. * docs(agents): note new sync_version.sh targets in gotcha This PR expanded the script to also write example/pubspec.yaml and the example app's pbxproj MARKETING_VERSION entries, but the gotcha entry still only mentioned the iOS plist. CLAUDE.md is a symlink to AGENTS.md so it picks up the change automatically. * docs(readme): update for 0.2.0 (Android 4.4.0 / iOS 5.3.0) (#90) * docs(readme): update for 0.2.0 (Android 4.4.0 / iOS 5.3.0) - Add Action Buttons subsection under Push Notifications - Note floating/flyout form layouts in In-App Forms intro - Replace raw onFormEvent example with typed onFormLifecycleEvent - Add FormLifecycleEvent types to API Reference * small readme update * docs(changelog): fill in 0.2.0 entry (#92) Replace the TODO placeholder with the full 0.2.0 changelog mirroring the GitHub release notes — What's New, Bug Fixes, Documentation, and updated Platform Support pinned to KlaviyoSwift 5.3.1 / Android 4.4.0. * fix(scripts): verify newly synced files in bump_version.sh (#93) `sync_version.sh` was extended in #89 to also sync the version into `example/pubspec.yaml` and the hardcoded `MARKETING_VERSION` entries in `example/ios/Runner.xcodeproj/project.pbxproj`, but the `check_file_version` block in `bump_version.sh` was not updated to verify those files. A silent sed-pattern mismatch in `sync_version.sh` would leave those files stale while `bump_version.sh` still reported "Version successfully bumped". - Add `check_file_version` for `example/pubspec.yaml` - Add a strict count check for `MARKETING_VERSION` entries in the example pbxproj — every hardcoded entry must match the new version. The dynamic `\$(FLUTTER_BUILD_NAME)` reference on the main Runner target is excluded by the same regex used in the sync script. Verified positive (NEW_VERSION=0.2.0 → 6/6 entries pass) and negative (NEW_VERSION=0.3.0 → 0/6 fails with ERRORS=1) cases. Addresses cursor[bot] review on #77. * fix(forms): tighten form lifecycle event handling for cross-platform parity (#94) * fix(forms): reject empty formName in FormLifecycleEvent.fromMap The `formName` validation was internally inconsistent with the rest of the parser — `event`, `formId`, and `deepLinkUrl` all reject both null and empty strings, but `formName` was only checking null. Tighten it to match. - `lib/src/models/form_lifecycle_event.dart`: extend the `formName` guard to include `formName.isEmpty` - `test/form_lifecycle_event_test.dart`: replace the `parses empty formName` case with `throws on empty formName`, mirroring the existing `throws on empty formId` pattern Addresses cursor[bot] review on #77. * fix(ios): use explicit switch mapping for form lifecycle event names The iOS form lifecycle handler was relying on the native Swift SDK's `event.eventName` property for the event-type bridge string, while the Android plugin explicitly mapped each subtype. The Dart `FormLifecycleEvent.fromMap` parser only accepts the literal strings `"formShown"`, `"formDismissed"`, and `"formCtaClicked"`, so any future change to the native `eventName` property would silently drop events on iOS (logged + swallowed by the stream's `expand` pipeline). Replace the property access with an explicit `switch event` that mirrors Android's `when (event)` block. The wrapper now owns its serialization contract end-to-end: - Cross-platform parity is enforced at the wrapper layer rather than inherited from a native API surface that wasn't designed for it - Adding a new event type natively becomes a Swift compile error (non-exhaustive `switch`) instead of a silent runtime drop - The `formCtaClicked` payload extraction (buttonLabel, deepLinkUrl) collapses into the same `switch` rather than a separate `if case let` afterward Addresses cursor[bot] review on #77. * docs(forms): add form lifecycle events section to README --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Andrew Balmer <ab1470@users.noreply.github.com>
This file contains hidden or 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
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.

Summary
Adds form lifecycle event bridging to the Flutter SDK, enabling Dart consumers to receive a stream of events when in-app forms are shown, dismissed, or when a CTA button is clicked.
Changes
FormLifecycleEventmodel — Dart sealed class withFormShown,FormDismissed, andFormCtaClickedsubtypes, each carryingformIdandformName, with CTA events also providingbuttonLabelanddeepLinkUrlonFormLifecycleEventstream onKlaviyoSdk— exposes lifecycle events via the existingEventChannelmechanismregisterForInAppFormsand unregisters onunregisterFromInAppForms, forwarding events through the Flutter event sinkregisterForInAppFormsand unregisters onunregisterFromInAppForms, forwarding events through the Flutter event sinkregisterForInAppForms/unregisterFromInAppFormsTest plan
FormLifecycleEventmodel parsingPart of MAGE-287