-
Notifications
You must be signed in to change notification settings - Fork 38
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
Wait until user is done typing for code update #550
base: master
Are you sure you want to change the base?
Conversation
self.first_diff_check = new Date(); | ||
} | ||
|
||
console.log(new Date() - self.first_diff_check); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
console logging really should get removed... :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than that, is it ready to merge?
Hmm.... I've tried using this for a bit, and I find the increased time before an update very distracting. It can take 2 or 3 seconds before anything updates to changes made (both the netgraph and the console). I guess I don't really understand what this change is doing. In the old code, it just checked every 0.1 seconds to see if anything has changed. In the new code, it looks like every 0.5 seconds it checks to see whether it has been 1.5s since the first new change to the code. Is that the intended logic? That seems like a strange choice to me. And it doesn't "wait until the user is done typing", which the name of this PR makes it seem like it is supposed to. |
As you stated correctly, the algorithm is basically:
I couldn't think of any other way to make sure the user was done typing. The lag that you're experiencing is probably a result of the period of checking for updates. Would you mind trying to play around with the constants (like switching the checkup period to 300ms and the change checker to 1s) and see if it feels smoother for you? |
Hmm.. I guess I'm not seeing the "wait until there hasn't been a change for 1.5 seconds" part of the code. It looks to me like it's "wait until it's 1.5 seconds after the first new change". Indeed, if you just try starting to type at random into the editor, it still updates things even if you're still typing. |
I don't quite understand. Wouldn't the majority of the lag come from the 1.5 second window? Is the goal of this design to wait for a person to have stopped typing for 1.5 seconds, and then fire off the data to the server? If so, then there's always going to be at least a 1.5 second lag, which seems like a lot to me. |
The auto-update toggle ( #676 ) makes this less of a pressing issue. Should we close this or should we leave it open in case someone wants to address it in the future? |
To me it seems to be still a nice to have in the future. |
Resolves #550