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

fix(): add pointer data to drop event #8156

Merged
merged 1 commit into from Aug 19, 2022
Merged

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Aug 18, 2022

the need for the pointer in a drop event is a must and because mouse:move fires just after, something very weird happens with cached pointers and I get a bad pointer...
Related #8081
I guess I should add for all drag events as well... (in follow up)

@github-actions
Copy link
Contributor

github-actions bot commented Aug 18, 2022

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   82.05 |    74.55 |    84.4 |   80.64 |                                               
 fabric.js |   82.05 |    74.55 |    84.4 |   80.64 | ...,27456,27514,27524-27568,27676,27763,27967 
-----------|---------|----------|---------|---------|-----------------------------------------------

@asturur
Copy link
Member

asturur commented Aug 18, 2022

No issue with this PR, but what actually happens with the pointer?
I m not sure what difference there is between calculating the pointer here in the code or on your event handler.
Did you manage to understand where the difference come from?

@ShaMan123
Copy link
Contributor Author

Not sure but I am guessing what happens is when I call getPointer it might be that a mouse:move event has fired filling the pointer cache and then when I can the method (perhaps is an async handler) it returns something wrong that was cached after the event was fired.
It is a guess...
Some race condition if not this one

@ShaMan123 ShaMan123 merged commit da0f27f into master Aug 19, 2022
@asturur
Copy link
Member

asturur commented Aug 19, 2022

weird, it should be all syncronous here, there shouldn't be time for the mousemove event to start running fabricjs code.

How much was the difference? 1px at most or more?

@ShaMan123
Copy link
Contributor Author

a lot more
Ohh I remember
It happened when I dragged something on to canvas and for some reason the mouse down got cached and wasn't cleared when I dropped

@ShaMan123 ShaMan123 deleted the drop-ev-data-pointer branch September 11, 2022 23:34
frankrousseau pushed a commit to cgwire/fabric.js that referenced this pull request Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants