-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
introduce share_plus_web #20
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
Conversation
@mhadaily can you please review this. |
@balvinderz thanks for the great contribution, I will review it shortly, before that, we would like to have integration and unit tests from the plugins. Can you please make sure you can write tests for this PR? |
@mhadaily i have added the integration tests now. |
Hi @balvinderz can you share which command do you use to run the integration test? |
@miquelbeltran first open chromedriver at 4444 port . then be in test directory and then type this command . |
Thanks @balvinderz that worked for me. @mhadaily what do you think? |
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.
Thanks! Should it also have dep/plugin entries in packages/share_plus/pubspec.yaml?
SharePlugin({@visibleForTesting html.Navigator debugNavigator}) | ||
: _navigator = debugNavigator ?? html.window.navigator; | ||
|
||
Future<void> share( |
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.
- missing @OverRide?
- /// Share text.
@jpnurmi yes ,but if i add it now , the tests will fail because share_plus_web doesn't exist. so i was thinking of making another PR when this is merged. |
Ah, okay! 👍 |
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.
Windows slipped in in a couple of places in the README, otherwise looks good!
Fixed the typos |
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.
Thanks!
Description
Adds web support for share_plus plugin.
Related Issues
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. Updating the
pubspec.yaml
and changelogs is not required.///
).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?