fix/docs(Switch): fix uncontrolled examples #7418
Conversation
|
Preview: https://patternfly-react-pr-7418.surge.sh A11y report: https://patternfly-react-pr-7418-a11y.surge.sh |
There was a problem hiding this comment.
Great update @kmcfaul !! It's really cool that we can get an uncontrolled input by setting isChecked to null!
According to the react docs, it looks like we can specify an initial value using defaultChecked but leave subsequent updates uncontrolled.
I see some defaultChecked logic in Checkbox.tsx and Radio.tsx, both of which also have uncontrolled examples, maybe we can expose defaultChecked in the props and use it in a similar way here? WDYT?
| @@ -176,19 +176,9 @@ import { Switch } from '@patternfly/react-core'; | |||
| aria-label="Message when on" | |||
| label="Message when on" | |||
| labelOff="Message when off" | |||
| isChecked | |||
| isChecked={null} | |||
There was a problem hiding this comment.
| isChecked={null} | |
| isChecked={null} | |
| defaultChecked |
| @@ -176,19 +176,9 @@ import { Switch } from '@patternfly/react-core'; | |||
| aria-label="Message when on" | |||
There was a problem hiding this comment.
Should we also update the aria-label when appropriate to demonstrate the intention? it's possible that, when it's uncontrolled, there are a few extra considerations to make to make sure the aria-label is meaningful. Like, what should we have the aria-label say if it wont change when the switch is on/off?
There was a problem hiding this comment.
I opened issue #7468 for this issue as well as an issue I noticed with the aria-labelledby attribute.
There was a problem hiding this comment.
Hmmm. Katie, I think you were right, this probably never worked before. I would expected it to work like the checkbox or radio in that you do not need to pass isChecked to make it uncontrolled. I think we need to update the code so that is the case. We can also support isChecked being passed as null in order to not break people...
|
I took the logic seen in the DataListCheck component to determine whether the isChecked or defaultChecked prop should be used on the inner input's checked attribute. |
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #7255
@tlabaj Did these work before? Uncontrolled inputs shouldn't be setting
isChecked, and half of the example switches are redundant now that it works. I'm curious if this was meant as a static demo initially.