-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[url_launcher] Use url_launcher_platform_interface
to handle calls
#2228
[url_launcher] Use url_launcher_platform_interface
to handle calls
#2228
Conversation
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.
LGTM.
I'd suggest waiting for #2230 to land first and adapt the mock.
}); | ||
} | ||
|
||
class MockUrlLauncher extends Mock implements UrlLauncherPlatform {} |
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.
This was a red flag for me as I was planning to enforce that it's not implemented with implements
.
I sent #2230 which leaves a back door for mocks.
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.
Done.
@@ -14,10 +14,13 @@ flutter: | |||
dependencies: | |||
flutter: | |||
sdk: flutter | |||
url_launcher_platform_interface: ^1.0.0 |
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.
If this lands after #2230 lets make this ^1.01
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.
Done.
|
||
dev_dependencies: | ||
flutter_test: | ||
sdk: flutter | ||
test: ^1.3.0 | ||
mockito: ^3.0.0 |
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.
Seems like latest mockito is 4.1.1 any reason to use v3?
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.
No strong preference, just saw that ^3.0.0
was used in other plugins in this repo.
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.
Switched to ^4.1.1
…lutter#2228) * [url_launcher] Use `url_launcher_platform_interface` to handle calls * Exclude platform interface from all-plugins-app * Update now that flutter#2230 has landed
…lutter#2228) * [url_launcher] Use `url_launcher_platform_interface` to handle calls * Exclude platform interface from all-plugins-app * Update now that flutter#2230 has landed
Description
Forward calls from
package:url_launcher
to the platform-specific implementation declared inUrlLauncherPlatform.instance
.Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?