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

Event before:selection:cleared doesn't receive the actual target #5616

Closed
0ro opened this issue Apr 3, 2019 · 3 comments · Fixed by #5658
Closed

Event before:selection:cleared doesn't receive the actual target #5616

0ro opened this issue Apr 3, 2019 · 3 comments · Fixed by #5658
Labels

Comments

@0ro
Copy link

0ro commented Apr 3, 2019

Version

2.7.0

Test Case

https://jsfiddle.net/DenBogdanov/9t0wdvs5/

Information about environment

browser

Steps to reproduce

Open console in jsfiddle, select two objects and then deselect them

Expected Behavior

Event listener of before:selection:cleared must to receive the ActiveSelection.

Actual Behavior

Event listener of selection:created receives in field target value of ActiveSelection, but event listener of before:selection:cleared receives the first object of ActiveSelection.

@0ro
Copy link
Author

0ro commented Apr 3, 2019

I don't understand why is in code this behavior with currentActives[0]

    discardActiveObject: function (e) {
      var currentActives = this.getActiveObjects();
      if (currentActives.length) {
        this.fire('before:selection:cleared', { target: currentActives[0], e: e });
      }
      this._discardActiveObject(e);
      this._fireSelectionEvents(currentActives, e);
      return this;
    }

I can try to fix it.

@asturur
Copy link
Member

asturur commented Apr 27, 2019

good question. looks wrong to me.
I think the point is that in the moment the deselection happen, the active selection gets destroyed, the objects mutated back, and you are basically working on invalid targets.

The event should be fired with the current active selection, since we are releasing a breaking release i think we should fix this now.

@0ro
Copy link
Author

0ro commented Apr 29, 2019

Great job!

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