Fix BUG: Dragging multiple blocks reverses the order #3547

[lexoyo]

Hello
I'm trying to fix #3547
This is more of an exercise for me to dive in grapesjs code, let me know if I'm loosing my time or if it sounds useful

[lexoyo]

Here is what I did:

• when the user drags multiple elements, reorder the dragged elements to reflect the elements positions in the dom - needs to handle the case where the elements are not in the same container
• when the user drops multiple elements, move them to the right place, i.e. on top of the previously dropped one

I'd appreciate any feedback on the code or tests to make sure I didn't break anything @japo32 ?

Also I did not create any tests because I couldn't find a suitable place, should I @artf ?

Here is how it behaves now:

Uploading 2022-03-29 12-50-56.mp4…

[artf]

Hi @lexoyo thanks for the PR.
I'd say I'd not expect any change in MoveComponent and I'd not rely on DOM for sorting as it might differ from the component tree (the component view can be completely custom).

I think this issue might be solved on the sorter level by relying on the component methods like component.index() and component.parent().

Here what I'd expect from a simple sort by component index.

const toMoveArr = ...;

if (toMoveArr.length > 1) {
  toMoveArr.sort((a, b) => {
    // inverted sort as we append always to the same lastPos
    return b.index() - a.index();
  })
}

toMoveArr.forEach(model => { ...

This obviously doesn't count the sort across different parents which is probably the main reason I always postponed this fix 😅

[lexoyo]

Hi @artf , thx for the quick reply

Hi @lexoyo thanks for the PR. I'd say I'd not expect any change in MoveComponent and I'd not rely on DOM for sorting as it might differ from the component tree (the component view can be completely custom).

It makes sense of course !

I think this issue might be solved on the sorter level by relying on the component methods like component.index() and component.parent().

Ok, very feasable, I did this

Here what I'd expect from a simple sort by component index.

const toMoveArr = ...;

if (toMoveArr.length > 1) {
  toMoveArr.sort((a, b) => {
    // inverted sort as we append always to the same lastPos
    return b.index() - a.index();
  })
}

toMoveArr.forEach(model => { ...

I did it like this, but then the problem is when we drop in the same container, after the elements, they get inverted

This is why this commit is needed
d0b37a0

You can test it here, I am still testing it too
https://lexoyo.me/grapesjs/demo.html

PS: Sorry for the useless mess in some files, due to linter, I'll clean it at one point, the only changes I made are in src/utils/Sorter.js
PPS: I added some unit tests, what do you think?

[artf]

Overall looks to work pretty well 👏

[lexoyo]

@artf I took into account your review

I reverted MoveComponent.js file to its version before I changed it

For your last 2 comments, I think I got it, please confirm. I have tested dropping images, now it works :))

[lexoyo]

also i updated https://lexoyo.me/grapesjs/demo.html

[artf]

Awesome, thank you @lexoyo ❤️