Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
isaacalves opened this issue Feb 24, 2017 · 9 comments
Closed
Labels

Comments

@isaacalves
Copy link

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
Copy link

+1

@nkbt
Copy link
Owner

nkbt commented Apr 13, 2017

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

@nkbt nkbt added the bug label Apr 13, 2017
@hozefaj
Copy link

hozefaj commented May 12, 2017

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
Copy link
Owner

nkbt commented May 14, 2017

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

@Data-Meister
Copy link
Contributor

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
Copy link
Owner

nkbt commented Jun 29, 2017

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

@Data-Meister
Copy link
Contributor

Data-Meister commented Jun 30, 2017

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 added a commit to Data-Meister/react-debounce-input that referenced this issue Jul 20, 2017
@Data-Meister
Copy link
Contributor

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

#73

@nkbt
Copy link
Owner

nkbt commented Aug 3, 2017

Should be fixed in [email protected]

@nkbt nkbt closed this as completed Aug 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants