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

Optimize useShortcut performance. #19341

Merged
merged 1 commit into from Dec 27, 2019
Merged

Conversation

@youknowriad
Copy link
Contributor

youknowriad commented Dec 27, 2019

#19332 introduced some performance regressions because of the high number of mousetrap event handlers triggers in the block list.

This PR adds a way to disable the handlers and we're back to similar performance results compared to master.

@youknowriad youknowriad force-pushed the try/performance-optim-shortcut branch from 5be6dde to eca5bef Dec 27, 2019
@youknowriad youknowriad merged commit b9ca11f into master Dec 27, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@youknowriad youknowriad deleted the try/performance-optim-shortcut branch Dec 27, 2019
@@ -34,9 +34,13 @@ function isAppleOS( _window = window ) {
function useKeyboardShortcut( shortcuts, callback, {
bindGlobal = false,
eventName = 'keydown',
isDisabled = false, // This is important for performance considerations.

This comment has been minimized.

Copy link
@aduth

aduth Jan 2, 2020

Member

It's important, but what is it meant to represent? I was confused when I first saw this, 'til I could understand the context for how it's used in this pull request. It would help to have some documentation for this hook.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.