aragon / ui

🦚 UI kit for decentralized apps
https://ui.aragon.org/
MIT License
343 stars 127 forks source link

Checkbox [fix]: prevent default onkeypress action #817

Closed eliobricenov closed 3 years ago

eliobricenov commented 3 years ago

Description

Given that the <Checkbox/> component renders a <span> element with a <button> element as a parent, by default, the Enter key triggers the keypress event.

This PR disables the default action on the keypress event that triggers the click event later on.

Note: the checkbox can still be checked with the Space key thanks to the role attribute.

Resolves #626

welcome[bot] commented 3 years ago

Thanks for opening this pull request! Someone will review it soon 🔍

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

eliobricenov commented 3 years ago

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 yarn. I figured they were not necessary but the Travis build was failing without them. Either way, I already removed them because I also think they're out of scope for this PR.

nivida commented 3 years ago

@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 :-)

welcome[bot] commented 3 years ago

Congrats on merging your first pull request! Aragon is proud of you 🦅 Eagle gif