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 isMobilePhone regex to allow for ###-###-#### format #838

Closed
wants to merge 4 commits into from

Conversation

tytoons
Copy link
Contributor

@tytoons tytoons commented Jun 1, 2018

Fixes #826

@chriso
Copy link
Collaborator

chriso commented Jun 1, 2018

Thanks for the PR. Do you mind rebasing to resolve conflicts, and then adding a test case?

@profnandaa
Copy link
Member

profnandaa commented Jun 15, 2018

Is this format only for US numbers? Could it be, for a more generalized case, we just sanitize the phone number string by stripping out the -, before taking it to the regex, e.g.

const _sanitizePhoneNumber = (number) => number.split('').filter(c => c !== '-').join('');

@profnandaa
Copy link
Member

Update, just seen @chriso comment here - #826 (comment), I think using phone.replace(/-/g, '') as he suggests is better; other than confining this regex to only US numbers...

# Conflicts:
#	lib/isMobilePhone.js
#	src/lib/isMobilePhone.js
#	validator.js
#	validator.min.js
* 'master' of https://github.com/chriso/validator.js:
  Additional stuff for validatorjs#858
  Fixed semicolon issue
  Fixed stuff using eslint
  Created includes function for array to cater older browsers
  Update the changelog and bundle
  Iraq locale added
  10.4.0
  Update the changelog and min version
  Squased commits, made changes to isIpRange to avoid array destructuring.
  Update the changelog
  chore: add tests and update readme
  Squashing the many disparit commits into a single commit.
  Bump
  Update the changelog and min version
  10.3.0
  Support German numbers without a separator after country code
  Code updated with test cases
  Adde  Kuwait Number  Regex for Mobile Number Validation
  Accepting array of locales for mobile phone validation

# Conflicts:
#	validator.min.js
@tytoons tytoons changed the title Fix isMobilePhone regex to allow for ###-###-#### format [WIP] Fix isMobilePhone regex to allow for ###-###-#### format Jul 6, 2018
@tytoons tytoons changed the title [WIP] Fix isMobilePhone regex to allow for ###-###-#### format Fix isMobilePhone regex to allow for ###-###-#### format Jul 6, 2018
@profnandaa
Copy link
Member

Bubbling this up - #838 (comment)

@brybrophy
Copy link
Contributor

brybrophy commented Jul 31, 2018

I think the proposed regex could be even more comprehensive for US numbers. For instance,m the proposed solution does not validate +1(234)3243234, which is a valid US number.

I added a new PR with a different regex and more test cases. #872

@chriso
Copy link
Collaborator

chriso commented Jul 31, 2018

Fixed by #872

@chriso chriso closed this Jul 31, 2018
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.

5 participants