-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat(isMobilePhone): add Palestine ar-PS locale #1767
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1767 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 102 102
Lines 2029 2029
Branches 457 457
=========================================
Hits 2029 2029
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @brendan-c and thank you for your PR! Can you please address my comment below?
src/lib/isMobilePhone.js
Outdated
@@ -14,6 +14,7 @@ const phones = { | |||
'ar-LY': /^((\+?218)|0)?(9[1-6]\d{7}|[1-8]\d{7,9})$/, | |||
'ar-MA': /^(?:(?:\+|00)212|0)[5-7]\d{8}$/, | |||
'ar-OM': /^((\+|00)968)?(9[1-9])\d{6}$/, | |||
'ar-PS': /^(\+?970|0)5[8|9](\d{7})$/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't prefixes for mobile phone numbers 9 and 6 instead of 8? The only source i can find is this wikipedia article
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could be correct. The information this was based on was from http://www.wtng.info/wtng-970-ps.html but it lists it's source as Paltel, which appears to no longer be a mobile carrier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ! Thank you for your efforts 🎉
@brendan-c - please fix the merge conflict and we should be good to go. |
Your work is now merged here #1851 |
feat(isMobilePhone) add Palestine validation (ar-PS)
Checklist