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

Remove jQuery workaround that's no longer needed #1191

Merged
merged 1 commit into from Dec 11, 2013

Conversation

mbest
Copy link
Member

@mbest mbest commented Nov 27, 2013

Code in question:

if (isClickOnCheckableElement(element, eventType)) {

jQuery 1.9 upgrade notes: http://jquery.com/upgrade-guide/1.9/#checkbox-radio-state-in-a-trigger-ed-quot-click-quot-event

jQuery bug ticket: http://bugs.jquery.com/ticket/3827

@mbest
Copy link
Member Author

mbest commented Nov 22, 2013

Related to issue #987

@mbest
Copy link
Member Author

mbest commented Nov 22, 2013

We should be able to remove it for jQuery version 1.9 and higher, at least.

The code should also be updated to check if the checked value was modified by the event callback (for example, in response to some validation check), and then not reset it.

@mbest
Copy link
Member Author

mbest commented Nov 27, 2013

It appears that this was only fixed in jQuery for checkboxes and not for radio buttons. The 1.9 upgrade notes mention both, but this is wrong.

… IE<9 and jQuery, use the element.click method. This is the same fix that jQuery added in 1.9 for checkboxes (but not for radio buttons).
@SteveSanderson
Copy link
Contributor

Nice, thanks!

SteveSanderson added a commit that referenced this pull request Dec 11, 2013
Remove jQuery workaround that's no longer needed
@SteveSanderson SteveSanderson merged commit 262d8cc into master Dec 11, 2013
@mbest mbest deleted the 1191-better-checkbox-click-fix branch December 11, 2013 20:05
@rniemeyer rniemeyer mentioned this pull request Feb 11, 2014
7 tasks
@shri3k
Copy link

shri3k commented Mar 9, 2015

@mbest Late to the party, but I'm facing the issue that you mentioned about the radio button. I've tried upgrading jQuery to 1.11.2 (can't upgrade to 2.x, blame IE 😞 ) too but didn't do much help. However, I did add back the fragment of code that was removed from this commit and it worked like it used to on Knockout 3.0. I'm currently on 3.3 right now but just wanted to check if there's any work being done on this part. Also, let me know if I can help in any form.

@mbest
Copy link
Member Author

mbest commented Mar 10, 2015

@sinkingshriek As far as we know, it's working. If you can put together an example that doesn't work properly, please open a new issue with the details.

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

Successfully merging this pull request may close these issues.

None yet

3 participants