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

DataListCheck: decide on one of checked/isChecked and make sure the checked property is propagated on the DOM #6762

Closed
KKoukiou opened this issue Jan 11, 2022 · 5 comments · Fixed by #6803 or #7163
Assignees
Milestone

Comments

@KKoukiou
Copy link
Collaborator

Describe the issue. What is the expected and unexpected behavior?

If providing only the isChecked property to the component we get errors in console:

Warning: A component is changing an uncontrolled input to be controlled.

Passing only the 'checked' property seems to work properly, apart from the DOM not being updated, aka the check attribute never appears on the in the DOM.

Is this a bug or enhancement? If this issue is a bug, is this issue blocking you or is there a work-around?
bug

What is your product and what release version are you targeting?
Anaconda WebUI

@mcarrano
Copy link
Member

@tlabaj can you take a look at this? Is there are good reason to have redundant props here?

@boaz0
Copy link
Member

boaz0 commented Jan 16, 2022

@KKoukiou @mcarrano

When you want the checkbox to be uncontrolled (you're not using React state to determine the state of the checkbox) then checked is the right prop.
However, once you're passing onChange the checkbox is controlled and the state value needs to be passed in isChecked.

An example of controlled DataListCheck

const [isChecked, setIsChecked] = React.useState(false)

<DataListCheck isChecked={isChecked} onChange={value => setIsChecked(value)} ... />

An uncontrolled DataListCheck example

const defaultValue = true
<DataListCheck checked={defaultValue} />

I hope that was clear enough.

@nicolethoen nicolethoen moved this from Backlog to PR Review in PatternFly Issues Jan 17, 2022
@nicolethoen nicolethoen added this to the 2022.01 milestone Jan 17, 2022
Repository owner moved this from PR Review to Done in PatternFly Issues Jan 20, 2022
@KKoukiou
Copy link
Collaborator Author

Sorry I forgot to answer here.

I am using it the expected way with the Controlled variant and I still get the warnings:

My code looks like:

                     <DataListCheck                                                                 
                             name={"check-action-" + disk.target}                                   
                             onChange={checked => {                                                 
                                 onChange(index, checked);                                          
                             }}                                                                     
                             isChecked={!!disk.checked} /> 

isChecked is obviouly assigned to a boolean, since it's casted.

However I still see the warnings in console:

Warning: A component is changing a controlled input to be uncontrolled. This is likely caused by the value changing from a defined to undefined, which should not happen. Decide between using a controlled or uncontrolled input element for the lifetime of the component

@KKoukiou KKoukiou reopened this Feb 23, 2022
@boaz0
Copy link
Member

boaz0 commented Feb 24, 2022

I couldn't reproduce this, What version of patternfly-react you're using?

@KKoukiou
Copy link
Collaborator Author

"@patternfly/react-core": "4.198.5"

@mcarrano mcarrano removed this from the 2022.01 milestone Feb 24, 2022
@mcarrano mcarrano added this to the 2022.05 milestone Mar 11, 2022
@nicolethoen nicolethoen moved this from Done to Not started in PatternFly Issues Mar 29, 2022
@thatblindgeye thatblindgeye moved this from Not started to PR Review in PatternFly Issues Apr 1, 2022
Repository owner moved this from PR Review to Done in PatternFly Issues Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
5 participants