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

defaultChecked not working #13

Open
seyade opened this issue May 8, 2020 · 4 comments
Open

defaultChecked not working #13

seyade opened this issue May 8, 2020 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@seyade
Copy link

seyade commented May 8, 2020

Hi Team

I'm trying to apply defaultChecked to Checkbox but it's not working. I get a warning saying:

Warning: ForwardRef contains an input of type checkbox with both checked and defaultChecked props..., etc.

How can I apply defaultChecked especially for pre-populating checkboxes?

Many Thanks

@AlexLisenkov
Copy link
Member

Hi @seyade ,

The defaultChecked is not a property of Checkbox but instead of CheckboxGroup.
Applying defaultChecked to the checkboxGroup will check all checkboxes by default.

If you wish to check a single Checkbox you can simply pass checked to it.

@AlexLisenkov AlexLisenkov self-assigned this May 8, 2020
@AlexLisenkov AlexLisenkov added the bug Something isn't working label May 8, 2020
@seyade
Copy link
Author

seyade commented May 8, 2020

Thanks @AlexLisenkov for replying. And great work by the way!

I think defaultChecked should also be applied to Checkbox as it is a supported props in React. For example I have a table full of checkboxes (about 50) in all columns and rows. I can populate the checkboxes with data from an api, but I also want to give users the choice to uncheck/check a populated checkbox. Using defaultChecked makes life easier in this context, I think.

@AlexLisenkov
Copy link
Member

Thank you for pointing out this issue, I believe there is something wrong with a Checkbox being controlled or not.

The misunderstanding, or flaw, is that the checked prop is now behaving like a defaultChecked. I agree with you, and the defaultChecked should be allowed and set the initial checked state.

If we add the defaultChecked there we will be left with the choice about what we should do with the checked prop.

  1. Disallow (or ignore) the checked prop, making it impossible to control checked states. Checkboxes will always and only be controlled by the group.

  2. Allow the checked prop, making it behave exactly like a checkbox. But making a checkbox controlled it will be impossible to make use of the allCheckers and noneCheckers, because they're not in charge of the state of a checkbox anymore.

Going for option 2 will be nice in a way that it will behave like a checkbox as how we're used to it. But it will also void the main feature of this package, easily add a checkbox to check all.

Can you give your opinion?

@createnl createnl deleted a comment from BerendPronk May 15, 2020
@seyade
Copy link
Author

seyade commented May 27, 2020

I guess, the most viable option will be to leave as it is since checked prop is the main prop for controlling state. Unless you give the user option to choose between defaultChecked and checked when necessary, but that will be too much for something simple like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants