Flyouts POC + clean up by belleklaviyo · Pull Request #535 · klaviyo/klaviyo-swift-sdk

belleklaviyo · 2026-03-30T20:45:23Z

Description

This was the original POC work. The branch was misnamed feat/flyouts-forms but will be going into an actual feature branch feat/iaf-flyouts

Due Diligence

I have tested this on a simulator or a physical device.
I have added sufficient unit/integration tests of my changes.
I have adjusted or added new test cases to team test docs, if applicable.
I am confident these changes are compatible with all iOS and XCode versions the SDK currently supports.

Release/Versioning Considerations

Patch Contains internal changes or backwards-compatible bug fixes.
Minor Contains changes to the public API.
Major Contains breaking changes.
Contains readme or migration guide changes.
- If so, please merge to a feature branch so documentation updates only go live upon official release.
This is planned work for an upcoming release.
- If no, author or reviewer should account for this in a release plan, or describe why not below.

Changelog / Code Overview

Test Plan

Related Issues/Tickets

…entationManager

…x test

Sources/KlaviyoForms/InAppForms/InAppWindowManager.swift

ab1470

btw sorry for hitting the "request changes" but I think I need to either do that or "Approve" for SLO purposes

Sources/KlaviyoForms/InAppForms/Models/FormLayout.swift

Sources/KlaviyoForms/InAppForms/Models/InAppFormsConfig.swift

Sources/KlaviyoForms/InAppForms/InAppWindowManager.swift

ab1470 · 2026-04-02T20:35:52Z

Sources/KlaviyoForms/InAppForms/Models/IAFNativeBridgeEvent.swift

        case .formWillAppear:
-            let payload = try? container.decode(FormWillAppearPayload.self, forKey: .data)
-            self = .formWillAppear(formId: payload?.formId, formName: payload?.formName)
+            let decodedData = try container.decode(AnyCodable.self, forKey: .data)


still thinking about this and whether there's an alternative to using AnyCodable here, will get back to you but first I'll submit my review so you can look at my other comments

belleklaviyo · 2026-04-03T13:25:46Z

to clean up the git history, I created a new feature branch feat/iaf-flyouts that this cleaned up POC will be merged into, so it'll be one clean commit. then other work (like #537) can be cleanly merged in as well.

… events Combines flyout/flexible form layout support (IAFWindowManager, FormLayout, FormPosition) with form-context propagation through lifecycle events (formId/formName in dismiss and formDisappeared events). Conflict resolutions: - IAFLifecycleEvent: keep withLayout param on present, add formId/formName to dismiss - IAFNativeBridgeEvent: keep formWillAppear(Data) for layout decoding; add formDisappeared(formId:formName:) and FormContextPayload from iaf-flyouts - IAFWebViewModel: keep HEAD layout-decoding path for formWillAppear; adopt case let .formDisappeared(formId, formName) from iaf-flyouts - IAFPresentationManager: thread formId/formName through presentForm and presentFormAsModal; keep InAppWindowManager guard in destroyWebView; adopt effectiveFormId/Name fallback in dismissForm - FormLifecycleHandlerTests: adopt iaf-flyouts tearDown (no handleFormEvent reset needed since context now flows through events) Fix: correct withLayout: label in pattern matches for IAFLifecycleEvent.present in IAFPresentationManager and IAFWebViewModelTests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Sources/KlaviyoForms/InAppForms/IAFPresentationManager.swift

cursor · 2026-04-03T14:17:39Z

Sources/KlaviyoForms/InAppForms/Models/IAFNativeBridgeEvent.swift

-            self = .formWillAppear(formId: payload?.formId, formName: payload?.formName)
+            let decodedData = try container.decode(AnyCodable.self, forKey: .data)
+            let data = try JSONEncoder().encode(decodedData)
+            self = .formWillAppear(data)


formWillAppear decode now throws on missing data

Medium Severity

The formWillAppear decoding changed from try? (graceful fallback) to try (throwing). If the JS bridge sends a formWillAppear event with a missing or null data field, the entire event decode now throws, and the form silently fails to present. Previously it would still create the event with nil values and present a fullscreen form. The sibling formDisappeared case still uses try?, making this inconsistency likely unintentional.

@ab1470 if you're proposing some decoding improvements this may be addressed by your PR here?

yeah that's it. I would love to avoid AnyCodable if possible, my PR is a vibe-coded possible solution to doing that

Sources/KlaviyoForms/InAppForms/Models/IAFNativeBridgeEvent.swift

Tests/KlaviyoFormsTests/IAFNativeBridgeEventTests.swift

…e handshake test Flexible form presentation was not setting currentFormId/currentFormName, causing dismissForm/destroyWebView to lose form identity on dismiss if the bridge sends nil identifiers. Mirrors the existing modal path behaviour. Also updates the handshake snapshot test to expect formWillAppear v2, matching the version bump already in IAFNativeBridgeEvent.swift. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

cursor · 2026-04-03T20:50:01Z

Tests/KlaviyoFormsTests/IAFWebViewModelTests.swift

        let lifecycleTask = Task {
            for await event in viewModel.formLifecycleStream {
-                if case let .present(formId, formName) = event {
+                if case let .present(formId, formName, withLayout: layout) = event {


Test expects old handshake version, will fail

Medium Severity

The testInjectHandshakeAttribute test in IAFWebViewModelTests.swift still expects formWillAppear at version: 1, but the production code in IAFNativeBridgeEvent.version was changed to return 2 for .formWillAppear. The corresponding test in IAFNativeBridgeEventTests.swift was correctly updated to version 2, but this test was missed. This will cause a test failure.

Additional Locations (1)

Sources/KlaviyoForms/InAppForms/Models/IAFNativeBridgeEvent.swift#L169-L170

evan-masseau

Commenting to silence reminders for now, since there's already some pending feedback. Will get back to this once feedback addressed and/or once i've had time to manually test. My read-thru won't be that helpful anyway.

belleklaviyo added 18 commits January 27, 2026 10:52

Add FormLayout structs

ae0a72b

Add presentation style to InAppFormsConfig

129b445

Add InAppWindowManager

b176f58

Remove misreturn, make InAppWindowManager a singleton

237ac46

Add presentWithLayout event

a0950aa

Create helper to determine if floating form window is active

e1dcf62

Implement branching logic for modal vs window presentation in IAFPres…

aa48e7c

…entationManager

Mock custom layout coming from js

3c0905b

Remove unused property

f9f9df9

Prevent mislayout during orientation changes

2357df4

Clean up guards

cf576f3

Reorganize code

a97f488

Unify legacy present with presentWithLayout action, clean up mock, fi…

158ebe2

…x test

Address cursor comments

04be763

Parse actual layout data coming from js

b1192c3

Merge branch 'master' into spike/flyouts-poc

68b9ccd

Fix formWillAppear test structure and uncomment log

b5bc175

Address bugbot comments

1fe5e8f

cursor bot reviewed Mar 30, 2026

View reviewed changes

Sources/KlaviyoForms/InAppForms/InAppWindowManager.swift Show resolved Hide resolved

ab1470 requested changes Apr 2, 2026

View reviewed changes

belleklaviyo changed the base branch from master to feat/iaf-flyouts April 3, 2026 13:23

belleklaviyo changed the title ~~feat/flyouts forms~~ Flyouts POC + clean up Apr 3, 2026

cursor bot reviewed Apr 3, 2026

View reviewed changes

belleklaviyo added 2 commits April 3, 2026 15:23

Bump formWillAppear to v2

2b972e9

Tidy up Margins

b478b71

cursor bot reviewed Apr 3, 2026

View reviewed changes

Sources/KlaviyoForms/InAppForms/Models/IAFNativeBridgeEvent.swift Show resolved Hide resolved

belleklaviyo added 2 commits April 3, 2026 15:41

Fix centering for top and bottom (cursor comment)

da65d64

Respond to open comments, tidy up docs

1bcdb31

Remove redundant effectiveWidth/effectiveHeight

5ca8a6d

cursor bot reviewed Apr 3, 2026

View reviewed changes

Tests/KlaviyoFormsTests/IAFNativeBridgeEventTests.swift Show resolved Hide resolved

cursor bot reviewed Apr 3, 2026

View reviewed changes

belleklaviyo marked this pull request as ready for review April 3, 2026 21:00

belleklaviyo requested a review from a team as a code owner April 3, 2026 21:00

klaviyoit requested a review from evan-masseau April 3, 2026 21:00

evan-masseau reviewed Apr 6, 2026

View reviewed changes

Conversation

belleklaviyo commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Due Diligence

Release/Versioning Considerations

Changelog / Code Overview

Test Plan

Related Issues/Tickets

Uh oh!

Uh oh!

ab1470 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ab1470 Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

belleklaviyo commented Apr 3, 2026

Uh oh!

Uh oh!

cursor bot Apr 3, 2026

Choose a reason for hiding this comment

formWillAppear decode now throws on missing data

Uh oh!

belleklaviyo Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

ab1470 Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Apr 3, 2026

Choose a reason for hiding this comment

Test expects old handshake version, will fail

Uh oh!

evan-masseau left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

belleklaviyo commented Mar 30, 2026 •

edited

Loading

`formWillAppear` decode now throws on missing data