digdir / designsystemet

Designsystemet
https://designsystemet.no
MIT License
73 stars 36 forks source link

Empty value for controlled Radio.Group should not throw React warning #1947

Open mgunnerud opened 4 months ago

mgunnerud commented 4 months ago

Description of the bug

When using a controlled <Radio.Group> component, what should be the default value of value? Using the following example from storybook in code with a slight modification, initial value of value is set to empty string:

const [value, setValue] = useState<string>('');

<Radio.Group legend='Velg pizza (påkreved)' description='Alle pizzaene er laget på våre egne nybakte bunner og serveres med kokkens egen osteblanding og tomatsaus.' value={value} onChange={setValue}>
    <Radio value='ost'>Bare ost</Radio>
    <Radio value='Dobbeldekker' description='Chorizo spesial med kokkens luksuskylling'>Dobbeldekker</Radio>
    <Radio value='flammen'>Flammen</Radio>
    <Radio value='snadder'>Snadder</Radio>
</Radio.Group>

When running in dev mode, checking one of the checkboxes will print a React warning to the console: Warning: A component is changing an uncontrolled input to be controlled. This is likely caused by the value changing from undefined to a defined value, which should not happen. Decide between using a controlled or uncontrolled input element for the lifetime of the component.

However, the value is not undefined, it is empty string. What should be the default value of a controlled radio group? It is possible to set the value to something that will not evaluate to a false value that is neither of the radio values (e.g '-') but that does not feel correct. Should controlled <Radio.Group> accept empty string as default value without throwing the warning?

Steps To Reproduce

  1. Create a controlled <Radio.Group> component with the code above
  2. Run the code in dev mode
  3. Check one of the checkboxes, and notice the warning in console

Additional Information

No response

mimarz commented 4 months ago

Thanks for bringing this to our attention!

I can confirm this is error is being thrown, we'll look into it in not to long hopefully :)

We are currently working on some bigger internal changes that we want to have finished by end of May, so unsure when we'll get to have a look at this.

We have had slightly different solutions for uncontrolled vs controlled state handling for our React components. We have been contemplating copying Radix controlled-state hook for a common way to solve this, but its still been on todo.

Putting this issue up for "help-wanted" if anyone out there wants to take a shot at this :)