Uh oh!
There was an error while loading. Please reload this page.
[webview_flutter_android] [webview_flutter_wkwebview] Adds support for PlatformNavigationDelegate.onUrlChange - #3653
Conversation
PlatformNavigationDelegate.onUrlChange
stuartmorgan-g
left a comment
There was a problem hiding this comment.
LGTM with some minor things.
Be sure to merge in master before landing this to pick up #3648. In this case it's fine to rebaseline (delete the package's lint-baseline.xml, then re-run the repo tool lint-android on the package), because otherwise you'll have to update Pigeon (to pick up the fixes to its generated code), and we shouldn't do that in this PR.
Uh oh!
There was an error while loading. Please reload this page.
| OCMStub([mockUrl absoluteString]).andReturn(@"https://www.google.com"); | ||
| FWFInstanceManager *instanceManager = [[FWFInstanceManager alloc] init]; | ||
| [instanceManager addDartCreatedInstance:mockUrl withIdentifier:0]; |
There was a problem hiding this comment.
Wasn't there a recent change that required this to be non-zero, or am I confusing this with something else?
There was a problem hiding this comment.
addDartCreatedInstance should be able to take a 0 since the Dart side reserves all identifiers 0->2^16
| /// The codec used by FWFNSUrlHostApi. | ||
| NSObject<FlutterMessageCodec> *FWFNSUrlHostApiGetCodec(void); | ||
| /// Host API for `NSUrl`. |
There was a problem hiding this comment.
Just to preserve this for posterity on the PR that actually lands it, I'm not sure NSURL is actually something we should wrap: flutter-team-archive/plugins#7113 (comment)
(But as I said there, it's fine to land this as-is, just something I want to have a record of when we're looking at the pros and cons of the direct-wrap approach.)
There was a problem hiding this comment.
I'll keep this in mind. My main rationale for it was that NSURL has some useful methods/constructors that are unique to iOS.
…pport for `PlatformNavigationDelegate.onUrlChange` (flutter/packages#3653)
…r `PlatformNavigationDelegate.onUrlChange` (flutter#3653) [webview_flutter_android] [webview_flutter_wkwebview] Adds support for `PlatformNavigationDelegate.onUrlChange`
…r `PlatformNavigationDelegate.onUrlChange` (flutter#3653) [webview_flutter_android] [webview_flutter_wkwebview] Adds support for `PlatformNavigationDelegate.onUrlChange`
…r `PlatformNavigationDelegate.onUrlChange` (flutter#3653) [webview_flutter_android] [webview_flutter_wkwebview] Adds support for `PlatformNavigationDelegate.onUrlChange`
Platform implementation PR for #3313
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.