Skip to content

Conversation

thiagorb
Copy link

The current implementation is not firing the onChange callback passed in
the options.

This happens because the oldValue variable is updated only on blur. With
this changes, two variables are used. One controls the value that the
element had when it got focus, and the other one is changed immediately
after every change.

Fiddle that reproduces the problem: https://jsfiddle.net/kp5hq1e8/7/
Steps to reproduce:

  • Click on the input to give focus
  • Type something (for example "1")
  • Click somewhere else on the page so the input loses focus
  • Click again on the input
  • Type another value ("12")
  • Type the previously typed value ("1")

It was expected that the onChange callback would be fired every time, but when entering the value that the input had when it lost focus nothing happens.

Output of the fiddle with the steps above:

  • focus
  • 1
  • blur
  • focus
  • 12
  • 12
  • 12
  • blur

Expected output:

  • focus
  • 1
  • blur
  • focus
  • 12
  • 1
  • 12
  • 1
  • 12
  • blur

The current implementation is not firing the onChange callback passed in
the options.

This happens because the oldValue variable is updated only on blur. With
this changes, two variables are used. One controls the value that the
element had when it got focus, and the other one is changed immediately
after every change.
@igorescobar
Copy link
Owner

Thanks for this @thiagorb

I will give it a shot ASAP.

@ankurk91
Copy link
Contributor

ankurk91 commented Aug 6, 2018

I really wants to be merged.
Same issues has been raised previously.

#358
#486

Another solution is to introduce a new callback onInput that will be called every time without comparing oldValue.

@igorescobar
Copy link
Owner

@ankurk91 have you tested this PR?

@ankurk91
Copy link
Contributor

ankurk91 commented Aug 8, 2018

@igorescobar
I have tested it, and it works as described.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants