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

Apply same fix to radio buttons as for checkbox #3423

Open
mbest opened this Issue Dec 2, 2016 · 9 comments

Comments

Projects
None yet
4 participants
@mbest

mbest commented Dec 2, 2016

See https://bugs.jquery.com/ticket/3827 and 1fb2f92

This fix should also include radio buttons. Currently if you use jQuery to trigger a click on a radio button, event handlers get called before the checked state is updated. The release notes for 1.9.1 when the above change was included say it was changed for radio buttons even though it wasn't.

@gibson042

This comment has been minimized.

Show comment
Hide comment
@gibson042

gibson042 Dec 5, 2016

Member

Yes, we appear to have a problem in

// For checkbox, fire native event so checked state will be right

using this.type === "checkbox" instead of rcheckableType.test( this.type ).

Member

gibson042 commented Dec 5, 2016

Yes, we appear to have a problem in

// For checkbox, fire native event so checked state will be right

using this.type === "checkbox" instead of rcheckableType.test( this.type ).

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Dec 5, 2016

Member

Hi there.

I think we understand what you mean, but would you mind providing a test-case on http://jsbin.com/ or on similar resource? So it would be clear what behaviour you expect

Member

markelog commented Dec 5, 2016

Hi there.

I think we understand what you mean, but would you mind providing a test-case on http://jsbin.com/ or on similar resource? So it would be clear what behaviour you expect

@gibson042

This comment has been minimized.

Show comment
Hide comment
@gibson042

gibson042 Dec 5, 2016

Member

Such test case will also be necessary for a PR. Would you like to work on this, @mbest?

Member

gibson042 commented Dec 5, 2016

Such test case will also be necessary for a PR. Would you like to work on this, @mbest?

@markelog markelog removed the Needs review label Dec 5, 2016

@mbest

This comment has been minimized.

Show comment
Hide comment
@mbest

mbest Dec 5, 2016

Here is the jsbin that was linked in the Knockout issue: http://jsbin.com/yinare/4/edit?html,js,output

mbest commented Dec 5, 2016

Here is the jsbin that was linked in the Knockout issue: http://jsbin.com/yinare/4/edit?html,js,output

@gibson042 gibson042 closed this in b442aba Jan 19, 2017

@timmywil timmywil reopened this Mar 20, 2017

@timmywil timmywil added this to the 3.3.0 milestone Mar 20, 2017

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Mar 20, 2017

Member

This will be backed out due to the regression seen here. We will re-land with a fix for that regression in 3.3.

Member

timmywil commented Mar 20, 2017

This will be backed out due to the regression seen here. We will re-land with a fix for that regression in 3.3.

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Mar 20, 2017

Member

Also see #1367

Member

timmywil commented Mar 20, 2017

Also see #1367

@mbest

This comment has been minimized.

Show comment
Hide comment
@mbest

mbest Mar 20, 2017

Seems the same issue would apply to trigger('click') on checkboxes (code that has existed since 1.9.1).

mbest commented Mar 20, 2017

Seems the same issue would apply to trigger('click') on checkboxes (code that has existed since 1.9.1).

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Mar 21, 2017

Member

Right, but that's not a regression since 3.1.1. We plan to address that as well in 3.3.

Member

timmywil commented Mar 21, 2017

Right, but that's not a regression since 3.1.1. We plan to address that as well in 3.3.

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Jan 16, 2018

Member

@gibson042 is working on this, but it will not make it in the 3.3 release.

Member

timmywil commented Jan 16, 2018

@gibson042 is working on this, but it will not make it in the 3.3 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment