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

Fix default value for checkbox group #7912

Conversation

tedliang
Copy link
Contributor

For checkbox group, the default value should be [] instead of false.
https://codesandbox.io/s/react-hook-form-js-forked-6hd9uc

@@ -254,6 +254,9 @@ export function createFormControl<
);

isUndefined(defaultValue) ||
(isUndefined(defaultValue) &&
Copy link
Member

Choose a reason for hiding this comment

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

i will try with check undefined.

Copy link
Member

Choose a reason for hiding this comment

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

hmm i think we need to look a bit further than doing === false check, as undefined check failed.

@bluebill1049
Copy link
Member

i think instead of introducing this fix (which i appreciated your time to look into), maybe we can simply apply a defaultvalues to be [] to resolve this issue.

@tedliang
Copy link
Contributor Author

tedliang commented Feb 25, 2022

If you have a look the test in this merge request, we init the form with string[] type for field test:

    const { register, handleSubmit } = useForm<{
      test: string[];
    }>();
    ...
    await waitFor(() =>
      expect(callback).toHaveBeenCalledWith(
        {
          test: [],
        },
        expect.any(Object),
      ),
    );

When submit button is clicked, the value returned from hook form is false instead of []. The type is not matched. Do you think it's a desired behaviour from user point of view?

It returns [] in v6. This is one of the backward incompatible changes when we are upgrading to v7.

@bluebill1049
Copy link
Member

bluebill1049 commented Feb 25, 2022

Do you think it's a desired behaviour from user point of view?

Probably not, but like what I have suggested above, there is a workaround to achieve what is desired behavior. The nature of not knowing what's in the render (without defaultValue) is the trouble here, which presented below with your fix

-> first checkbox does not check ok default
-> the second checkbox oh ok the last one is default false and there is more than one I change the default value to [] on the run time.

I would like to avoid this above, as this is troublesome as it depends on the number of the checkbox to determine defaultValue while it's more predictable for the user to simplify supply with a default value as an empty array for the lib to fallback.

@tedliang
Copy link
Contributor Author

Right, I can see your point. Determine it's checkbox group or not by the default value instead of number of elements. I do have this kind of requirement. There is only one option in the checkbox group. We still want the return value to be string[]. At the moment, my workaround is adding a hidden input. I would like to avoid this workaround. Will you consider to support this?

@bluebill1049
Copy link
Member

could you provide a defaultValues at the useForm? or that's not a valid worked around? I think it's safer to tell hook form from the useForm level, that's a group of checkboxes or a single, like suggested above the fix you are trying to introduce is a just bandit, which could lead to an issue with a number of checkbox increase or decrease at the render phase.

@tedliang
Copy link
Contributor Author

tedliang commented Feb 25, 2022

Have a look this one: https://codesandbox.io/s/react-hook-form-js-forked-bz9j1g
I will expect the checkbox is not check after initialised.
click submit should return [],
click checkbox and then submit should return ['A'],
click checkbox again and then submit should return [].

I will prefer to tell hook form the checkbox field is single or group from register level.

@bluebill1049
Copy link
Member

I think you want to build a controlled component for your requirement and wrapped with Controller, all of these can be avoided. register is pretty simple and opinionated in terms of checkboxes, in terms of flexibility which you want with single/array, array/array, no default/array.

@bluebill1049
Copy link
Member

@tedliang
Copy link
Contributor Author

for the second one https://codesandbox.io/s/react-hook-form-js-forked-1b5njd?file=/src/App.js
It returns false when you click the submit button.
What do you expect when the first checkbox is clicked twice(check and then uncheck)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants