🐛 Issues with Checkbox onClick event

[tjosepo]

Describe the bug

The Checkbox component has some odd behaviors when compared to <input type="checkbox" />, when reacting to a user input.

• The onClick event is fired twice, once for the label, then for the underlying input element.
• The event.target.value of the checkbox is always set to "on", even when a value prop is specified.

These issues make working with the onClick event difficult.

Steps to reproduce

<Checkbox
 value="custom value"
 onClick={(e) => {
  console.log(e.target);
  console.log(e.target.value)
}}>
  Milky Way
</Checkbox>

Expected results

• Only one onClick event fired, targetting the input DOM element.
• event.target.value is equal to the value prop of the Checkbox element.

Reproducible demo

You can copy/paste the code from this issue in the Orbit Storybook.

[fraincs]

For one I think that indeed the checkbox value should be synced with the input value.

As for the double onClick firing, what if someone wants to click on the label and retrieve it's text? Removing the onClick does not seem to be the right approach.

[patricklafrance]

Hey @tjosepo!

Indeed it should not be fired twice and the value should match the current value. I can't remember if it's doable thought given how the component is currently structured or if it would requires a rewrite of the component.

The double click and the wrong target value are happening because the onClick event handler ends up being rendered on the label rather the input. It's always an issue with components wrapping other elements.

Most of the time, I add additional properties for sub elements:

• ...rest would render on the wrapping element
• ìnputProps would render on the input element

Not sure if it should be done or not for this one.

What's your use case? For the example you provided, you are expected to use onValueChange .

[tjosepo]

I think you're right, onChange and onValueChange does prevent the wierd behaviors.

However, the reason I couldn't use them is because they did not fire consistently when holding the Shift Key.

I was working on a page that let you hold shift to select multiple checkboxes. onClick was the only reliable way to listen to events from the user while holding shift, but it had the issue I described that prevented me from doing so.

[patricklafrance]

Oh, interesting.

Indeed it does work most of the time when holding the Shift Key but it seems to hang from time to time. That's odd.

IMO this is the real bug, would you mind renaming this issue to phrase it around the Shift Key issue?

The other issues you discovered could also be fix while fixing the shift key issue:

• onChange and onValueChange should work consistently with Shit Key + Click
• onClick event handler should be forwarded to the input element
• Other event handlers like onFocus, onBlur, etc.. should also be tested