Always go through the adapter to decide how to propose visits #1054
Conversation
…pdate the window.location for external urls. This allows the turbo-ios and turbo-android adapters to see the proposed visits and handle them appropriately.
There was a problem hiding this comment.
These are great implementation-side changes!
I wish there were a way to add test coverage to prevent similar regressions in the future. At the moment, the suite doesn't have any tests that imitate native adapters, nor do they test these classes at the Unit level.
I wonder if there's a way to change the test harness to mimic the native adapter, or swap the adapter in the same way the native packages do in their bundled JS.
|
@seanpdoyle that's a great point — I would like the main native adapter methods to be tested. Really, we just need to swap the adapter for a native stub and verify that the correct methods are being called. We can let the native libraries continue to be responsible for testing their custom behaviors. I'm actually aware of another bug I need to look into soon where Frame Navigation doesn't appear to propose new visits through the adapter. Proper testing would help isolate issues like this and of course prevent regressions in the future. I'll look into this soon 👍 |
…t the proper methods are called for the native adapters
|
@seanpdoyle I've added unit tests for the native adapter interface, which would have caught this particular issue: 2414740 |
This fixes a bug introduced in: #410
#410 made a change that made the decision to visit internal vs external urls outside of the
adapter. This works fine in the browser context, but breaks the expectations for theturbo-iosandturbo-androidnative adapters.Native apps using the native turbo libraries already must provide logic for properly routing internal/external urls to the WebView or an external app/browser. This change brings back the capability for programmatic
Turbo.visit(url)proposals to flow through the adapter, even for external urls. This gives the native adapters the opportunity to pass external url visit proposals to the native apps.Before this change
Turbo.visit("https://google.com")would result in the native app's Turbo WebView navigating directly togoogle.com, bypassing the Turbo visit flow, which should never happen.Additionally this adds a set of unit tests for the native adapter interface to prevent regressions in the future. Previously, there weren't any unit tests for the native adapter interface.