Change date range immediately and only debounce chart updates #1025
Conversation
|
Apologies for not testing this yet. I've just reviewed a bunch of PRs, and hope to get to this on the next round! |
There was a problem hiding this comment.
Seems like the slider performance is much smoother, thanks @avin-kavish!
Before we merge this can you run npm lint to make sure there are no linting errors? Even better, if you rebase this onto master, it will run the linting automatically along with some tests in a github action. Thanks for your work on this!
| updateFrequencyDataDebounced(dispatch, getState); | ||
| clearTimeout(timeout) | ||
| // Quickdraw also means debounce in this context, but better change it. | ||
| // TODO: Refactor to two seperate variables for quickdraw and debounce |
There was a problem hiding this comment.
@avin-kavish Definitely agree with the TODO here, I think it would make this more readable. Could you add that refactor here please? Thanks!
|
Yeah sounds like reasonable changes. btw @eharkins I don't see a badge next to your name... are you also maintaining this library? I'm only asking because If I make changes but if @jameshadfield (who seems to be the maintainer) wants different changes that would be a waste of time. |
Yep :) |
This improves the responsiveness of the data range slider to not be laggy.
Fixes #1021
Related to #955
Testing
Probably deploy and test
Thank you for contributing to Nextstrain!