Checkbox Group Validation

[dbani-dev-shell]

Provide a general summary of the issue here

It seems the Checkbox Group validation example does not revalidate after the checkbox is pressed.

To note: Fantastic work on react-aria-components, keep up the great work!

🤔 Expected Behavior?

Checkbox Group should clear validation when at least one checkbox is selected.

😯 Current Behavior

Checkbox Group does not clear validation.

💁 Possible Solution

No response

🔦 Context

No response

🖥️ Steps to Reproduce

React Aria's Documentation Example for Checkbox Group Validation

1. Press Submit
2. Press a Checkbox
3. Validation is still displayed

Version

[email protected]

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

MacOS

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

[LFDanLu]

Thanks for filing this issue. I noticed this actually doesn't happen in our React Spectrum CheckboxGroup equivalent, seems to be due to our usage of a visually hidden input for the RAC Checkbox vs using an actual input. Typically onChange would fire here as a result of the checkbox state changing, updating the form's state but RAC's visually hidden input doesn't actually get pressed when you click on it hence no onChange firing. A potential solution would be to fire state.commitValidation here instead for RAC Checkbox group items, will need to test if this has any unforseen behavior changes though. cc @devongovett in case he has any further insight into this form behavior

[smozely]

#5836 appears to be a duplicate, but in that case isolating it to also happening on just Checkbox rather than a group. And that the behavior is different when using a mouse vs keyboard which is the same in the example

[nwidynski]

@LFDanLu I took some time to take a look at this.

This issue seems pretty isolated to RAC's implementation of a visually hidden input, so can't we fix this inside RAC? Maybe i am missing something related to how event dispatching is handled across different browsers/devices.

A slightly hacky, but working solution could look something like this:

/**
 * Setup a stable callback to propagate changes in the state.
 */
let onChange = props.onChange;

if (!groupState || groupState.isInvalid) {
  const event = new Event('change');
  const dispatch = () => inputRef.current?.dispatchEvent(event);

  onChange = chain(dispatch, onChange);
}

props.onChange = useEffectEvent(onChange);

[LFDanLu]

@nwidynski While that may certainly fix it for RAC, my concern is that someone may build their own checkbox using our checkbox hooks instead of opting to use RAC and use a visually hidden input in a similar fashion like we suggest here. They would then run into original problem as well then and need to create a similar fix local to their implementation.

[nwidynski]

@LFDanLu Got it.

Since a fix here won't do the trick for the isolated scenario, how about something like this in useCheckbox:

/**
 * Hook the label `onClick` handler to clear the validation state. 
 * This is useful when implementing `<Checkbox />` with a hidden input for styling.
 *
 * See: https://github.com/adobe/react-spectrum/issues/5693
 */
const { [privateValidationStateProp]: groupValidationState } = props;

const dispatch = () => {
  const { realtimeValidation, commitValidation } = groupValidationState
    ? groupValidationState
    : validationState;
  
  if (realtimeValidation.isInvalid) {
    commitValidation();
  }
};

labelProps.onClick = chain(dispatch, labelProps.onClick);

[LFDanLu]

@nwidynski Yep, that seems promising! Do you want to open a PR for that change?

[aulonm]

Hi,

We've come across the same bug, but it seems that it works with tab + space to check the checkbox, but it does not work with mouse clicks. Would #6079 fix that as well?

[LFDanLu]

Yes I believe so, is the behavior in these built docs essentially what you are looking for?

[aulonm]

Yes I believe so, is the behavior in these built docs essentially what you are looking for?

Yes! Both the Group Validation and the Individual Validation behaviour is what we have been trying to do, and the examples on the individual are spot on for our use case as well.

[poulet42]

Hello ! Just stumbled upon the issue too, I checked the storybook of #6079 and this seems to do the trick 🎉 Out of curiosity, is there anything preventing the PR to be merged @LFDanLu ? This would be awesome if it get released 🙏

[LFDanLu]

Thanks for the reminder, if I remember correctly there was some simplification that could be made via https://github.com/adobe/react-spectrum/pull/6079#discussion_r1561839144. The team was thinking of picking this PR (and some others that have fallen by the wayside) up ourselves but are crunching on getting the next release out first.