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 debounce on variant search dropdown #8451

Merged

Conversation

Matt-Yorkley
Copy link
Contributor

What? Why?

This wasn't applied correctly so there was zero debounce, leading to a big excess of pointless requests whilst typing in the variant search dropdown.

What should we test?

If you try the "add variants" dropdown in the new subscription page it should be obvious that each letter typed in the box triggers a new request (check the network tab). After this, there should be a short delay, and only the last value will be sent in a single request (after half a second).

Screenshot from 2021-11-04 20-45-21

Release notes

Fixed debounce on variant search dropdowns

Changelog Category: Technical changes

This wasn't applied correctly so there was zero debounce, leading to a big excess of pointless requests whilst typing in the variant search dropdown
@Matt-Yorkley Matt-Yorkley self-assigned this Nov 4, 2021
@mkllnk
Copy link
Member

mkllnk commented Nov 5, 2021

Is this something we could test for? Like sending five keys in and expect that only one request has been made after 600 ms.

@Matt-Yorkley
Copy link
Contributor Author

Is this something we could test for? Like sending five keys in and expect that only one request has been made after 600 ms.

It might be tricky with Capybara. I could have a go, but it's much easier to test it manually. I'd like to get this one in the release as the effects are really problematic in some cases (large hubs with lots of variants and overrides).

@Matt-Yorkley Matt-Yorkley mentioned this pull request Nov 5, 2021
8 tasks
@filipefurtad0 filipefurtad0 self-assigned this Nov 5, 2021
@filipefurtad0 filipefurtad0 added the pr-staged-uk staging.openfoodnetwork.org.uk label Nov 5, 2021
@filipefurtad0
Copy link
Contributor

Hey @Matt-Yorkley ,

Awesome 👀

Indeed huge difference in what the request numbers is concerned 👍

Below (left) a staging server without this PR and on the right the staging server after staging:

Peek 2021-11-05 18-51

The string above is not representative in what existing variants is concerned, but this was tested as well and all works as before.

500 ms seems just about the right value not to compromise usability/responsiveness. This looks perfect 🚀

@filipefurtad0 filipefurtad0 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Nov 5, 2021
@Matt-Yorkley Matt-Yorkley merged commit 8cb2a2c into openfoodfoundation:master Nov 5, 2021
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.

3 participants