Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: match area codes or priority when determining country code #13

Merged

Conversation

ghollingworthh
Copy link

  • Suggesting this basic solution to check the area codes when determining the country code.

Why: When the phone number control value is set programatically, eg. when a record is read and the control is populated with the phone number so that it can be edited, the area codes were NOT taken into consideration when attempting to determine the country code.

For example, when I programmatically set the value of the control with a US phone number, eg. +19781234567 the country returned was American Samoa (+1 684).

  • Suggesting a check of the country 'priority'.

Why: To ensure that the highest priority country is associated with any country code that is utilized by multiple countries, eg. +39 is shared by Italy and Vatican City. I'm not sure how to can figure out which country was used when the phone number was created. The phone numbers, eg. +39 312 345 6789 do NOT contain any 'priority' info.

  • Suggesting adding the country name is displayed on hover.

Why: The country name could be useful to user unfamiliar with the selected flag and the hover tooltip is the quickest and easiest way to provide that information.

These changes would go a long way to help me adopt using the component I my project. While I have tinkered with the code, and offered some basic solutions, please don't hestitate to make different, better changes (perhaps make the tooltip configurable) yourself that might support the needs described above.

I added an input field to the test application for testing that programmatically setting the component value returns the correct country code based on country and area code.

Please let me know if you have any questions or concerns.

* Suggesting this basic solution to check the area codes when determining the country code.

Why: When the phone number control value is set programatically, eg. when a record is read and the
control is populated with the phone number so that it can be edited, the area codes were NOT taken
into consideration when attempting to determine the country code.

For example, when I programmatically set the value of the control with a US phone number,
eg. +19781234567 the country returned was American Samoa (+1 684).

* Suggesting a check of the country 'priority'.

Why: To ensure that the highest priority country is associated with any country code that is
utilized by multiple countries, eg. +39 is shared by Italy and Vatican City. I'm not sure how to
can figure out which country was used when the phone number was created. The phone numbers,
eg. +39 312 345 6789 do NOT contain any 'priority' info.

* Suggesting adding the country name is displayed on hover.

Why: The country name could be useful to user unfamiliar with the selected flag and the hover
tooltip is the quickest and easiest way to provide that information.

These changes would go a long way to help me adopt using the component I my project.
While I have tinkered with the code, and offered some basic solutions, please don't hestitate to
make different, better changes (perhaps make the tooltip configurable) yourself that might support
the needs described above.

I added an input field to the test application for testing that programmatically setting the
component value returns the correct country code based on country and area code.

Please let me know if you have any questions or concerns.
Copy link

nx-cloud bot commented Aug 14, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 1e0e003. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 6 targets

Sent with 💌 from NxCloud.

@juanjotorres90
Copy link
Owner

Thank you for your contribution and fixing this issue. I have been testing the changes and everything seems working fine.

Showing the country name on hover is a really nice addition. No need to be configurable for now.

Everything will be available in tomorrow's release 18.2.0.

Your efforts are much appreciated!

@juanjotorres90 juanjotorres90 merged commit bcdcbda into juanjotorres90:main Aug 14, 2024
4 checks passed
This was referenced Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants