taiga-ui icon indicating copy to clipboard operation
taiga-ui copied to clipboard

🚀 - InputPhoneInternational component with phone number & type validation

Open rvalitov opened this issue 3 years ago • 9 comments

Which @taiga-ui/* package(s) are relevant/releated to the feature request?

core, i18n

Description

Problem 1. Validation and trust of data used in Taiga

It's not specified what source is used for internalization and phone masks in Taiga and if it can be trusted. According to my tests I faced an issue with incorrect phone mask #863. I tried to make a little PR #868 to fix that. But later I found much more inconsistencies with other countries, too. Besides, there are issues with invalid ISO codes, #915, #916.

Problem 2. Phone mask and validation

It's not the best approach to use the mask constants as the are now. The reason is that in some countries the phone number length is different for mobiles and landline phones. Also, some countries have strict rules about the phone numbers, for example, mobile phones can start only with digit 5, and landlines with digit 4. All other prefixes are invalid. Therefore, even if the input is correct in term of length, it's still invalid phone number.

How to solve that?

I think Taiga should use a trusted source of data for phone number masks & validation, and internalization, i.e. include and use some other package that keeps updated when ISO codes change, etc.

My solution

I recommend to use libphonenumber-js:

  • https://www.npmjs.com/package/libphonenumber-js
  • https://gitlab.com/catamphetamine/libphonenumber-js

Because:

  • it covers all the officially assigned country codes and phone numbers
  • regularly updated
  • based on official Google lib, which adds more trust that the data is valid
  • perfectly documented
  • provides phone masks
  • provides sample phone numbers for each country and type (mobile, landline)
  • detects phone number type (mobile, landline, toll free, etc.)
  • provides a limited phone number validation (by length) and full validation (that this number is really confirms the phone number rules of the specified country)

Demo

I modified the InputPhoneInternational component to work with libphonenumber-js. The modification includes:

  • removed deprecated country-iso-codes.ts, instead used import {TuiCountryIsoCode} from '@taiga-ui/i18n'
  • removed countries.ts file with mask constants, used masks generated by libphonenumber-js
  • displays additional info as the user types in. The following is displayed in tui-form__field-note below the input field:
    • check if phone number is valid
    • check if phone number length is valid and how to fix that
    • show phone type.
    • show sample phone number

GIF demo:

phoneinput

I think there's a plenty room for discussion here. I'm eager to make a PR.

rvalitov avatar Oct 21 '21 05:10 rvalitov

The data was given to us by internal analysts long time ago, so I guess it changed since then. I believe it should just be tokenized, it's a mystery to me why it wasn't done in the first place :) Will you be willing to submit a PR? The best resolution would be to sync current constant value with the lib you suggested and use it as a default value for the token. And an info on how to change that to other library should be added to the demo page.

Another point to consider is deprecating built-in values in favor of 3rd party solution, not sure though if it's a good idea not to bundle anything. We definitely not going to add a dependency to main packages. Perhaps we can move these components to a separate addon and add dependency + validators you displayed. So that's something to consider as well.

waterplea avatar Oct 22 '21 13:10 waterplea

I can make a PR. Perhaps, the approach with separate addon & dependency there is the best. In this case the InputPhoneInternational moves completely to the addon? Or there will be 2 versions of it: as addon, and simple bundled with the core?

rvalitov avatar Oct 25 '21 10:10 rvalitov

I don't really like to put dependencies and make more packages under the hood of this repo. Would you be interested in making your own separate package for Taiga? Ideally I see it like this:

  1. ​A PR to fix current list and make a token for the list.
  2. A separate package with libphonenumber-js that has a new value for that token and a validator and whatever else you see helpful
  3. We can then put a link to that package on the docs page for InputPhoneInternational
  4. Perhaps we can remove default value for the token in 3.0 citing your package as recommended solution and this way we will make bundle a little bit smaller for people who don't need all countries

waterplea avatar Nov 01 '21 14:11 waterplea

Not sure if it's related, sometimes in console of the Taiga manual site I see errors:

Failed to load resource: the server responded with a status of 404 ()
main-es2015.35912c4ae7216946e268.js:1 ERROR Error: Uncaught (in promise): ChunkLoadError: Loading chunk 43 failed.
(error: https://taiga-ui.dev/43-es2015.53efa5f66ee8aeabfce1.js)
ChunkLoadError: Loading chunk 43 failed.
(error: https://taiga-ui.dev/43-es2015.53efa5f66ee8aeabfce1.js)
    at Function.r.e (runtime-es2015.d4890cf23e46165775ab.js:1)
    at loadChildren (main-es2015.35912c4ae7216946e268.js:2)
    at Xt.loadModuleFactory (main-es2015.35912c4ae7216946e268.js:2)
    at Xt.load (main-es2015.35912c4ae7216946e268.js:2)
    at u.project (main-es2015.35912c4ae7216946e268.js:2)
    at u._tryNext (main-es2015.35912c4ae7216946e268.js:1)
    at u._next (main-es2015.35912c4ae7216946e268.js:1)
    at u.next (main-es2015.35912c4ae7216946e268.js:1)
    at e._subscribe (main-es2015.35912c4ae7216946e268.js:1)
    at e._trySubscribe (main-es2015.35912c4ae7216946e268.js:1)
    at w (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at w (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at polyfills-es2015.74b69fa587adac4cf724.js:1
    at u.invokeTask (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at Object.onInvokeTask (main-es2015.35912c4ae7216946e268.js:1)
    at u.invokeTask (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at a.runTask (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at m (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at l.invokeTask [as invoke] (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at h (polyfills-es2015.74b69fa587adac4cf724.js:1)
Xn @ main-es2015.35912c4ae7216946e268.js:1
common-es2015.c78a287d4c889cfd8725.js:1 Failed to load resource: the server responded with a status of 404 ()
0-es2015.f13e589447611031432e.js:1 Failed to load resource: the server responded with a status of 404 ()
0-es2015.f13e589447611031432e.js:1 Failed to load resource: the server responded with a status of 404 ()
main-es2015.35912c4ae7216946e268.js:1 ERROR Error: Uncaught (in promise): ChunkLoadError: Loading chunk 0 failed.
(error: https://taiga-ui.dev/0-es2015.f13e589447611031432e.js)
ChunkLoadError: Loading chunk 0 failed.
(error: https://taiga-ui.dev/0-es2015.f13e589447611031432e.js)
    at Function.r.e (runtime-es2015.d4890cf23e46165775ab.js:1)
    at loadChildren (main-es2015.35912c4ae7216946e268.js:2)
    at Xt.loadModuleFactory (main-es2015.35912c4ae7216946e268.js:2)
    at Xt.load (main-es2015.35912c4ae7216946e268.js:2)
    at u.project (main-es2015.35912c4ae7216946e268.js:2)
    at u._tryNext (main-es2015.35912c4ae7216946e268.js:1)
    at u._next (main-es2015.35912c4ae7216946e268.js:1)
    at u.next (main-es2015.35912c4ae7216946e268.js:1)
    at e._subscribe (main-es2015.35912c4ae7216946e268.js:1)
    at e._trySubscribe (main-es2015.35912c4ae7216946e268.js:1)
    at w (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at w (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at polyfills-es2015.74b69fa587adac4cf724.js:1
    at u.invokeTask (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at Object.onInvokeTask (main-es2015.35912c4ae7216946e268.js:1)
    at u.invokeTask (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at a.runTask (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at m (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at l.invokeTask [as invoke] (polyfills-es2015.74b69fa587adac4cf724.js:1)
    at h (polyfills-es2015.74b69fa587adac4cf724.js:1)
Xn @ main-es2015.35912c4ae7216946e268.js:1
common-es2015.c78a287d4c889cfd8725.js:1 Failed to load resource: the server responded with a status of 404 ()
45-es2015.083c5b2a9c7777b1e7a1.js:1 Failed to load resource: the server responded with a status of 404 ()
styles.cd96327f16f5dc487bb7.css:1 Failed to load resource: the server responded with a status of 404 ()
icons:1 Failed to load resource: the server responded with a status of 404 ()

rvalitov avatar Nov 02 '21 09:11 rvalitov

Looks like a lazy loaded chunk fail because new content was deployed while you were on one page and the cache busting id has changed.

waterplea avatar Nov 02 '21 09:11 waterplea

@rvalitov what do you think about my idea of a separate package, providing countries from libphonenumber-js? Are you interested in making one?

waterplea avatar Dec 10 '21 10:12 waterplea

@waterplea Yes, I'm interested for sure. Besides, I already have a solution I use that I just need to rewrite to make it as a package. Perhaps, you could give me some tips how to make a package, because I never done that before. However, currently I'm very busy finalizing my projects at work. If it's not urgent, may I return to this issue in January?

rvalitov avatar Dec 20 '21 11:12 rvalitov

Sure. We could work on this together, we have an angular starter that we can use, but we wanted to update it for a while now. I think we can try doing that in January and test run it on that package.

waterplea avatar Dec 20 '21 12:12 waterplea

@rvalitov are you still interested in this one? We've updated our starter: https://github.com/Tinkoff/angular-open-source-starter

waterplea avatar May 16 '22 10:05 waterplea