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

Checkbox: onChange called multiple times in a row #2370

Closed
MrSauceman opened this issue Dec 9, 2017 · 6 comments
Closed

Checkbox: onChange called multiple times in a row #2370

MrSauceman opened this issue Dec 9, 2017 · 6 comments
Labels

Comments

@MrSauceman
Copy link

I found that the Checkbox element calls its onChange event several times each time you check or uncheck the box. It has something to do with the label that is generated alongside the checkbox because when I manually remove the label from the html, the call is only made once.

https://codesandbox.io/s/34vrxvlwqp

That link reproduces the issue if you check the console. Notice how the number of times onChange is called varies for each checkbox because of its props. For instance, adding an id prop to the Checkbox and clicking its label causes it to call its onChange event 3 times, while clicking the actual checkbox causes onChange to be called twice. I believe this has something to do with the <label for="id">Label</label>. The number of times called varies based on whether you click the actual checkbox or the label.

Seems very inconsistent and you can see that checkbox 3 in the demo cannot even set its value correctly when you click its label because onChange is called 3 times.

@layershifter layershifter changed the title Checkbox onChange called multiple times in a row Checkbox: onChange called multiple times in a row Dec 10, 2017
@HariSoni87
Copy link

We had a same problem and we have resolved it using a wrapper component.

const CheckBoxComponent = props => {
const { onChange, id, ...rest } = props
return (<Checkbox {...rest} onChange={(e, data) => onChange(e, { ...data, id })} />)
};

@jkudish
Copy link

jkudish commented Dec 11, 2017

Having the same issue with 0.77.1 -- not a problem in 0.76.0

@yoshatest
Copy link

I've got a similar problem on Radio (no label). Clicking fires events twice 1) checked=true, 2) checked=false, result in no toggle at all. Reverting to 0.76.0 fixes the problem.

@ghost
Copy link

ghost commented Dec 20, 2017

Added a PR, Hopefully it solves the issue 👍

@ericbock
Copy link

I'm also seeing the change handler being called twice. Thinking that it might be related to the component binding both onChange and onClick to the same handler, I tested focusing the checkbox and toggling with the spacebar. That seems to work; my change handler is only called once per toggle.

levithomason pushed a commit that referenced this issue Jan 10, 2018
* Fix: [#2370]

-  onChange triggered twice when passing id attribute

Details: The Checkbox Component was rendering a label htmlFor attribute which will bind between
the label and the input tag when being clicked. The problem is that the click (change) handler
is NOT attached on the input tag but it's attached on the parent <ElementType ... /> component.

Below this ElementType we've both the input tag and the label tag (as element children), and once we click on the label
the ElementType element will get the click (change) event thanks to Event Bubbling in JavaScript,
which therefor will trigger the checkbox state change, but if htmlFor={id} is supplied then clicking on the
label tag will trigger the input tag and the ElementType, and input tag click will be bubbled up again
into ElementType which will make it trigger the onChange event twice.

Since we've the correct behavior of the label being linked to the input because of attaching the event on
the parent element then the usage of htmlFor is buggy and already implemented.

* Fix lint things

* fix(Checkbox): update usage of `id` prop

* style(Checkbox): add `id` prop to typings

* test(Checkbox): update tests, add new tests
@levithomason
Copy link
Member

Fixed in #2392

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

No branches or pull requests

7 participants