Add ability to mask card number #231
Conversation
|
Thanks for your PR and sorry for the long delay — just catching up! I'm going to leave a few comments, but I'm excited to merge this feature in! |
| cardType = payment.fns.cardType(val) | ||
| numbers = val.split(' ') | ||
|
|
||
| if (cardType == 'amex' && (numbers.length == 3)) || numbers.length >= 4 |
There was a problem hiding this comment.
Can you explain why we have to add this exception for amex? I'm a little bit wary of adding card type specific code here and wonder whether we can come up with a general solution.
There was a problem hiding this comment.
Hi Jesse
The amex check isn't really necessary as I dont know much about card numbers and how many groups numbers can exists added amex check to make sure that my code will not break with other card type.
I made a quick check removing the cardType check and works without problems, but again I dont know all the details of card numbers.
|
Maybe the same can be done for the CVV? |
|
Haven't this PR been merged yet? |
| @@ -183,7 +188,7 @@ $('form').card({ | |||
| ## Using with other javascript libraries | |||
|
|
|||
| Card has wrappers that make it easy to use with other javascript libraries: | |||
|
|
|||
There was a problem hiding this comment.
This should be done on a separated commit.
|
@hernan Are you intending to remove the AMEX specific check since you said it works fine without it in there? |
|
@BallisticPain adjusted the fix, please review. |
|
@hernan, please fix the latest conflict. I'll review right after. Sorry I missed your last comment to review. |
|
Hi @BallisticPain is older than current master and don't see conflicts, I'm not sure how to proceed in this case to provide a correct commit, if you could guide me would be grat |
|
@hernan Hi! Sorry if I am wrong, but you can remove all your changes on README.md file, backing it to original. Wait, are you working on branch master? Why? Anyway, if you back README.md to original (you can copy the README.md content from the server and paste on your file), update your branch with master branch from the server, later make your changes on README.md again and commit again, your branch will be synchronized with server and the approval of the PR will be easier. |
|
@hernan When I have merge conflicts, I usually attempt to rebase the upstream branch onto mine. Below are the steps I would usually use to perform a rebase. Be sure you're on the master branch.
Edit: Moved the upstream add to the first paragraph. Thanks for your continued responses and diligence! |
|
@BallisticPain your guide worked without a glitch! thank you very much, to patch should be fine now, please review. |
Related to #179 I implemented masking the card number on user input, added as a filter for numberInput handler.