-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[connectivity] Bring "experimental" connectivity web to main repo. #2820
Conversation
This reverts commit 43494c6.
``` Checking that connectivity_web can be published. Suggestions: * line 1, column 1 of test/lib/main.dart: This package does not have connectivity_web_example in the `dependencies` section of `pubspec.yaml`. ╷ 1 │ import 'package:connectivity_web_example/src/connectivity_mocks.dart'; │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ╵ * line 2, column 1 of test/lib/main.dart: This package does not have e2e in the `dependencies` section of `pubspec.yaml`. ╷ 2 │ import 'package:e2e/e2e.dart'; │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ╵ * line 3, column 1 of test/lib/main.dart: This package does not have flutter_test in the `dependencies` section of `pubspec.yaml`. ╷ 3 │ import 'package:flutter_test/flutter_test.dart'; │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ╵ Package has 3 warnings. ```
Surprisingly, the script doesn't detect that "test" is a package of its own, and it requires the connectivity_web package to dev-depend on things that are only used by "test"...
Make private members final.
8f2b9e3
to
ce3d486
Compare
|
||
``` | ||
cd test | ||
flutter run -d chrome |
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.
flutter test
?
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.
Not yet. This requires the browser, so it's an unfinished e2e/driver test.
packages/connectivity/connectivity_for_web/lib/connectivity_for_web.dart
Outdated
Show resolved
Hide resolved
/// Checks the connection status of the device. | ||
@override | ||
Future<ConnectivityResult> checkConnectivity() async { | ||
return html.window.navigator.onLine |
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.
Heads up: these things will become non-null soon. Consider performing null checks using JS-interop, or this hack:
bool _unsafeIsNull(dynamic object) => object == null;
(I'm not sure if this hack will work for long though; dart2js can inline it and decide to elide the null check)
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.
Hm onLine
is a boolean; I hope the whole chain is not-null and defaults to "false" when null safety comes!
switch (effectiveType) { | ||
case 'slow-2g': | ||
case '2g': | ||
case '3g': |
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.
What about 4g?
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.
'4g' according to MDN is a fabled connection with 0 ping and infinite bandwidth. It falls through to the 'wifi' default.
…er#2820) Move the package formerly known as `experimental_connectivity_web` to flutter/plugins master, as `connectivity_for_web`.
…er#2820) Move the package formerly known as `experimental_connectivity_web` to flutter/plugins master, as `connectivity_for_web`.
…er#2820) Move the package formerly known as `experimental_connectivity_web` to flutter/plugins master, as `connectivity_for_web`.
Description
Connectivity for web has been baking for a while as "experimental_connectivity_web"; and as far as I know, we haven't received any major concerns with it.
This PR brings "experimental_connectivity_web" up to date with the latest master, and renames it to "connectivity_for_web" so it can become an endorsed implementation of "connectivity".
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?