fabricjs / fabric.js

Javascript Canvas Library, SVG-to-Canvas (& canvas-to-SVG) Parser
http://fabricjs.com
Other
29.07k stars 3.51k forks source link

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

Closed jasie closed 4 years ago

jasie commented 7 years ago

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 commented 7 years ago

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

asturur commented 7 years ago

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 commented 6 years ago

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 commented 6 years ago

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

solomax commented 6 years ago

I can help with testing :)

alexhuot1 commented 5 years ago

@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 commented 5 years ago

@alexhuot1 It is here: https://github.com/fabricjs/fabric.js/blob/abc1ca299681193d4a24ec87b2e847014f6cadbb/src/mixins/canvas_events.mixin.js#L717

asturur commented 5 years ago

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

alexhuot1 commented 5 years ago

Thanks a lot, very appreciated!

asturur commented 5 years ago

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:

alexhuot1 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

asturur commented 5 years ago

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

jakedowns commented 4 years ago

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

jakedowns commented 4 years ago

got a fix working! :)

Screen Recording 2019-12-03 at 10 02 AM

alexhuot1 commented 4 years ago

@jakedowns Nice work! Hope it gets merge quickly!