nkbt / react-debounce-input

React component that renders Input with debounced onChange
MIT License
451 stars 61 forks source link

onChange handler called on Enter and once again after debounceTimeout #64

Closed isaacalves closed 7 years ago

isaacalves commented 7 years ago

I'm using something like this:

handleInputChange(e) { this.props.onInputChange(e.target.value); }

and on my render method:

<DebounceInput type='text' onChange={ this.handleInputChange.bind(this) } debounceTimeout={ 1000 } />

I see that handleInputChange is also called when I press Enter from the input field (although I could handle that myself in an onSubmit handler on the form). I assume that's intended, but why then does handleInputChange fires again after the timeout?

Scenario:

  1. focus on text field
  2. type something and press Enter
  3. handleInputChange() is called immediately
  4. handleInputChange() is called again after the timeout

I see that I can use forceNotifyByEnter so it's not called on Enter key press. That solves the problem, but actually it would be nice to have it being called on Enter key press as well, as long as it clears the timeout.

emhagman commented 7 years ago

+1

nkbt commented 7 years ago

Makes sense. I would be happy to accept a PR on this

hozefaj commented 7 years ago

Let me take a crack at fixing this.

@nkbt One thing though, why would you need to handle enter once timeout has passed. Rather it should be the Enter can override the timeout?

nkbt commented 7 years ago

Thanks @hozefaj! Timeout should be cancelled on Enter, that feels like the most logical way.

Data-Meister commented 7 years ago

This is an issue with both notify on enter, and also notify on blur.

The problem is this line

this.notify is a method, but the code is attempting to access a property this.notify.cancel as if this.notify were an object

nkbt commented 7 years ago

That is ok, since lodash.debounce has .cancel method available ondebounced functions

Data-Meister commented 7 years ago

In that case, the problem is this.notify is not itself a debounced function. The debounced function is called from within this.notify

     const debouncedChangeFunc = debounce(event => {
        this.isDebouncing = false;
        this.doNotify(event);
      }, debounceTimeout);

      this.notify = event => {
        this.isDebouncing = true;
        debouncedChangeFunc(event);
      };
    }
Data-Meister commented 7 years ago

I've now fixed this bug, with code submitted as part of this PR

https://github.com/nkbt/react-debounce-input/pull/73

nkbt commented 7 years ago

Should be fixed in react-debounce-input@3.0.1