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

Check whether the mousedown event belonged to a valid drop target before preventing default #1778

Merged
merged 1 commit into from May 8, 2014

Conversation

Projects
None yet
7 participants
@andrewnicols
Contributor

andrewnicols commented Apr 11, 2014

This issue affects 3.16.0 (yay).
Sorry for the spam on the previous issue. I have yet to get hub to make sensible pull requests against non-master branches.

@yahoocla

This comment has been minimized.

yahoocla commented Apr 11, 2014

CLA is valid!

@andrewnicols

This comment has been minimized.

Contributor

andrewnicols commented Apr 11, 2014

Note, I did try and write some tests for this, but I'm not sure how best to simulate an actual mousedown. Simulate simulates the events rather than the actual mousedown so it's not possible to check whether the field was properly focused.

@triptych

This comment has been minimized.

Contributor

triptych commented Apr 11, 2014

//cc @tilomitra for review?

andrewnicols added a commit to andrewnicols/moodle that referenced this pull request Apr 22, 2014

@andrewnicols

This comment has been minimized.

Contributor

andrewnicols commented Apr 22, 2014

@tilomitra - any chance you can look at this. we have a release in around 2-3 weeks and I'd really like to backport this fix into our repo before then.

@andrewnicols

This comment has been minimized.

Contributor

andrewnicols commented Apr 22, 2014

Also note, this is a major breakage. Any page where DD is enabled will not be gesture scrollable by a touch-based device.

@robframpton

This comment has been minimized.

robframpton commented May 2, 2014

This also fixes a regression caused by 9f96abc where inputs cannot receive focus via mousedown if they are nested inside of a drag node (see here http://codepen.io/Robert-Frampton/pen/FhdmD).

@andrewnicols

This comment has been minimized.

Contributor

andrewnicols commented May 2, 2014

@Robert-Frampton that's exactly the bug it's fixing.

@andrewnicols

This comment has been minimized.

Contributor

andrewnicols commented May 2, 2014

@triptych, @tilomitra - any chance of a review on this - it's now been three weeks.

@marclundgren

This comment has been minimized.

Contributor

marclundgren commented May 2, 2014

@andrewnicols +1 for this fix

@triptych

This comment has been minimized.

Contributor

triptych commented May 4, 2014

I asked @tilomitra to take a look. I'll follow up on Monday.

@tilomitra

This comment has been minimized.

Contributor

tilomitra commented May 5, 2014

Hey guys - sorry I was heads-down on some stuff last week. Taking a look at this now @andrewnicols. Will get back to you by EOD.

@ezequiel

This comment has been minimized.

Contributor

ezequiel commented May 8, 2014

It's a shame this took so damn long to "review."

@andrewnicols,

Thanks. This looks great.

@ezequiel ezequiel merged commit f89052f into yui:dev-3.x May 8, 2014

2 checks passed

continuous-integration/travis-ci The Travis CI build passed
Details
default User has a valid Yahoo CLA
Details
@tilomitra

This comment has been minimized.

Contributor

tilomitra commented May 9, 2014

Thanks @ezequiel. Been swamped :(

MorrisR2 added a commit to MorrisR2/moodle that referenced this pull request May 30, 2014

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