SX-2129 Edit phone number Circuit UI component

[roma-claudio]

Addresses SX-2129

Purpose

Update edit phone number component

Approach and changes

1. Improved test coverage for PhoneNumberInput.tsx and PhoneNumberInput.service.tsx
2. Granular control of readonly and invalid attributes for each field (country code and subscriber number)
3. Updated country code field pattern to limit the number of max allowed digits
4. Added placeholder prop to the subscriber number
5. Updated docs

Definition of done

• Development completed
• Reviewers assigned
• Unit and integration tests
• Meets minimum browser support
• Meets accessibility requirements

[vercel]

Deployment failed with the following error:

You don't have permission to create a Preview Deployment for this project.

View Documentation: https://vercel.com/docs/concepts/teams/roles-and-permissions

[changeset-bot]

⚠️ No Changeset found

Latest commit: 1428caf

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[codecov]

Codecov Report

Attention: Patch coverage is 79.64602% with 23 lines in your changes missing coverage. Please review.

Project coverage is 87.85%. Comparing base (843c6e0) to head (1428caf).
Report is 192 commits behind head on feature/phone-number-input.

Additional details and impacted files

@@                      Coverage Diff                       @@
##           feature/phone-number-input    #2512      +/-   ##
==============================================================
- Coverage                       97.15%   87.85%   -9.31%     
==============================================================
  Files                             251      200      -51     
  Lines                           19894    21684    +1790     
  Branches                         1226     1313      +87     
==============================================================
- Hits                            19329    19050     -279     
- Misses                            551     2582    +2031     
- Partials                           14       52      +38     

Files	Coverage Δ
...onents/PhoneNumberInput/PhoneNumberInputService.ts	89.28% <40.00%> (ø)
...i/components/PhoneNumberInput/PhoneNumberInput.tsx	95.45% <83.49%> (ø)

... and 193 files with indirect coverage changes

[connor-baer]

Thank you for finishing the component and especially for writing such detailed docs. Great job! 🌟

I don't see any logic to handle pasting of a (full) phone number. Have you tested how the component could/would handle this?

[roma-claudio]

Thank you for finishing the component and especially for writing such detailed docs. Great job! 🌟

I don't see any logic to handle pasting of a (full) phone number. Have you tested how the component could/would handle this?

At the moment we don't handle this scenario (meaning the entire phone number will be pasted in the subscriber number field), the question is: how do we want to handle it? Stripping it and setting the country code and subscriber number values looks error prone in this case because those can have different lengths and it's difficult to determine them 🤔

[connor-baer]

At the moment we don't handle this scenario (meaning the entire phone number will be pasted in the subscriber number field), the question is: how do we want to handle it? Stripping it and setting the country code and subscriber number values looks error prone in this case because those can have different lengths and it's difficult to determine them 🤔

I looked into this when I initially researched this component. It's possible to reliably extract the country code from a complete phone number, assuming that all country codes are known. Given that the component usually only has access to a subset of country codes (from the options prop), we will only be able to match supported country codes 🤔 I think that's fine though since I would consider unsupported country codes an edge case.

I'm happy to tackle this in a follow-up PR. I don't think it's necessary for the MVP.

[roma-claudio]

At the moment we don't handle this scenario (meaning the entire phone number will be pasted in the subscriber number field), the question is: how do we want to handle it? Stripping it and setting the country code and subscriber number values looks error prone in this case because those can have different lengths and it's difficult to determine them 🤔

I looked into this when I initially researched this component. It's possible to reliably extract the country code from a complete phone number, assuming that all country codes are known. Given that the component usually only has access to a subset of country codes (from the options prop), we will only be able to match supported country codes 🤔 I think that's fine though since I would consider unsupported country codes an edge case.

I'm happy to tackle this in a follow-up PR. I don't think it's necessary for the MVP.

Ah ok I now see your point, indeed the country code can be extracted from a full number if:

1. The list of country codes is known
2. There are no ambiguous country codes, e.g. +20 and +201, but I've checked the list of country codes and this is never the case: https://en.wikipedia.org/wiki/List_of_country_calling_codes )

Unfortunately I've no capacity to address this right now, so if you agree let's have it in a follow-up PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
oss-circuit-ui	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 6, 2024 11:16am

[connor-baer]

Thank you for addressing all the feedback. This looks great!

Let's merge it back into #2500, then I'll implement the logic for pasting a phone number so we can release the component 🎉

[roma-claudio]

Thank you for addressing all the feedback. This looks great!

Let's merge it back into #2500, then I'll implement the logic for pasting a phone number so we can release the component 🎉

Great! I'll merge it right away then, do you have a rough idea on when we can release the component?

[connor-baer]

do you have a rough idea on when we can release the component?

I'm aiming for early next week 🤞🏻