ActiveSelection does not fire Events on addWithUpdate(), removeWithUpdate()

[awehring]

Version

3.6.1

Test Case

https://jsfiddle.net/awehring/ygxmbt6d/73/

Information about environment

Browser.
Tested with current Edge, FireFox

Bug Description

An ActiveSelection does not fire Events on addWithUpdate(), removeWithUpdate().
I would expect that selection:updated events are fired.

Steps to reproduce

Run the fiddle and inspect the browser console.

Click 3 times the "Add to Selection" button until all 3 objects are selected.
Click 3 times the "Reduce Selection" button until no object is selected any more.

The console reports the fired events.

Expected Behavior

addWithUpdate(), removeWithUpdate() should fire selection:updated events.

Actual Behavior

These two Methods don't fire selection:updated events.

[asturur]

they won't fire it. you made that button and you are in control of it, you do not need an event. On the button handler, run the code you need.

Does it make sense?

[awehring]

Yes, thats possible.
But somewhat inconsistent: canvas.setActiveObject() fires selection:created or selection:updated events (depending if the selection already existed).
canvas.discardActiveObject() fires before:selection:cleared and selection:cleared.

See fiddle above.

[asturur]

well but your button is going to add or remove from selection, and before doing so you know if you are going to modify or create a selection, depending if you already have one. I do not think addWithUpdate or removeWithUpdate should fire events.

i think that the selection is also firing object added and object removed being it part of the group class?

[awehring]

The selection / group does fire events?

What's the name of these events? Never heared of them.

[asturur]

No it doesn't.
I was confused with the collection.mixin. Just canvas has events... that is silly too since users can't really add objects or remove them with embedded actions.

Please if you fill strongly about it, try to prove why you think is necessary. I can be wrong about it.

[awehring]

I appreciate that you are interested in an open discussion.

My use case is a special kind of drawing application, where the user is able to select fabric objects with a button at the user interface. Afterwards (s)he can change some properties like color, stroke, fontStyle, etc.
I need to know which type of objects are selected to enable the appropriate input elements (like a HTML select element for the fontFamily, when an IText-object (or several) are selected). The easiest way is to do this in a selection:created, selection:updated, selection:cleared event handler.

Currently it is already somewhat complicated to select or deselect additional fabric objects programmatically. The necessery methods are different if no, one or more objects are already selected. I use the following code (it is in the linked fiddle too)

switch (canvas.getActiveObjects().length) {

    // fobj holds the fabric object, that should be selected (in addition)
    case 0:
      // Select object
      canvas.setActiveObject(fobj);
      break;

    case 1:
      // Create new Selection
      sel = new fabric.ActiveSelection([canvas.getActiveObjects()[0], fobj], {
        canvas: canvas
      });
      canvas.setActiveObject(sel);
      break;

    default:
      // Add object to existing Selection
      canvas.getActiveObject().addWithUpdate(fobj);
      break;
  }

canvas.setActiveObject() fires the expected selection:created respectively selection:updated events, addWithUpdate() fires no event (I would expect selection:updated).

Of course it is possible to trigger selection:updated after addWithUpdate() in the application. But the behavior of fabricjs appears inconsistent. In addition it requires some logic to populate the event-object with the deselected, selected, target, updated properties. Logic that is already present in fabricjs.

Therefore I think the API would be easier to use - and I think more logical - if addWithUpdate() would trigger selection:updated as well.

Talking about it, I would even suggest, that addWithUpdate() should incorporate the switch logic from above. Than I could be used in any situation, regardless how many objects are already selected. This would not break any existing application.

About the same holds true for the removeWithUpdate() and discardActiveObject() methods. The former does not trigger a selection:updated event.
As well, the consumer of the API has to check if the last object is deselected or not:

    if (canvas.getActiveObjects().length > 1) {
    	// Remove one object from ActiveSelection
      canvas.getActiveObject().removeWithUpdate(fobj);
      
    } else {
    	// Remove last object from ActiveSelection
      canvas.discardActiveObject();
    }

[asturur]

i ll finally be able to read this today!

[ShaMan123]

#7037 #6619 #7241