feat(share_plus): remove direct dependence of url_launcher #1295
Conversation
|
Is this PR still in progress? |
|
Yes, I just need to fix some test. |
SirusCodes
force-pushed
the
remove_url_launcher_dep
branch
from
c2448c9 to
167c604
Compare
|
I will have to test on Monday before merging |
miquelbeltran
added
Hacktoberfest
Someone wanted to save a few KBs in their app, so why not. |
|
Thanks! |
| await urlLauncher.launchUrl(uri.toString(), const LaunchOptions()); | ||
| } else { | ||
| throw Exception('Unable to share on web'); | ||
| } | ||
| } |
There was a problem hiding this comment.
FYI: changing to this construction will break functionality, and has no advantages; see https://pub.dev/packages/url_launcher#checking-supported-schemes
There was a problem hiding this comment.
So, it's better to have only launchUrl in try catch block?
There was a problem hiding this comment.
Checking the return of launchUrl is better construct?
There was a problem hiding this comment.
So, it's better to have only
launchUrlin try catch block?
I'm not sure what the goal of a try/catch would be given that you are currently throwing.
Checking the return of launchUrl is better construct?
If you don't have a specific need to know in advance of trying (which is rare, and certainly isn't the case given the code here), checking what actually happened is better than using a potentially inaccurate prediction of what might happen, yes.
There was a problem hiding this comment.
If anyone can send a PR with the fix, I can review and merge asap
There was a problem hiding this comment.
Yeah I can do that.
Description
Remove direct dependence of url_launcher, instead depending on platform specific packages.
Related Issues
resolves #1070
Checklist
CHANGELOG.mdnor thepubspec.yamlfiles.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?
!in the title as explained in Conventional Commits).