Skip to content

Use leading flag set to true in debouncing events from extension trees#88051

Merged
alexr00 merged 9 commits intomicrosoft:masterfrom
solomatov:tree-debounce-to-throttle
Jan 24, 2020
Merged

Use leading flag set to true in debouncing events from extension trees#88051
alexr00 merged 9 commits intomicrosoft:masterfrom
solomatov:tree-debounce-to-throttle

Conversation

@solomatov
Copy link
Contributor

@solomatov solomatov commented Jan 3, 2020

The trees in the extension host has a problem: all events are debounced by 200ms, thus delaying changes by this amount. This PR sets the leading flag in debounce to true, thus removing this delay for well behaved extensions, i.e. extensions which fire just one change event at the right place, thus improving UX for them.

For not so well behaved extensions, this isn't that a big problem, we just make one more tree refresh, which, in my opinion, is a good tradeoff.

@alexr00 alexr00 added this to the January 2020 milestone Jan 6, 2020
@solomatov
Copy link
Contributor Author

@alexr00, I refactored test to test actual production behavior. Found in the process a small bug in event merging in Event.debounce.

@alexr00
Copy link
Member

alexr00 commented Jan 10, 2020

Thanks for making the changes! I'm not going to have a chance to take a good look at this for the next couple days, but I'll get back to it sometime next week.

@solomatov solomatov closed this Jan 14, 2020
@solomatov solomatov deleted the tree-debounce-to-throttle branch January 14, 2020 22:53
@solomatov solomatov restored the tree-debounce-to-throttle branch January 14, 2020 22:53
@solomatov solomatov reopened this Jan 15, 2020
@solomatov
Copy link
Contributor Author

@alexr00 Could you please take a look? It would be very nice to have it in Jan release.

@alexr00
Copy link
Member

alexr00 commented Jan 23, 2020

@joaomoreno are you ok with the changes in event.ts and event.test.ts?

Copy link
Member

@alexr00 alexr00 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@alexr00 alexr00 merged commit 28bf0b1 into microsoft:master Jan 24, 2020
@solomatov solomatov deleted the tree-debounce-to-throttle branch February 5, 2020 20:14
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants