Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Checkbox / Radio: Setting checked attribute on underlying input #906 #939

Conversation

BenjaminNeilDavis
Copy link
Contributor

Checkbox / Radio: Setting checked attribute on underlying input #906

@interactivellama
Copy link
Contributor

This is working for me. I will look into it further.

}
this.state.checked = !this.state.checked;

this._toggleCheckedState();
this.$element.trigger('change', e);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might need to wrap this in the if(Boolean(e)){} check to make sure that e exists before you use it. This function can be called from JS without an event having happened...

@interactivellama
Copy link
Contributor

@BenjaminNeilDavis Yeah, let's wrap the event in an if and clean up the comments. Great find though!

@BenjaminNeilDavis BenjaminNeilDavis force-pushed the checkbox-radio-change-event branch from 0a46f3b to bacd170 Compare December 16, 2014 17:22
# The first commit's message is:
Checkbox / Radio: Setting checked attribute on underlying input #906

# This is the 2nd commit message:

bump dist

# This is the 3rd commit message:

Checkbox / Radio: Setting checked attribute on underlying input #906
@BenjaminNeilDavis BenjaminNeilDavis force-pushed the checkbox-radio-change-event branch from bacd170 to 9a4721f Compare December 16, 2014 17:22
@interactivellama interactivellama added this to the 3.5.0 milestone Dec 17, 2014
interactivellama added a commit that referenced this pull request Dec 17, 2014
…event

Checkbox / Radio: Setting checked attribute on underlying input #906. Fixes #941.
@interactivellama interactivellama merged commit 79b7a75 into ExactTarget:master Dec 17, 2014
@@ -113,15 +113,14 @@
toggle: function(e) {
//keep event from firing twice in Chrome
if (!e || (e.target === e.originalEvent.target)) {
if(Boolean(e)){
//stop bubbling, otherwise event fires twice in Firefox.
e.preventDefault();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WAIT! what happened to e.preventDefault()??? This was necessary to fix Firefox firing the event twice.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, just tested, you broke Firefox. I'll fix...

interactivellama added a commit that referenced this pull request Jan 1, 2015
(refix-firefox-checkbox) fix firefox bug where checkbox appears to not... Fixes bug introduced by #939.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants