Incorrect order of items if a height of one item has been changed manually

[ruslauz]

Subject of the issue

In case if you are changing a height of one item by changing the h property for instance by changing a state in react app you can face an incorrect order of items, i.e. the item standing next to the changed item is moved to the end of the list.

Your environment

• version of gridstack.js - 7.2.3
• OS - MacOs 13.0
• Browser - Chrome Version 108.0.5359.98 (Official Build) (arm64)

I managed to fix it locally by changing the third parameter of the _fixCollisions method on the 671 line of the gridstack-engine.ts file from collide to undefined forcing it to define collisions once again and to start moving items from the last to the first in the list

I can send a PR if it is a bug and not the feature

[adumesny]

hey thanks for filling it and fixing it ! let me reproduce it locally and see your fix, because we do want to re-use the collision node , but maybe not for the next nested level down...

[ruslauz]

Hey, Alain. My solution looks like a hack. When i was debugging the problem i found out that the list of collisions is fixed from the last item to the first, but in the line 658 of the gridstack-engine.ts we take the first one and then pass it to the _fixCollisions method.

First of all I tried to change this line to take the last item in the list of collisions.

Logically this approach should have fixed the problem but due to some unknown logic (i have not debugged) it fixed only the position of the first item and as far as i remember it broke the position of the last. Therefore i decided to change the collision parameter to undefined and that worked for me.

I am sure almost on 100% that there is an another way to fix the problem but this was a quick fix. And you are as a maintainer know better what should have done. By the way the working example from the screenshots above can be found in that commit

[adumesny]

can you post a jsfiddle (or similar) TS example like https://jsfiddle.net/adumesny/jqhkry7g showing the issue (as bug request mention) without React ? then I can add it to the automated tests...

If it works by user resizing but not by update() then we need to make update behave the same (maybe misisng the active item being 'dragged') as loosing the collision and not re-using is not an optimization I want to loose.

[ruslauz]

Please let me know if it is good example - https://jsfiddle.net/sergeifomin/7x9apehq/

[ruslauz]

Will this type of fix save necessary optimizations?

[adumesny]

FYI,calling update() on that one widget instead of reloading entire grid, which is much more efficient, works.
still a bug, but lower priority...
https://jsfiddle.net/adumesny/jau5s473/

Update: this is actually hard to fix the right way (though it was a sorting issue, but it's not) as both cases update() is called on the same item 1, BUT in load() we do batchUpdate() so the collision is different which it shouldn't. not gonna fix this right now...

[ruslauz]

Thank you! The quick question - if we add an additional check before call of update method in the load method will we achieve the same as wee skip nodes which do not need any update and also get an additional optimization?

For instance like this

[adumesny]

that's not the problem (only item 1 does anything) - and there's a lot more to update than just position... any field

[ruslauz]

Yes you are right samePos checks only position