Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[connectivity] Bring "experimental" connectivity web to main repo. #2820

Merged
merged 35 commits into from
Jun 30, 2020

Conversation

ditman
Copy link
Member

@ditman ditman commented Jun 8, 2020

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.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • No, this is not a breaking change.

ditman added 30 commits April 7, 2020 15:15
```
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"...
@ditman ditman requested a review from cyanglaz June 8, 2020 18:16
@ditman ditman force-pushed the connectivity-for-web branch from 8f2b9e3 to ce3d486 Compare June 8, 2020 18:21
@ditman ditman requested a review from mehmetf June 8, 2020 18:31

```
cd test
flutter run -d chrome
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flutter test?

Copy link
Member Author

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.

/// Checks the connection status of the device.
@override
Future<ConnectivityResult> checkConnectivity() async {
return html.window.navigator.onLine
Copy link
Contributor

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)

Copy link
Member Author

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':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about 4g?

Copy link
Member Author

@ditman ditman Jun 30, 2020

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.

@ditman ditman merged commit 388c66e into flutter:master Jun 30, 2020
@ditman ditman deleted the connectivity-for-web branch June 30, 2020 03:29
agent3bood pushed a commit to agent3bood/flutter-plugins that referenced this pull request Jul 10, 2020
…er#2820)

Move the package formerly known as `experimental_connectivity_web` to flutter/plugins master, as `connectivity_for_web`.
jorgefspereira pushed a commit to jorgefspereira/plugins_flutter that referenced this pull request Oct 10, 2020
…er#2820)

Move the package formerly known as `experimental_connectivity_web` to flutter/plugins master, as `connectivity_for_web`.
FlutterSu pushed a commit to FlutterSu/flutter-plugins that referenced this pull request Nov 20, 2020
…er#2820)

Move the package formerly known as `experimental_connectivity_web` to flutter/plugins master, as `connectivity_for_web`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants