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

Slightly Improved phone validator. #115

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mmsaffari
Copy link

@mmsaffari mmsaffari commented Jun 1, 2024

The RegExp used in the original code was too loose for a valid Phone number and on the other hand, phone numbers have somewhat different patterns around the world. According to Mozilla one should be able to use the pattern attribute on an <input type="tel" /> element to indicate the RegExp to be used to validate the phone number.

This PR takes the pattern attribute into account.

@mmsaffari mmsaffari marked this pull request as ready for review June 1, 2024 02:20
@mmsaffari
Copy link
Author

I should have checked to see if the element has a pattern attribute.
But why is it trying to get the pattern attribute from a select?

Property 'pattern' does not exist on type 'HTMLSelectElement'.

@@ -333,7 +333,7 @@ export class MvcValidationProviders {
return false;
}

let r = /^\+?[0-9\-\s]+$/;
let r = element.pattern ? new RegExp(element.pattern) : /^\+?[0-9\-\s]+$/;
Copy link
Owner

@haacked haacked Jun 2, 2024

Choose a reason for hiding this comment

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

Maybe it should be:

Suggested change
let r = element.pattern ? new RegExp(element.pattern) : /^\+?[0-9\-\s]+$/;
let pattern = element.getAttribute('pattern');
let r = pattern ? new RegExp(pattern) : /^\+?[0-9\-\s]+$/;

Copy link
Collaborator

Choose a reason for hiding this comment

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

But why is it trying to get the pattern attribute from a select?

Property 'pattern' does not exist on type 'HTMLSelectElement'.

Because element: ValidatableElement and only HTMLInputElement has pattern:

export type ValidatableElement = HTMLInputElement | HTMLSelectElement | HTMLTextAreaElement;

The getAttribute solution works, but you can make TS happy with either:

Suggested change
let r = element.pattern ? new RegExp(element.pattern) : /^\+?[0-9\-\s]+$/;
let r = 'pattern' in element && element.pattern ? new RegExp(element.pattern) : /^\+?[0-9\-\s]+$/;

or

Suggested change
let r = element.pattern ? new RegExp(element.pattern) : /^\+?[0-9\-\s]+$/;
let r = element instanceof HTMLInputElement && element.pattern ? new RegExp(element.pattern) : /^\+?[0-9\-\s]+$/;

While we're in the area, it could be handy to assign/use the default pattern as something like static phonePattern to allow overriding everywhere by assigning MvcValidationProviders.phonePattern.

@Grizzlly
Copy link

@mmsaffari Hello! Would you be able to update this?

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.

4 participants