[#60649] Change op-autocompleter debounce to 250ms #17646
Draft
+7
−14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Increases the debounce to a more typical value (200-300ms) for a complex web application. The current value (50ms) results in a large number of unnecessary API requests - on installatons with large numbers of work packages, this can lead to severe performance degradation. This partially reverts a change made in commit b0fffbc Signed-off-by: Alexander Brandon Coles <[email protected]>
myabc
force-pushed
the
fix/60649-missing-op-autocompleter-debounce
branch
from
c13b2f2 to
b7cf586
Compare
Fixes `Type 'Timer' is not assignable to type 'number' error when running `npm run test:watch`. node typings for `setInterval()` specify `Timer` as its return type. This patch explicitly calls the browser variant, which returns `number`. See also TypeStrong/atom-typescript#1053
3 tasks
Implements a `debounceTimeMs` input property, allowing us to tweak the time in component subclasses and individual instances if need be. This also reverts the zero initial debounce (also introduced in commit b0fffbc).
myabc
commented
Jan 17, 2025
Comment on lines
-526
to
-529
| if (this.initialDebounce) { | ||
| this.initialDebounce = false; | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Removing the logic that disables the debounce on first user interaction - I don't think this behaviour makes sense (do we really want to make API calls for single letter query strings - in fact it would make more sense to me to only make requests when a minimum number of characters have been entered by the user)
There was a problem hiding this comment.
I vaguely remember that I added that to stabilize end-to-end tests. It was done in this commit: 9aa36e5
Sadly, I don't remember much of it and I did not add a lot of comments or information in the commit message.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
OpAutocompleterComponent, not just the work package relations autocompleterWorkPackageRelationsAutocompleteComponentTicket
https://community.openproject.org/wp/60649
What are you trying to accomplish?
Change op-autocompleter debounce to 250ms
Increases the debounce to a more typical value (200-300ms) for a complex web application. The current value (50ms)* results in a large number of unnecessary API requests - on installatons with large numbers of work packages, this can lead to severe performance degradation.
Screenshots
Screen.Recording.2025-01-17.at.13.17.08.mov
What approach did you choose and why?
Merge checklist