Skip to content
This repository was archived by the owner on Aug 26, 2021. It is now read-only.

Fix docs for control inputs with initial checked state #551

Merged

Conversation

sukhrajghuman
Copy link
Contributor

@sukhrajghuman sukhrajghuman commented Nov 13, 2018

Since this is an uncontrolled component, passing the checked attribute in <AUradio> and <AUcheckbox> is overriding the checked attribute in the DOM. This means the checkbox/radio box will remain checked. If we use defaultChecked, it will be checked initially and allow for subsequent updates to be uncontrolled.

fixes #544

I'm also going to put #548 on hold for now. I believe it's fairly low priority.

@sukhrajghuman sukhrajghuman self-assigned this Nov 13, 2018
@sukhrajghuman sukhrajghuman added the 🐛 bug Something isn’t working the way it should. label Nov 13, 2018
@sukhrajghuman sukhrajghuman added this to the Sprint 116 milestone Nov 13, 2018
Copy link
Contributor

@azerella azerella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where you've changed the prop definitions for the AUradio component ?

@sukhrajghuman sukhrajghuman modified the milestones: Sprint 116, Sprint 117 Nov 13, 2018
@sukhrajghuman
Copy link
Contributor Author

sukhrajghuman commented Nov 13, 2018

no need to change it for the actual component itself. It's just a fix in the example. then will update the example in designsystem.gov.au.

I updated my comment in the PR to make it more clear.
See below as well
https://codesandbox.io/s/3v65nqxkvp

@azerella
Copy link
Contributor

@sukhrajghuman Now I'm more confused lol, the codepen example sets defaultChecked={true} and defaultChecked="some text". Is this correct way to use the prop?

@sukhrajghuman
Copy link
Contributor Author

sukhrajghuman commented Nov 14, 2018

It sets defaultChecked={true} and *defaultValue*="some text". try editing the text or unclicking the text-input and checkbox examples

@azerella
Copy link
Contributor

@sukhrajghuman Ahh, gotcha - LGTM

@alex-page
Copy link
Contributor

We need to clean these PR's up they have a lot of unrelated file changes 👍

Since this is an uncontrolled component, passing the `checked` prop is
overriding the `checked` attribute in the DOM. If we use
`defaultChecked` it will allow for subsequent updates to be
uncontrolled.
@azerella azerella force-pushed the feature/changed-control-input-to-defaultchecked branch from e616a43 to 4420b7c Compare November 21, 2018 22:47
@azerella
Copy link
Contributor

@alex-page Fixed with a rebase

@sukhrajghuman sukhrajghuman merged commit f903d89 into develop Nov 22, 2018
@sukhrajghuman sukhrajghuman deleted the feature/changed-control-input-to-defaultchecked branch November 22, 2018 02:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐛 bug Something isn’t working the way it should.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants