Allow checkboxes and radio buttons to be selected with the keyboard #1709

Merged
merged 2 commits into from May 27, 2011

Conversation

Projects
None yet
3 participants
@shanag
Contributor

shanag commented May 25, 2011

No description provided.

@StevenBlack

This comment has been minimized.

Show comment Hide comment
@StevenBlack

StevenBlack May 25, 2011

With latest ,Query these should be .prop(), not .attr()... Same throughout.

With latest ,Query these should be .prop(), not .attr()... Same throughout.

@shanag

This comment has been minimized.

Show comment Hide comment
@shanag

shanag May 25, 2011

Contributor

In this case it should be .attr(), because the attribute needs to be set for the refresh

Contributor

shanag commented May 25, 2011

In this case it should be .attr(), because the attribute needs to be set for the refresh

@shanag shanag closed this May 25, 2011

@shanag shanag reopened this May 25, 2011

@shanag

This comment has been minimized.

Show comment Hide comment
@shanag

shanag May 25, 2011

Contributor

I accidentally closed this pull request, it's still valid.

Contributor

shanag commented May 25, 2011

I accidentally closed this pull request, it's still valid.

@shanag

This comment has been minimized.

Show comment Hide comment
@shanag

shanag May 25, 2011

Contributor

I don't think it should be prop() because it needs to work for both label click and native control checked and using prop() won't allow going back and forth from the label-click case and native control (keyboard) toggle.

Contributor

shanag commented May 25, 2011

I don't think it should be prop() because it needs to work for both label click and native control checked and using prop() won't allow going back and forth from the label-click case and native control (keyboard) toggle.

@scottjehl

This comment has been minimized.

Show comment Hide comment
@scottjehl

scottjehl May 26, 2011

Contributor

Thanks Shana,
So, it looks like you're right that refresh needs attr. Maybe they all need to use prop then (all meaning, inside refresh too)?
Does that sound correct or am I missing something?
Thanks!

Contributor

scottjehl commented May 26, 2011

Thanks Shana,
So, it looks like you're right that refresh needs attr. Maybe they all need to use prop then (all meaning, inside refresh too)?
Does that sound correct or am I missing something?
Thanks!

@shanag

This comment has been minimized.

Show comment Hide comment
@shanag

shanag May 27, 2011

Contributor

Yes, that's right. I updated the code to use prop() throughout, including refresh. Thanks!

Contributor

shanag commented May 27, 2011

Yes, that's right. I updated the code to use prop() throughout, including refresh. Thanks!

scottjehl pushed a commit that referenced this pull request May 27, 2011

Scott Jehl
Merge pull request #1709 from shanag/checkboxradio-keyboard-fix
Allow checkboxes and radio buttons to be selected with the keyboard

@scottjehl scottjehl merged commit 4b40896 into jquery:master May 27, 2011

@scottjehl

This comment has been minimized.

Show comment Hide comment
@scottjehl

scottjehl May 27, 2011

Contributor

Excellent. Thanks, Shana.
Landed: c9d97ef

Contributor

scottjehl commented May 27, 2011

Excellent. Thanks, Shana.
Landed: c9d97ef

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