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

no mouseover/mouseout event on objects in groups (mousedown/mouseup exist, though) #4115

Closed
jasie opened this issue Jul 21, 2017 · 18 comments
Closed

Comments

@jasie
Copy link

jasie commented Jul 21, 2017

I have a fabric.Group to which I added a couple of fabric.Rect via group.AddWithUpdate(). (All are displayed correctly in my canvas.)

this event doesn't fire (same with 'mouseout'):

rect.on('mouseover', function () {
    console.log('mouseover');
});

yet this event DOES fire (same with 'mouseup'):

rect.on('mousedown', function () {
    console.log('mousedown');
});

As per http://fabricjs.com/fabric-intro-part-2#events and https://github.com/kangax/fabric.js/wiki/Working-with-events, it should work though:
"For convenience, Fabric takes event system even further, and allows you to attach listeners directly to canvas objects. [...] We're attaching event listeners directly to rectangle and circle instances. Instead of "object:selected", we're using "selected" event."

my group and rect code:

var buttonsGroup = new fabric.Group([], {
    left: 0,
    top: 55,
    selectable: false,
    hoverCursor: 'auto',
    subTargetCheck: true,
    myID: 'group_menubuttons'
});

var menuButton = new fabric.Rect({
    left: buttonPosX,
    top: buttonPosY,
    width: buttonSize.width,
    height: buttonSize.height,
    fill: '#FFF',
    hoverCursor: 'pointer',
    hasControls: false,
    myClass: 'menuButton',
    myID: 'menuButton_' + type
});

buttonsGroup.addWithUpdate(menuButton);

AndreaBogazzi confirmed on Stackoverflow, that "Sadly mouseUp, mouseDown events are for now the only one supported inside groups."

Example for use case: buttons in a button container. the container is a group, to which the buttons are added to. the buttons shall change their appearance on mouseover.

Adding these events would be greatly appreciated.

@asturur
Copy link
Member

asturur commented Jul 21, 2017

It should be possible to add them, at least for first subTarget with little or no effort.

@asturur
Copy link
Member

asturur commented Jul 29, 2017

i will have no time to do it now, if you want to try tocontribute to the project:

in the file canvas.class.js you find this function:

    /**
     * @private
     */
    _fireOverOutEvents: function(target, e) {
      var overOpt, outOpt, hoveredTarget = this._hoveredTarget;
      if (hoveredTarget !== target) {
        overOpt = { e: e, target: target, previousTarget: this._hoveredTarget };
        outOpt = { e: e, target: this._hoveredTarget, nextTarget: target };
        this._hoveredTarget = target;
      }
      if (target) {
        if (hoveredTarget !== target) {
          if (hoveredTarget) {
            this.fire('mouse:out', outOpt);
            hoveredTarget.fire('mouseout', outOpt);
          }
          this.fire('mouse:over', overOpt);
          target.fire('mouseover', overOpt);
        }
      }
      else if (hoveredTarget) {
        this.fire('mouse:out', outOpt);
        hoveredTarget.fire('mouseout', outOpt);
      }
    },

this function is comparing hoveredTarget with target and firing events.
You could also apply the same logic to this.targets ( is an array of sub targets )
if hoveredTarget did not change but this.targets[0] does, you are mouseover/mouseout inside an object probably.

@dearsina
Copy link

I'm just starting to play with FabricJS and I have stumbled upon this issue also. Any update on adding mouseover/out for objects in groups? Anything I can help with?

@asturur
Copy link
Member

asturur commented Oct 26, 2018

this can probably be fixed. When you have 260+ open issues you easily forget of the low hanging fruits in the older pages

@solomax
Copy link

solomax commented Oct 26, 2018

I can help with testing :)

@alexhuot1
Copy link

@asturur
I would like to fix this. We need the mouseover inside a group. So i'm willing to put the time on that.

Could you point me out where in the code i should put the fix. I can't find _fireOverOutEvents in /src/canvas.class.js. (has mentioned earlier)

@solomax
Copy link

solomax commented Mar 23, 2019

@alexhuot1 It is here:

_fireOverOutEvents: function(target, e) {

@asturur
Copy link
Member

asturur commented Mar 23, 2019

Yes i can help. Tomorrow i ll pull out some indication on where to start

@alexhuot1
Copy link

Thanks a lot, very appreciated!

@asturur
Copy link
Member

asturur commented Mar 23, 2019

So @alexhuot1 considering that implement this will require you also to write some boring UT with Qunit, here is my suggestion:

canvas_events.mixinx.js at line 711 you find this

    /**
     * Manage the mouseout, mouseover events for the fabric object on the canvas
     * @param {Fabric.Object} target the target where the target from the mousemove event
     * @param {Event} e Event object fired on mousemove
     * @private
     */
    _fireOverOutEvents: function(target, e) {
      this.fireSyntheticInOutEvents(target, e, {
        targetName: '_hoveredTarget',
        canvasEvtOut: 'mouse:out',
        evtOut: 'mouseout',
        canvasEvtIn: 'mouse:over',
        evtIn: 'mouseover',
      });
    },

this function will fire mouse over and out for target. It takes as argument a target and the value that represent the last time it was overed, called _hoveredTarget

Ideally you go over the array of subTargets and for each of them you fire this function.
You need to add a new argument that will override the targetName property of the object.
like _hoveredSubTarget0, _hoveredSubTarget1

This is the step1.

Step 2 is that you have to verify that when moving inside a group those references to old subtargets get cleaned up or they stay attached to canvas and creates unwanted events and leaks.

This is the starting point, then we need to check what happen in the PR.

If you never contributed to fabric take this in mind:

  • make a fork and make a branch, do not work on yuor master
  • be sure npm install works smooth and that after a npm run build you can run npm run test
  • before committing and pushing remember to do git checkout master dist so you restore the dist folder to its original version, or the PR get messed up.

@alexhuot1
Copy link

Good thanks, I'll be working on it in a couple of weeks. I'll write you back if i need any more help.

@asturur
Copy link
Member

asturur commented Mar 28, 2019

open pr early for review.

comments and refinements will be probably requested, so the sooner you share the pr, the lesser the chance you do too much work in the wrong direction.

@alexhuot1
Copy link

Ok! I'll open the PR once i'll start. We need this feature at my job and i've managed to get me some hours to work on that. That's why i'll start in about a month. Thanks again for you heads up!

@nscozzaro
Copy link

I also need this feature for the same reason as above... changing the appearance of a button on hover.

@asturur
Copy link
Member

asturur commented Apr 27, 2019

When the pr comes out i ll be here to review it fast.

@jakedowns
Copy link
Contributor

any updates on this? I find myself in a situation where it would be very helpful to have this feature.

jakedowns added a commit to jakedowns/fabric.js that referenced this issue Dec 3, 2019
jakedowns added a commit to jakedowns/fabric.js that referenced this issue Dec 3, 2019
…ls _setCursorFromEvent for sub targets

pass linting
@jakedowns
Copy link
Contributor

got a fix working! :)

Screen Recording 2019-12-03 at 10 02 AM

@alexhuot1
Copy link

@jakedowns Nice work! Hope it gets merge quickly!

jakedowns added a commit to jakedowns/fabric.js that referenced this issue Dec 15, 2019
ickata pushed a commit to ickata/fabric.js that referenced this issue Jul 27, 2021
* Fixes fabricjs#4115 - triggers in/out events for sub targets; and calls _setCursorFromEvent for sub targets
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

7 participants