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 React combobox debounce #4331

Closed
wants to merge 8 commits into from
Closed

Conversation

RobertJoonas
Copy link
Contributor

@RobertJoonas RobertJoonas commented Jul 10, 2024

Changes

In the PlausibleCombobox component (used in the filter menus and the prop_key picker), we're currently making too many API calls to fetch selectable options. Currently, we're calling the API for every keystroke. This PR fixes that - we will only make a request once 200ms has passed from the last keystroke (reusing the useDebouncedCallback custom hook introduced in #4318).

Also in this PR:

  • Alter the useDebouncedEffect custom hook to not set a delay for the initial function call. Currently the search in all breakdown modals is waiting 300ms before calling the API for the first time. This fixes that.
  • Remove redundant doubled "search" state from the BreakdownModal component.

Tests

  • This PR does not require tests

Changelog

  • This PR does not make a user-facing change

Documentation

  • This change does not need a documentation update

Dark mode

  • This PR does not change the UI

@RobertJoonas RobertJoonas force-pushed the react-combobox-debounce branch 2 times, most recently from 6568150 to 7d91706 Compare July 10, 2024 11:35
Copy link

Preview environment👷🏼‍♀️🏗️
PR-4331

if (isOpen) {
fetchOptions()
}
}, [search], 200)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this 200 in one case and 300 in another?

Proposal: Let's unify and make it a exported const somewhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, agreed. The 'standard' delay could even be baked in to theuseDebouncedEffect and not be configurable.

@RobertJoonas RobertJoonas mentioned this pull request Jul 12, 2024
4 tasks
@RobertJoonas
Copy link
Contributor Author

Closed in favor of #4349

@RobertJoonas RobertJoonas deleted the react-combobox-debounce branch July 12, 2024 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants