Windowscommunitytoolkit: TextBoxRegEx doesn't validate Phone Numbers in (###) ###-#### format properly

[michael-hawker]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  <!-- Please search GitHub for a similar issue or PR before submitting -->
[ ] Feature request <!-- Please file a UserVoice request and include the link below https://wpdev.uservoice.com/forums/110705-universal-windows-platform/category/193402-uwp-community-toolkit -->
[ ] Sample app request
[ ] Documentation issue or request
[ ] Question of Support request => Please do not submit support request here, instead see https://github.com/Microsoft/UWPCommunityToolkit/blob/master/contributing.md#question

Current behavior

Entering a phone number in the style (###) ###-#### is marked as invalid both of the sample apps phone number examples.

Expected behavior

Should be recognized as a valid phone number.

Minimal reproduction of the problem with instructions

1. Open Sample App
2. Go to Extensions -> TextBoxRegEx sample
3. Enter phone number in either of first two textboxes in (###) ###-#### format

Note: it's marked as invalid.

Environment

Nuget Package(s): 

Package Version(s): 

Windows 10 Build Number:
- [ ] Anniversary Update (14393) 
- [ ] Creators Update (15063)
- [x] Fall Creators Update (16299)
- [ ] Insider Build (xxxxx)

App min and target version:
- [ ] Anniversary Update (14393) 
- [ ] Creators Update (15063)
- [x] Fall Creators Update (16299)
- [ ] Insider Build (xxxxx)

Device form factor:
- [x] Desktop
- [ ] Mobile
- [ ] Xbox
- [ ] Surface Hub
- [ ] IoT

[avknaidu]

Here is a regex that i built.

(\([0-9]{3}\) ?)[0-9]{3}-[0-9]{4}|[0-9]{3}-?[0-9]{3}-?[0-9]{4}|([00|+][0-9]{1,2}) ?[0-9]{3}-?[0-9]{3}-?[0-9]{4}

Below is all formats that it would match.

here is a link to regexr.com

Let me know if this fits the requirement and I will do a PR.

[michael-hawker]

Thanks @avknaidu, I think we should address this in 3.0, as I think it'll be hard to validate all the same scenarios from the current regex to the new regex and people's expectations.

We probably also want to have different modes for International vs US/Canada. This also seemed pretty comprehensive: http://phoneregex.com

Unfortunately it also looks like the C# port of libphonenumber is out of date.

[avknaidu]

libphonenumber is updated just 8 hrs ago on nuget. are you saying it still does not handle all scenario's?

[michael-hawker]

Nice @avknaidu, I think I must have found a dead fork then by mistake.

@nmetulev thoughts on pulling in this dependency for 3.0?

[windowstoolkitbot]

This issue seems inactive. Do you need help to complete this issue?

[nmetulev]

If we can avoid adding another dependency, I think we should try.

[skendrot]

I think a generic solution for phone numbers is fine and users can add their own regex for their needs

[windowstoolkitbot]

This issue seems inactive. Do you need help to complete this issue?

[windowstoolkitbot]

This issue seems inactive. Do you need help to complete this issue?

[nmetulev]

Agree with @skendrot, let's not add another dependency and instead try to solve this through a regex

[Bmartin2013]

By default, this control looks for a custom regular expression in order to check their inputs. In other words, if you don't force this control to validate phone numbers, you will be able to set your own format validation through a custom regular expression by declaring the Regex property in XAML file.

For more information, you can check the control documentation: https://docs.microsoft.com/en-us/windows/uwpcommunitytoolkit/extensions/textboxregex

[Bmartin2013]

Since the refactor of the phone number method will be a breaking change and it's possible to work around this problem by setting a custom RegEx to check the input values @nmetulev can we close this issue?

[nmetulev]

I don't think it's a breaking change as right now it doesn't work as expected, phone numbers should be recognized. We should update the regex here which should resolve the issue when ValidationType="PhoneNumber" is used.

[skendrot]

@avknaidu Do you want to provide a PR with your regex?

[avknaidu]

@skendrot will do

[nmetulev]

PR merged