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

e is no longer sent with selection:cleared event #6306

Closed
virror opened this issue Apr 30, 2020 · 13 comments
Closed

e is no longer sent with selection:cleared event #6306

virror opened this issue Apr 30, 2020 · 13 comments

Comments

@virror
Copy link
Contributor

virror commented Apr 30, 2020

Version

3.6.3

Im upgrading from fabric 1 -> 3, and e used to be sent with the selection:cleared event but no longer is. Would it be possible to once again include that? Its sent in selection:created so atm it feels a bit unbalanced and my code kind of requires e to be able to get the mouse position when a selection is removed.

@asturur
Copy link
Member

asturur commented May 1, 2020

I would suggest you, upgrade to 4 unless is very hard.
I'll try to verify this bug asap.

@virror
Copy link
Contributor Author

virror commented May 1, 2020

The plan is for sure to upgrade to v4, but since this is a commercial project, it feels a bit dicey to upgrade to a beta version or even i newly released major version : )
The new controls stuff is really tempting though!

@asturur
Copy link
Member

asturur commented May 1, 2020

i know, i often find myself to put my own projects with betas in order to get an idea of the solidity of the current status.
The difference is that i can drive the beta commit after commit and i have a better understanding of what i m changing.

@asturur
Copy link
Member

asturur commented May 1, 2020

To me it seems that the event is there if the selection:cleared is generated by a mouse event.
I see also how there is a possibility of firing the event from deleting the active object.
Could you please replicate your case in a fiddle?

@awehring
Copy link

awehring commented May 1, 2020

@virror:
The selection and deselection logic changed considerably from version 1.x to 2.x.
http://fabricjs.com/v2-breaking-changes-2

When I did the upgrade, I looked especially at the triggered events and documented it here: #5695
To add this to the Fabric.js documentation is still on my todo list.

@virror
Copy link
Contributor Author

virror commented May 4, 2020

This is the case that used to return evt.e before, but no longer does.
Might be totally intentional, then see it as a feature request instead : p

https://jsfiddle.net/1k6njboc/4/

@awehring
Copy link

awehring commented May 4, 2020

I checked your fiddle and added a little bit, to inspect the event behavior closer:

https://jsfiddle.net/awehring/toubzsrL/20/

If the object is de-/selected programmatically, e is undefined (with canvas.setActiveObject(rect),
canvas.discardActiveObject())
If the object is de-/selected with a mouseclick or a selection area, e holds the mouse event.

If the object is removed, the selection:cleared event has no e (as the object:removed event). I consider this as somewhat inconsistent. No e vs. undefined e.

@virror
Copy link
Contributor Author

virror commented May 4, 2020

My biggest issue is that this used to work before, and now it does not work anymore : p
A workaround for me would be if i could get e programmatically somehow, but im not sure thats possible?

@awehring
Copy link

awehring commented May 4, 2020

What does e hold in your case?

I checked my jsfiddle with fabric.js 1.7.22 and it delivers the same (e is missing in selection:cleared event after removing the object).

@virror
Copy link
Contributor Author

virror commented May 4, 2020

Have to check tomorrow when im back at the office : )

@virror
Copy link
Contributor Author

virror commented May 5, 2020

I must be seriously confused : D
This does not work in 1.7.6 either as you say, hmm.
I wonder if it used to work back in the days with an even older version of fabric.

No matter, i guess you can disregard this issue then, is there another way of getting the mouse position then using @canvas.getPointer? Because that one needs e which i dont have?

@awehring
Copy link

awehring commented May 5, 2020

Your application code must be different to the fiddle example.
canvas.remove() is not triggered by a mouse interaction, it sits just somewhere in the program. So it does not have a mouse position.

If youre remove() is triggered by a mouseclick at the canvas, you can catch it with a mouse:up event, or so. I suppose your application does this somehow now.

If you think that it is not a bug, please leave a short note and close this issue.

@virror virror closed this as completed May 5, 2020
@asturur
Copy link
Member

asturur commented May 5, 2020

I was trying to make sense of this, but whatever road i take it brings me to possible more inconsistency.
I always have been of the idea that events should be fired when we are not in control of things, so that we can know what is going on. I m also asking myself why we fire selection:cleared if we have been the one writing the code to clear it with canvas.remove();

Using events as hubs for all the logic of the app as always brought me to dead ends for a series of reasons, one of which is that too many events fires at some point.

I was thinking of adding a source property to the event, but is also true that the event missing kind of speak for itself.

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

No branches or pull requests

3 participants