Skip to content
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

Regression with Drag&Drop #9598

Closed
derrell opened this issue Dec 4, 2018 · 2 comments
Closed

Regression with Drag&Drop #9598

derrell opened this issue Dec 4, 2018 · 2 comments

Comments

@derrell
Copy link
Member

derrell commented Dec 4, 2018

Describe the bug
When the drag target passes over a possible drop target enroute to a different drop target, and is dropped on the latter, the drop event is fired on the former instead of the latter as expected.

This regression was introduced with commit 4160586
@johnspackman This was your commit.

To Reproduce
In the following playground sample, drag the "Drag Me" label over "First Window" then continue on to "Second Window". When you release the mouse while over "Second Window", the alert will tell you that the drop event was actually wrongly fired on "First Window". If, on the other hand, you drag the round-about way, around the windows so that the drag never passes over "First Window" and goes immediately to "Second Window" where it is dropped, the correct object receives the drop event.

http://tinyurl.com/y7k8a2zq

Expected behavior
A drop on "Second Window" would be expected to fire a drop event on 'Second Window", not on "First Window" as it is doing with the regression commit through master.

@reedspool
Copy link

Found the cause:

return self.__fireEvent("dragleave", self.__dropTarget, self.__dragTarget, false, e);

This returns after emitting an event. In the previous version, there was no return, and this.__dropTarget is subsequently updated. I think the solution is to just remove return.

@johnspackman
Copy link
Member

I think this PR will fix your issue; thank you @reedspool, that bit of diagnostics work probably saved me a whole heap of work.

You were right in that the return was wrong because it meant the code was skipping essential stages, the final fix is slightly different in that it's necessary to chain the results together (the events system operates a kind of ultra lightweight promise system)

@oetiker oetiker closed this as completed in 14d51cc Dec 6, 2018
oetiker added a commit that referenced this issue Dec 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants