feat: add formatPhone #155
Conversation
Codecov Report
@@ Coverage Diff @@
## main #155 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 51 51
Lines 938 955 +17
Branches 113 122 +9
=========================================
+ Hits 938 955 +17
Continue to review full report at Codecov.
|
| @@ -107,3 +108,25 @@ describe('isValid', () => { | |||
| }); | |||
| }); | |||
| }); | |||
|
|
|||
| describe('format', () => { | |||
| test('should format phone', () => { | |||
There was a problem hiding this comment.
I think it would be interesting to add tests using a string with non numeric characters and strings with length larger than a phone number length.
i.e.:
expect(format('11988#887AB777')).toBe('(11) 9888-877777'); expect(format('11988887777123123123')).toBe('(11) 9888-877777')
There was a problem hiding this comment.
after the change suggested by @hyanmandian this test case:
expect(format('11988887777123123123')).toBe('(11) 9888-877777')
will be impossible, instead it'll return:
+11 (98) 88877-77123123123
is this ok?
|
Hello nice MR @linconkusunoki and thanks @pedrovsp for the review! Another thing that I think could be improved is the removal of the config param... I know that I've suggested this but looking at the code and the test cases, maybe its better to just format the prefix when receiving this prefix. Something like: What do you think about it? |
|
hey guys thanks for the feedback and sorry for the late, I'll work on these changes |
|
I like the idea of receiving the phone number and format the country code if it is present (eg. length > 11). |
|
Hello guys, do you need some help? |
|
hey @hyanmandian for me it's ok if anyone wants to get this task #186, maybe close this PR and open another one? |
related: #146