Skip to content
This repository has been archived by the owner on Dec 16, 2021. It is now read-only.

Delete onChange attribute when aliased #247

Merged
merged 1 commit into from
Dec 8, 2016

Conversation

pekala
Copy link
Contributor

@pekala pekala commented Dec 3, 2016

This should prevent the change event from firing twice when clicking on a checkbox, addressing this issue: preactjs/preact#358

This should prevent the change event from firing twice when clicking on a
checkbox, see here: preactjs/preact#358
@developit
Copy link
Member

Awesome! Thanks for this!

@developit developit merged commit 9e926ca into preactjs:master Dec 8, 2016
@cedmax
Copy link
Contributor

cedmax commented Jan 19, 2017

Hi, sorry to bother you on a closed issue, but I think the change you introduced broke the input[type="radio"]: looking at the code it seems like you are deleting the onChange for any input type not checkbox, in favour of an onInput, but the radio wouldn't work with it.

Sorry if I'm wrong, I've been trying to debug this issue since a while in my app and I'm a little tired :D

cedmax added a commit to cedmax/preact-compat that referenced this pull request Jan 19, 2017
As suggested in the comment to the merged PR (ref: preactjs#247) the event normalisation has broken the support for onChange on radio buttons. This is a potential fix
@developit
Copy link
Member

oooh, very good catch @cedmax! Are you able to open a new issue for that? We'll get it fixed right away.

@cedmax
Copy link
Contributor

cedmax commented Jan 19, 2017

sure thing, I'm on my way to do it

developit pushed a commit that referenced this pull request Jan 19, 2017
* Fix for the radio button not working with onChange

As suggested in the comment to the merged PR (ref: #247) the event normalisation has broken the support for onChange on radio buttons. This is a potential fix

* Using match on attribute type to identify checkbox and radio inputs

* added back the lowercase enforcing

* moved to test to ensure empty strings work too
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants