-
Notifications
You must be signed in to change notification settings - Fork 122
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
Checkbox [fix]: prevent default onkeypress action #817
Conversation
Thanks for opening this pull request! Someone will review it soon 🔍 |
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.
The change itself looks good. But was it required to update the dependencies as for example rollup/plugin-node-resolve
? I especially ask this because it is a major version increase.
Overall we can say that changes per PR should be kept at a minimum. This means the description of a PR and the related issue/task defines the scope of it and any further changes have to be done in a separate PR with a related task.
@nivida Those changes were the result of running |
@eliobricenov Thanks for the insights! :) Feel free to create a task on Linear related to the required dependency updates and add it to the circle. I think it will be a quick thing for you to open a PR for and we would better see the work you already have done :-) |
Description
Given that the
<Checkbox/>
component renders a<span>
element with a<button>
element as a parent, by default, theEnter
key triggers thekeypress
event.This PR disables the default action on the
keypress
event that triggers theclick
event later on.Note: the checkbox can still be checked with the
Space
key thanks to therole
attribute.Resolves #626