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

Preserve order of selected objects in multiple selection #4529

Closed
kika opened this issue Dec 4, 2017 · 16 comments · Fixed by #8665
Closed

Preserve order of selected objects in multiple selection #4529

kika opened this issue Dec 4, 2017 · 16 comments · Fixed by #8665

Comments

@kika
Copy link

kika commented Dec 4, 2017

Feature request:

Currently when user selects multiple objects (holding down Shift key) the code in the library converts single selection to multiple selection when selecting the second object with the following code:

_createGroup: function(target) {
  var objects = this.getObjects(),
      isActiveLower = objects.indexOf(this._activeObject) < objects.indexOf(target),
      groupObjects = isActiveLower
        ? [this._activeObject, target]
        : [target, this._activeObject];
  this._activeObject.isEditing && this._activeObject.exitEditing();
  return new fabric.ActiveSelection(groupObjects, {
    canvas: this
  });
}

As implemented the order of two first items in the multiselection depends on the Z-order of these two first objects. As such, the library prevents application developer from knowing the order in which objects were selected. Such knowledge is important to implement group operations (like align for example) where first selected object is used as a "reference".

https://stackoverflow.com/questions/47614090/preserve-the-order-of-selected-objects

@asturur
Copy link
Member

asturur commented Dec 4, 2017

Well we have many problem about this, we can discuss them

  1. you have 4 objects A - B - C - D. A bottom , D top.

you select first C then B. that means probably C is down and B up.
we have preserveObjectStacking true/false.
For preserveObjectStacking = true the object must be rendered in the same order
For preserveObjectStacking = false the objects must be rendered in which order? i guess now is undefined

Then if we would group them at some point, chosing toGroup we should pickup which order selection or original stack?

@kika
Copy link
Author

kika commented Dec 5, 2017

For my application the z-order is irrelevant. I allow selecting objects only on the same logical "layer" and from the user perspective all such objects are on the same "plane". Of course the z-order is still present (when the objects overlap during move, etc) but I need to hide it from the user as much as possible.
I traced the implementation further and discovered that even if I undo the reorder from the selection:updated then the library still reorders my selection after the render. This reordering happens in _chooseObjectsToRender and only if the preserveObjectStacking is true. I believe that the reordering in the abovequoted _createGroup should also be controlled by preserveObjectStacking and then I can just set it to true and call it a day. Does this make sense?

@asturur
Copy link
Member

asturur commented Dec 5, 2017

so and so.
is true the objects are reordered from _chooseObjectToRender, and that is wrong and can be fixed since is just a shortcut and not a feature.

I'm unsure how many things change and how may apps change as a consequence

@kika
Copy link
Author

kika commented Dec 5, 2017

What about the parallel data structure in activeSelection object that always keeps objects in the order they were selected? The main data structure is _objects array, and we can keep _selectionOrder array (with appropriate getter(s)) with the same references to the same objects, but only used for purposes like mine and never for render, etc. I can volunteer for the PR.

@mertasan
Copy link

mertasan commented Oct 13, 2018

I want to position selected objects according to the last selected object. But I am having trouble because the array list of choice is alphabetical. Can you help me with this? @asturur

@asturur
Copy link
Member

asturur commented Oct 13, 2018

i really can t help each question. You should really make an use case on stackoverflow with everything possible to help people answer

@mertasan
Copy link

You're right. Thank you, though. :)

@asturur
Copy link
Member

asturur commented Oct 13, 2018

you should post back the link question here

@Clem-D
Copy link

Clem-D commented Oct 17, 2018

Hello,
I'm facing the same issue ; I want to draw an arrow between 2 shapes.
This arrow is actually a Line and a Triangle objects. I want the triangle to be oriented to the 2nd object but in my selection the first selected object is always the same...
What about the PR @kika were talking almost a year ago ?

@asturur
Copy link
Member

asturur commented Oct 17, 2018

@Clem-D can you make a fiddle that show what difficulties are you facing?

@Clem-D
Copy link

Clem-D commented Oct 17, 2018

Actually I can't, I'm using fabricJS v2.4.2 and fiddle only goes to 1.7 so I get a strange error if you want to have a look : http://jsfiddle.net/zsthLj6v/2/
Plus the CDN is not accessible https://cdnjs.cloudflare.com/ajax/libs/fabric.js/2.4.2/fabric.min.js

Anyway my issue is not so complicated ; when user select shapes I want to have access the order he selected them.
So far "canvas.getActiveObjects()" doesn't keep this order.

@asturur
Copy link
Member

asturur commented Oct 17, 2018

http://jsfiddle.net/fabricjs/Da7SP

the suggested fiddle is always linked to the master dist folder. You can have a look here. I ll give a look at the cdn meanwhile

@Clem-D
Copy link

Clem-D commented Oct 17, 2018

http://jsfiddle.net/74qav0k8/

Failed to load resource: net::ERR_BLOCKED_BY_CLIENT

I'm sorry I'm very not familiar with JSFiddle

@rikkarv
Copy link

rikkarv commented Dec 6, 2018

https://jsfiddle.net/z5av340j/1/

I have the following case:

I must have 'canvas.preserveObjectStacking: true' and 'perPixelTargetFind: true'.

I have Red square at bottom, Green Circle in middle and Blue triangle at top.

Then i want to create a group that preserve this order.

I cannot use mouse click and drag to create the selection because i may have other objects in the area.

Using shift and click, starting in the most top till bottom, i get the group with the red square at front.

To achieve the result i pretend, i have to click the objects from bottom to top, which in my opinion can be less friendly to the user.

In my opinion toGroup() should always respect the relative order that the involved objects have in canvas.getObjects()

@asturur
Copy link
Member

asturur commented Dec 8, 2018

i think toGroup is taking the objects as they are in the activeSelection and moving them in the group. As they are.
The objects are probably stored in the same order they are clicked.

There is not a solution that fits all.

There are diffeent things at stake in this issue default rendering order with preserveObjectStacking true and default rendering order with preserveObjectStacking false and default rendering order of a group created from a selection.

Now toGroup is a method called once in a while and not each render. We could provide a callback that takes in input the objects of the activeSelection, and the objects of the canvas and let you output an array of objects reordered.
This would be provided in the options if available. or we would make an argument.

For the default rendering order we can make an option eventually if is an easy implementation.
Display in canvas order vs display in selection order.

I do not feel like taking a stance toward a natural order, since i do not think there is one.

@ShaMan123 ShaMan123 linked a pull request Apr 17, 2022 that will close this issue
1 task
@ShaMan123
Copy link
Contributor

Group rewrite makes this pretty straight forward to handle however you like.

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