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 onClick provides wrong data [onChange works fine] #1936

Closed
niiapa opened this issue Aug 6, 2017 · 3 comments
Closed

Checkbox onClick provides wrong data [onChange works fine] #1936

niiapa opened this issue Aug 6, 2017 · 3 comments
Labels

Comments

@niiapa
Copy link

niiapa commented Aug 6, 2017

✔ BUGS → This form is required:

Steps

Check or uncheck a checkbox

Expected Result

Checked empty checkbox returns true for 'checked' value in data params and vice versa

Actual Result

onClick registers too early and the 'checked' value is what is was prior (inverse of truth)

Version

0.71.3

Testcase

Toggle a checkbox and view the data params for checked. It's the inverse of what it's supposed to be. I'm assuming the onClick event registers before it actually changes.

@layershifter
Copy link
Member

I think that current behaviour is correct. onClick represents only click event and changes nothing, while onChange represents the state change.

I'll left this issue open, possible @levithomason has another opinion.

@levithomason
Copy link
Member

levithomason commented Aug 20, 2017

The order of events is correct and the onChange handler should be used instead. However, comparing this to vanilla React with an <input type='checkbox' />, both onClick and onChange events return e.target.checked === true when clicking a controlled input.

We should update ours to behave identically:

  • create an unchecked Checkbox
  • user clicks Checkbox
  • onClick fires with checked: true (the proposed new value)
  • onChange fires with checked: true (the proposed new value)

@areinmeyer
Copy link
Contributor

I just ran across this issue as well. I'd be interested in trying out a PR for this and will try to put one together for it this week.

areinmeyer pushed a commit to areinmeyer/Semantic-UI-React that referenced this issue Aug 28, 2017
change onClick's checked value to be the new representation of the
checked state instead of the representation before the event was
triggered

BREAKING CHANGE: Checkbox.onClick has changed how the checked property
is defined.  To migrate update code that references checked property to
the opposite value currently assumed.
Closes Semantic-Org#1936
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

4 participants