Skip to content
This repository has been archived by the owner on Jan 27, 2024. It is now read-only.

Implementing Web support #51

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Faendis
Copy link

@Faendis Faendis commented Jun 3, 2021

This pull request is linked to issue #44.
Here you can find, an implementation of your plugin for web using google libphonenumber js library using https://github.com/natintosh/plugin_libphonenumber/tree/develop/libphonenumber_web as a starting point.

There is a deprecated call on MethodChannel constructor with Registrar.messenger, but I failed to get the plugin to work without using it.

@@ -11,16 +11,27 @@ environment:
dependencies:
flutter:
sdk: flutter
flutter_web_plugins:
sdk: flutter
js: ^0.6.3
Copy link
Contributor

Choose a reason for hiding this comment

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

does this need to indented?

Copy link
Author

Choose a reason for hiding this comment

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

it seems to me that it is necessary to be indented as it is written in How to Write a Flutter Web Plugin:

name: url_launcher_web
version: 0.0.1

flutter:
  plugin:
    platforms:
      web:
        pluginClass: UrlLauncherPlugin
        fileName: url_launcher_web.dart

dependencies:
  flutter:
    sdk: flutter
  flutter_web_plugins:
    sdk: flutter

environment:
  sdk: ">=2.0.0-dev.28.0 <3.0.0"
  flutter: ">=1.5.0 <2.0.0"

I'm still new in flutter development and I always saw dependencies indented therefore I'm sorry if I'm misunderstanding your question.
I also read this :

"Each node must be indented further than its parent node. All sibling nodes must use the exact same indentation level. However the content of each sibling node may be further indented independently. "
(YAML Specs v1.2 3rd edition.)

Doesn't know if this can help you but js package is on pub.dev (and allows to call js API in dart code) while flutter_web_plugins is in the flutter API (and provides the Registrar class implementing for web implementations).

Copy link
Contributor

Choose a reason for hiding this comment

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

I just want to make sure we are not having the js dependency in my mobile apps that do not use web.

Copy link

Choose a reason for hiding this comment

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

Hi, as one can read here : https://developer.android.com/studio/build/shrink-code
The unused dependencies are shrink at compile time when creating a release.
So don't worry to much about this JS deps. It will ultimately be removed when creating any release of a mobile application.

@nashfive
Copy link
Contributor

Hi there! Sorry for the late reply..
I've tried the code in this PR and unfortunately, I found an issue right from the start by trying to parse an international number, without providing a region code. it works with the native sdks, but the web sdk just fails. The libphonenumber's JS implementation might not be ready yet.
Do you have the same results ?

@Faendis
Copy link
Author

Faendis commented Jul 5, 2021

Hello,
I don't remember if I had this results. I have made some tests and everything seemed to work fine.
I will try to reproduce your issue and probably fix it.

@Faendis
Copy link
Author

Faendis commented Jul 5, 2021

I find the problem and fix it (I was forcing regionCode to not be null when parsing international phone numbers).
Tell me if you still encountered this issue or if you find another.
If you can provide me any scenarios to test in order to correct the rest of my code, it will be nice.

I find two other issues :

  1. carrierRegionCode() : I can't get the carrier RegionCode with libphonenumber but I can try to get current browser region if you want
  2. getAllSupportedRegions() : I can't get the display names from RegionCode in dart or with libphonenumber. I can try to use some js code (using Intl.DisplayNames) or a flutter package (locale_names which is not null safe)

@dorsamet
Copy link

dorsamet commented Apr 25, 2022

Hey all, is this planned on being merged soon?

@ghost
Copy link

ghost commented Oct 13, 2022

Is there any update on this merge?

@deandreamatias
Copy link
Contributor

Take a look on this @gabriel8belts #86

@github-actions github-actions bot added the Stale label May 15, 2023
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.

6 participants