[CHNL-16220] present webview from host app #259
Conversation
| defer { isLoading = false } | ||
|
|
||
| try await viewModel.preloadWebsite(timeout: 8_000_000_000) | ||
| topController.present(viewController, animated: true, completion: nil) |
There was a problem hiding this comment.
Could this have changed after 8 seconds?
There was a problem hiding this comment.
Potentially. I can change the timeout to whatever we think would be appropriate. I just didn't want to wait forever for a website that never loads.
Any suggestions for what I should set the timeout to? 15 seconds? 30?
There was a problem hiding this comment.
Sorry I mean the top view controller could that change before the site is loaded? The eight seconds is maybe fine...i wish we had a good way to track metrics like this from the sdk.
There was a problem hiding this comment.
it's okay if the topController changes before the site loads, that's sorta the point of preloading. Notice the video above; I tap the "Show Picsum Overlay" button, which kicks off the preload. While it's preloading, I'm still able to navigate to different tabs (which changes the topController). Then, when the preload finishes, it presents the web view on top of whatever the current topController is
ok good catch. I changed it so that it grabs the topController after the preload finishes and right before presenting.
| @@ -5,7 +5,7 @@ | |||
| // Created by Andrew Balmer on 1/21/25. | |||
| // | |||
|
|
|||
| @testable import KlaviyoUI | |||
| @testable @_spi(KlaviyoPrivate) import KlaviyoUI | |||
There was a problem hiding this comment.
Do you need this for a test if it's public already ( and 'testable')?
There was a problem hiding this comment.
yeah, it won't compile without both @testable and @_spi
|
|
||
| private let (navEventStream, navEventContinuation) = AsyncStream.makeStream(of: WKNavigationEvent.self) | ||
| public let (navEventStream, navEventContinuation) = AsyncStream.makeStream(of: WKNavigationEvent.self) |
There was a problem hiding this comment.
Seems like a lot of public things in here...
There was a problem hiding this comment.
yeah, this was a tradeoff I made in order to make this change possible. I thought about dropping the KlaviyoWebViewModeling protocol and providing default implementation within the KlaviyoWebViewModel class, then allowing customization through subclassing, but I didn't like that approach because it doesn't force subclassers to implement their own script handlers
and removed implementation from `KlaviyoWebViewModel` & `JSTestWebViewModel`
this ensures that the load scripts are injected before the WKWebView loads the URL
ab1470
force-pushed
the
ab/CHNL-16220/present-webview-from-host-app
branch
from
0a04b9d to
f4186e7
Compare
Description
This PR adds the architecture to a) preload a website, and b) once the site is preloaded, present the website as a modal on top of whatever screen is currently visible.
I have also marked some objects as
@_spi(KlaviyoPrivate), because we need to use these objects in the iOS test app without exposing them publicly to consumers of the SKD.I also added some default implementation for the
preloadWebsiteandhandleNavigationEventfunctions to theKlaviyoWebViewModelingprotocol.Check List
Manual Test Plan
Supporting Materials
This video shows the example I wired up in the iOS test app. After tapping the button, the KlavyioWebViewController preloads an "expensive" website (it's loading a very large image). After the preload completes, the image appears as a modal. Notice that I can still navigate through the app while the site is preloading.
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-01-22.at.01.20.49.mp4