react-maskedinput icon indicating copy to clipboard operation
react-maskedinput copied to clipboard

modify componentWillReceiveProps to check with mask to see if mask va…

Open mdgbayly opened this issue 8 years ago • 5 comments

…lue would be updated by new props rather than just checking to see if props have changes. Fixes issue where if a change is rejected by the client onChange handler, the component does not re-render.

See: https://github.com/insin/inputmask-core/pull/16

mask: '111%'

Type: 9 MaskValue: 9%

Type: 9 MaskValue: 99%

Type: 9 MaskValue: 999%

But the value of 999 is rejected by onChange handler so component re-renders with props of 99 again. But the mask does not re-render as componentWillReceiveNewProps checks to see if props have changed and in this case have not.

mdgbayly avatar Mar 11 '16 19:03 mdgbayly

This looks like a solid pull request to me. What's holding this back?

alpjor avatar Jun 14 '16 22:06 alpjor

To follow up on this:

But the value of 999 is rejected by onChange handler so component re-renders with props of 99 again. But the mask does not re-render as componentWillReceiveNewProps checks to see if props have changed and in this case have not.

Is the end result that the Mask incorrectly renders 999% in this scenario?

More directly, does this implement a bug fix or an optimization?

iamdustan avatar Jun 16 '16 15:06 iamdustan

@iamdustan Yes, the end result is that the mask incorrectly renders 999%. So from my perspective its a bug fix.

Possibly this is related to how we are using the component. We are effectively using it as a controlled component.

Interestingly, I just upgraded our app to React 15, and I'm now getting warnings that MaskedInput needs to decide whether it is a controlled component or an uncontrolled component... Not sure if that is because our fork is behind and this has been remedied in the master origin, or whether this is a general problem with the implementation.

e.g.

MaskedInput contains an input of type undefined with both value and defaultValue props. Input elements must be either controlled or uncontrolled (specify either the value prop, or the defaultValue prop, but not both). Decide between using a controlled or uncontrolled input element and remove one of these props.

warning.js:44 Warning: MaskedInput is changing a controlled input of type undefined to be uncontrolled. Input elements should not switch from controlled to uncontrolled (or vice versa). Decide between using a controlled or uncontrolled input element for the lifetime of the component.

mdgbayly avatar Jun 16 '16 16:06 mdgbayly

Actually, scratch my last comment about the warnings - I think that is a result of how we've integrated it. We're using it with React Redux Form, and I think that is where the duplicate defaultValue/value props are coming from.

mdgbayly avatar Jun 16 '16 16:06 mdgbayly

Would love to see this merged sometime

shkaper avatar Jul 21 '17 00:07 shkaper