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.

Sign up for GitHub

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

Jump to bottom

[ButtonBase] Reduce keyup timeout interval to 500ms #14120

Merged
merged 2 commits into from Jan 9, 2019
Merged

[ButtonBase] Reduce keyup timeout interval to 500ms #14120

merged 2 commits into from Jan 9, 2019

Conversation

rfbotto
Copy link
Contributor

@rfbotto rfbotto commented Jan 8, 2019

Closes #12873

mbrookes
mbrookes reviewed Jan 8, 2019
View reviewed changes
packages/material-ui/src/ButtonBase/focusVisible.js Outdated
clearTimeout(internal.keyUpEventTimeout);
internal.keyUpEventTimeout = setTimeout(() => {
internal.focusKeyPressed = false;
}, 1e3);
}, 5e2);
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need scientific notation?

Copy link
Member

Choose a reason for hiding this comment

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

I would use 500 here. It's more accessible to the people reading the source code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personally i would use the ms notation as i feel that is more familiar for people reading the code, but wanted to keep notation consistency, hence the 5e2.

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: button This is the name of the generic UI component, not the React module! labels Jan 8, 2019
@oliviertassinari oliviertassinari added component: ButtonBase The React component. PR: good for merge and removed component: button This is the name of the generic UI component, not the React module! labels Jan 8, 2019
@oliviertassinari oliviertassinari merged commit 1a20435 into mui:master Jan 9, 2019
@oliviertassinari
Copy link
Member

@rfbotto Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbrookes mbrookes mbrookes left review comments

@oliviertassinari oliviertassinari oliviertassinari approved these changes

Assignees
No one assigned
Labels
bug 🐛 Something doesn't work component: ButtonBase The React component.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rfbotto @oliviertassinari @mbrookes