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

Trigger mouse over on canvas enter #3389

Merged

Conversation

stefanhayden
Copy link
Member

As talked about in #3384 this adds the existing mouse:over event to fire when the mouse enters the canvas.

If the mouse enters the canvas and there is no target the fire the event. We need to do a check if there is a target, other wise the event will fire 2 times on entering the canvas with an object on the edge. once with null and once with the target,

var target = this._hoveredTarget || null;
this.fire('mouse:over', { target: target, e: e });
this._hoveredTarget = null;
target && target.fire('mouseenter', { e: e });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think mouseenter is our mouseover.

But beside that, if you do not find any target, this.findTarget(e) why do you want to fire the event for the last hoverdTarget ?

@asturur
Copy link
Member

asturur commented Oct 31, 2016

Thanks @stefanhayden i left a comment on the code, i m not sure of something.

@stefanhayden
Copy link
Member Author

you were right. So we only fire this event if there is no target. we also set the _hoveredTarget to null or mouse:out reports undefined instead of null for the target.

@asturur asturur merged commit 5915ca8 into fabricjs:master Nov 1, 2016
@asturur
Copy link
Member

asturur commented Nov 1, 2016

nice thanks!

@asturur asturur mentioned this pull request Nov 1, 2016
asturur pushed a commit that referenced this pull request Nov 12, 2016
* fire a mouse:over event once when mouse enters the canvas
* only fire if we don't find a target
* only fire if we find no target and set the hoveredTarget to null
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

Successfully merging this pull request may close these issues.

2 participants