Option to add "Cardholder Name" field to manual entry form #39
Conversation
|
Field Empty Example:
Field With Text Example:
|
|
@braebot I got the localized strings using google translate like you suggested, let me know what the next step is whenever you have a chance |
| @@ -48,6 +49,7 @@ | |||
| * PayPal REST Apis only handle max 20 chars postal code, so we'll do the same here. | |||
| */ | |||
| private static final int MAX_POSTAL_CODE_LENGTH = 20; | |||
| private static final int MAX_NAME_ON_CARD_LENGTH = 50; | |||
There was a problem hiding this comment.
Why is this 50? According to Braintree docs, 175 is the max, while Visa/Mastercard are issuing new cards with 22 char limit. Would be good to have a reason for the length here.
There was a problem hiding this comment.
I couldn't find a standard max value but I can update it to 175 if that's the braintree max
There was a problem hiding this comment.
Yes please, let's do that, unless another limiting factor requires it to go lower.
|
Overall code looks good. However, I don't like the "Name on Card" string or related extra/variable names. According to the git history, it was originally Cardholder name, but was modified to Name on Card, without a reason given. What were your thoughts there? |
|
So we actually had an internal discussion about "Name on Card" vs "Cardholder Name" and our product team landed on "Name on Card", however if you think "Cardholder Name" is more inline with Braintree terminology then we can definitely switch it back to that (I personally did like "Cardholder Name" more). |
|
@braebot I have a branch with the rename back to "Cardholder Name". do you think that's the way to go? |
|
Yes, but I think at least one of the other @card-io/developers should chime in, since whatever name we pick will go in iOS too. |
dsn5ft
changed the title
|
@braebot Now that there is an iOS equivalent pull request for this feature (card-io/card.io-iOS-source#45), is there anyway we can get a final confirmation on the Cardholder Name string so we can update our pull requests if needed and get them merged in once they are reviewed and certified? |
|
@dsn5ft, thanks for iOS PR! I'll bug the other contributors to chime in so we can send this off to translators. |
|
sounds good, thank you! |
|
|
|
Sounds good, I can merge in my branch which renames everything to Cardholder Name |
updated translations
|
Thanks @dsn5ft! I will put it through a few tests (including one in TestDroid), and merge/release when done. |
|
Merged! Feel free to add yourselves as contributors in the readme in another PR, since I forgot to comment about that before merging. 🏆 |
|
will do. thanks! |
All tests passed on a Moto X 2nd Gen device running Android 5.1
Localized strings from Google translate, waiting on official list from PayPal translation team