gridstack / gridstack.js

Build interactive dashboards in minutes.
https://gridstackjs.com
MIT License
6.62k stars 1.28k forks source link

Vertical order is not respected when packing nodes, if space has become available after a swap. #2789

Open mvkampen opened 1 week ago

mvkampen commented 1 week ago

Subject of the issue

When attempting to move node 0 underneath node 3 in column 0. It seems to collide with both node 3 AND 4. When packing the nodes, it seems to fit node 4 first not respecting the current vertical order of the nodes. Resulting in an unexpected layout.

Your environment

Steps to reproduce

Drag node 0 down underneath node 3, see how node 4 jumps over node 3. This is because node 1 is not as tall as node 0 and 2. This indicates that a node may take up this space once the dragged node 0 is swapped with node 3 (snapped underneath) You will see that node 4 will jump unexpectedly ahead of node 3

https://jsfiddle.net/9ruhoabm/5/

Expected behavior

I would expect that the order of the nodes top to bottom would remain the same. As the vertical space between node 1 and node 3 is not large enough to fit widget 4. As node 0 is swapped, space has become available. When packing nodes we see that there is space to move node 3 up to the bottom of node 1. Node 4 should retain its position relative to node 3. (underneath, and remain in column 1)

Start situation Screenshot 2024-09-12 at 16 33 35

Resulting situation Screenshot 2024-09-12 at 16 34 52

Expected situation Screenshot 2024-09-12 at 16 34 34

adumesny commented 1 week ago

yep. collision (which I worked in v4 a lot) is incredibly difficult turns out... not looking forward to figure this one out.

mvkampen commented 6 days ago

I get it, after looking into the engine. Seeing 3 functions working recursively with intermittent mouse events is hard debugging. 😅 Appreciate your and others effort in the migration out of jQuery and moving to Typescript.

I just keep adding information to narrow it down:

Currently we are using gridstackjs 7.3.0 in production, where I cannot reproduce this issue. I was trying to debug some other issue there. So we thought to migrate to a newer version, to see if that would have an impact. Then I found this issue.

Seems to be introduced in version 8.

Can reproduce with: 9.5.1 https://jsfiddle.net/qak4t9rz/ 8.4.0 https://jsfiddle.net/qak4t9rz/1/

adumesny commented 3 days ago

Narrowing it down to an exact rev would help diff and see what might be the cause.