-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
fix(checkbox): pass disabled prop to input #2800
Conversation
💖 Thanks for opening this pull request! 💖 Here is a list of things that will help get it across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
@@ -250,6 +237,7 @@ export default class Checkbox extends Component { | |||
{...htmlInputProps} | |||
checked={checked} | |||
className='hidden' | |||
disabled={disabled} |
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.
This is the actual change, passing disabled attribute to the input if set as a prop to Checkbox
Codecov Report
@@ Coverage Diff @@
## master #2800 +/- ##
=======================================
Coverage 99.74% 99.74%
=======================================
Files 160 160
Lines 2762 2762
=======================================
Hits 2755 2755
Misses 7 7
Continue to review full report at Codecov.
|
Thanks much! Let's add a test for this. The
|
…in case the Checkbox has truthy disabled property
wrapper.find('input').should.be.checked() | ||
}) | ||
|
||
it('is applied to the underlying html input element', () => { |
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.
These are the tests, better to review in Split mode as GitHub did a diff mess because of the Lint
@levithomason - done, added two tests, one check for when disabled property is truthy, and the other when it's false. |
Thank you so much! |
Released in |
There is an issue with the input element that is created by
<Checkbox>
.When the
<Checkbox>
is disabled, the input is not disabled and it allows to focus on the element.See an example in: https://codesandbox.io/s/3x9rn7lo5
The proposed solution is to add disabled attribute to the
input
in case the user asks the<Checkbox>
to be disabled.