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

Fix suggestions being requested with every keystroke #2708

Merged
merged 1 commit into from
Jul 12, 2018

Conversation

javierv
Copy link
Contributor

@javierv javierv commented Jun 28, 2018

Objectives

  • Make suggestions work as intended, sending a request only when users stop typing.

Context

Here's what happens when filling in the budget investment title with "search":

Six XHR requests

As shown in the image, there are 6 XHR requests, one per keystroke. The test affecting the investments spec randomly sends between 3 and 6 requests (maybe even between 0 and 6).

The relevant code is in suggest.js.coffee:

$this.on 'keyup', ->
  window.clearTimeout(callback)
  window.setTimeout(callback, 1000)

$this.on 'change', callback

The callback is being used as the parameter for the clearTimeout() method; however, it looks like the ID value returned by setTimeout() should be used as the parameter.

Explain why your PR fixes it

By fixing the way clearTimeout() is being used, now the keyup event only generates one request:

One XHR request

Notes

The browser was generating one AJAX request per keystroke, ignoring the
timeout. The clearTimeout() function needs to be called with the ID
value returned by setTimeout().
Copy link
Member

@voodoorai2000 voodoorai2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Niiiiice

@voodoorai2000 voodoorai2000 merged commit e370069 into consuldemocracy:master Jul 12, 2018
@javierv javierv deleted the fix_suggestions_keyup branch July 12, 2018 15:09
@javierm javierm self-assigned this Nov 11, 2021
@javierm javierm added the Bug label Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants