-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for registering Strada components per web view #191
Conversation
…ers in the Turbo config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch removing the references to TurboNavigationHierarchyController
, that cleans things up a bit!
I made one suggestion and one request, the latter hopefully improving the DX a bit.
@@ -289,7 +289,9 @@ extension TurboNavigator { | |||
guard let _ = session.activeVisitable?.visitableViewController, | |||
let url = session.activeVisitable?.visitableURL else { return } | |||
|
|||
let newSession = Session(webView: Turbo.config.makeWebView()) | |||
let newSession = Session( | |||
webView: Turbo.config.makeWebView(for: session == self.session ? .main : .modal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
webView: Turbo.config.makeWebView(for: session == self.session ? .main : .modal) | |
webView: Turbo.config.makeWebView(for: session === self.session ? .main : .modal) |
@@ -19,7 +19,11 @@ public class TurboConfig { | |||
|
|||
/// Optionally customize the web views used by each Turbo Session. | |||
/// Ensure you return a new instance each time. | |||
public var makeCustomWebView: WebViewBlock = { (configuration: WKWebViewConfiguration) in | |||
public var mainWebViewProvider: WebViewBlock = { configuration in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to keep the old function and only add the two new ones. That gives developers a way to customize both at the same time, like in the Demo app, or dive into more specific customizations.
@joemasilotti, I discussed this PR with @jayohms, and we've decided to close it. This approach doesn't address one of our specific cases, and the default implementation should suffice for most users. We'll revisit the approach once we begin integrating TurboNavigator into our apps. |
No description provided.