cchanxzy / react-currency-input-field

React component for an input field
MIT License
680 stars 120 forks source link

onValueChange is firing when clicking outside of input field (ie. onBlur) #147

Closed wevial closed 3 years ago

wevial commented 3 years ago

onValueChange is firing when clicking outside of the input field/on an onBlur event. Not sure if this is a bug or not, but it'd make more intuitive sense to only have it fire when the onChange event is triggered.

https://codesandbox.io/s/onvaluechange-firing-onblur-n9lbw?file=/src/App.js

cchanxzy commented 3 years ago

Yes onValueChange is supposed to fire onBlur.

This was a deliberate choice, because there might be some formatting to the value (padding/trimming) that happens in the onBlur cycle. Since the method is called onValueChange, it seems sensible to trigger when the value changes. It will also also be called when stepping up/down, because the value is changing.

slipets-a commented 3 years ago

Is there any flag that shows that the update is caused by the onBlur event instead of the onChange? I think that's not the best choice to treat onBlur and onChange in one onValueChange method. In my case onBlur must be skipped but I can't do this with current behavior.

cchanxzy commented 3 years ago

@slipets-a there currently isn't a flag, but that could be added as the third param in onValurChange callback.

I think that's not the best choice to treat onBlur and onChange in one onValueChange method.

I agree, it's not ideal. But like I said above, it's supposed to be called whenever the value changes, which might happen onBlur, onChange or onKeyPress. I think adding the origin like you've suggested is a good option, and I'll definitely look into it.

slipets-a commented 3 years ago

@cchanxzy I totally understand your reasoning, but adding the param to onValueChange will make the interface more complicated. Currently, even without the flag, I can't achieve what I need with onValueChange.

And as a result, I think onValueChange should be split into onChange, onBlur and onKeyPressed. 2 reasons for that: 1) specification designed to distinguish those handlers 2) it'll be way easier to provide the same callback to onChange and onBlur if needed than to add additional logic into the callback to differ the events.

cchanxzy commented 3 years ago

@slipets-a I don't disagree with your reasoning and logic, and I appreciate your input. The change you suggested would be a breaking change though, so I'll take your points into consideration for the next major version release.

Furthermore, may I ask what your use case is where you need to distinguish between onChange, onBlur and onKeyPressed ? I'd like to understand your situation a bit more.

slipets-a commented 3 years ago

@cchanxzy Sure. I have two input fields on the Foreign Exchange page and I do the network requests on field update but also I have two steps - Details Overview and Confirm Quote page.

And when I update the input field and click the "Proceed" button I got two requests - the first one happens on blur and the second one handles click event.

Blur is the latest event in the queue because the onValueChange wrapped into a custom useDebounce hook. So I need to skip that blur event.

image

slipets-a commented 3 years ago

@cchanxzy Any update on this? Or I didn't convince you on the topic?

ericblade commented 3 years ago

any input in my app would need an onblur event, whenever you pop out of any input in the app, i need it to return focus to either that input or the main input box in the app.

chogarcia commented 3 years ago

same here

bruno-silva5 commented 1 year ago

A great workaround for this is using a useEffect, like the following component:

const CustomCurrencyInput = ({ defaultValue, exampleFunction }) => {

    const [value, setValue] = useState(defaultValue);

    useEffect(() => {
        exampleFunction(value);
    }, [value])

    return (
        <CurrencyInput
            decimalsLimit={2}
            value={value || ''}
            onValueChange={value => setValue(value)}
        />
    )
}

export default React.memo(CustomCurrencyInput)

This way solved my problem. It only calls the function when the value change.