Skip to content

Button: Fixed #7665 - Radio button & checkboxes ignore mouseclicks for minor mouse movements (2)#945

Closed
UltCombo wants to merge 2 commits into
jquery:masterfrom
UltCombo:button7665
Closed

Button: Fixed #7665 - Radio button & checkboxes ignore mouseclicks for minor mouse movements (2)#945
UltCombo wants to merge 2 commits into
jquery:masterfrom
UltCombo:button7665

Conversation

@UltCombo
Copy link
Copy Markdown
Contributor

Follow up of PR854.

@ghost
Copy link
Copy Markdown

ghost commented Mar 26, 2013

I know this is random, but thank you SO MUCH for this fix. This has an absolutely HUGE impact on touchscreen kiosk environments which also want the same app to work in the Mobile world. Kudos.

@UltCombo
Copy link
Copy Markdown
Contributor Author

@ohma1989 No problem. =]

@mikesherov and others, I'd like some comments/feedback/suggestions as well. To don't leave this PR in such void, I'll re-add the main points, reasons and possible drawbacks of this patch:

  • Most browsers (mainly Firefox) will not fire a click event in case the mouse has been dragged more than 2~3px after starting the click. Hence I'm using a mousedown+mouseup sequence instead of the native click event.
  • I've considered still using the native clicks and only firing a click after a mousedown+mouseup sequence that didn't trigger a click, but for that we'd need a setTimeout, thus making the code asynchronous (harder to test), inconsistent (native clicks would still trigger native handlers while dragged [synthetic] clicks would not) and finally could generate more serious event handler order due to the asynchronicity.
  • I've also considered putting a button element (inside the label) which natively allows for long drags. This would mess up the DOM structure (adding elements that the user didn't ask for), and it'd require feature testing as old IE does not toggle a checkbox for a button click inside of a label.

From the alternatives listed above, I believe mine is currently the cleanest. The only drawback of tracking the mousedown+mouseup sequences is that native click handlers will no longer fire (may generate issues with e.g. KO "checked" binding), but as jQuery has no warrant to trigger native handlers, this shouldn't be an issue for this PR.

If anyone can think of a better solution/improvement, please comment.

@jotavejv
Copy link
Copy Markdown

great ultcombo, congratulations

@mikesherov
Copy link
Copy Markdown
Member

@scottgonzalez, will you have some time to review this this week? My understanding of the button quirks is not that robust. While I can say I think this does the right things, I'll need you or @jzaefferer to review.

@mikesherov mikesherov closed this Mar 31, 2013
@mikesherov mikesherov reopened this Mar 31, 2013
@mikesherov
Copy link
Copy Markdown
Member

Oops, accidental close. My fingers are fat today.

@mikesherov
Copy link
Copy Markdown
Member

Closing this as per #1120 .

@mikesherov mikesherov closed this Oct 29, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants