CreditCardNumber field should report cardType #189
Conversation
| @@ -71,7 +71,7 @@ export default class CreditCardNumber extends Component { | |||
|
|
|||
There was a problem hiding this comment.
Unrelated toI'm thinking this parsing logic might be better outside of the React component - can we make separate JS file in react-gears for this? Would be easier to test and update too.
There was a problem hiding this comment.
The parsing logic is already outside the component - it's handled by an external lib. I'm also reticent to refactor just for refactoring sake - is there a use-case that the current implementation doesn't support?
src/components/CreditCardNumber.js
Outdated
| @@ -71,7 +71,7 @@ export default class CreditCardNumber extends Component { | |||
|
|
|||
| // Only accept the change if we recognize the card type, and it is/may be valid | |||
| if (!this.props.restrictInput || cardType && (isValid || isPotentiallyValid)) { | |||
| this.props.onChange(value, isValid); | |||
| this.props.onChange(value, isValid, card.type); | |||
There was a problem hiding this comment.
If cardType and card.type are same can we combine or use one or the other here vs both?
There was a problem hiding this comment.
They are not the same. cardType is the icon name and card.type is the name returned by the card parsing library. I'll rename cardType to be more clear.
|
I think that this component is going the wrong direction. We should think in layers and very small ones. Component A - the view: a component that allows users to type in a credit card and that shows an icon. This component will do it's own parsing of the card number to see which icon to show. Changes will be notified via an onChange handler that passes just the current value. Component B - the validator: It renders component A and listens to onChange. If something is invalid, it will add Component A is what can be in React-Gears. Component B is better fitted in people's own code base. For credit card validation there is a simple dependency that each can install and also each consumer of the component can decide which cards to accept. |
|
Agreed with @balloob 's comment above, let's work on this pattern for our form fields in next releases |
|
That seems like a nice abstraction for the future, sure. There are 2 libs involved in parsing / validating the CC number. Essentially, in the description above -
I'd be happy to contribute to another PR to make this split. |
No description provided.