Compaction doesn't work as expected

[andrewhathaway]

Thanks for submitting an issue to RGL!

Please mark the type of this issue:

• Bug
• Feature Request
• Question

I have a project currently using this package due to the previous versions of RGL not support horizontal compaction.

When using this version, the compaction happens in way in which space is not wasted. Shown below:

When using RGL 0.16.0, the compaction is not as desired, and just lines up against one side. I've also found that is switches between left and right, randomly when state has been updated. Shown below:

I'd prefer it if compaction was just done like this, and blocks fit in to where they could?

Thanks.

[larrydahooster]

I can confirm it. The compacting seems to be broken on vertical compacting too after upgrade to 16.0.0 or it is a breaking change. I haven't played with

compactType: ?('vertical' | 'horizontal') = 'vertical';
preventCollision: ?boolean = false;

parameters yet, but the behaviour with default params seems broken in my opinion.

0.15.3

0.16.0

[larrydahooster]

I could also easily reproduce weird behaviour in the basic demo

[torkelo]

I am so seeing this. Feel strange that items flow up through full width panels

[andrewhathaway]

I just want it to fit nicely together and work out where to position things, making it a nice grid of things, rather than a vertical line!

[torkelo]

So after debugging the behavior @larrydahooster is seeing I found a number of issues.

1. Compaction is not recursive and can easily cause items to flow through other items. Fixed this by making act of moving items back down after compaction recursive (and take into account collisions). Implementation here: https://github.com/grafana/react-grid-layout/blob/grafana/lib/utils.js#L181

2. The second problem was in moveElementAwayFromCollision found two issues here one I think introduced when horizontal compaction was introduced.
   This line https://github.com/STRML/react-grid-layout/blob/master/lib/utils.js#L327 causes fakeItem.y be collidesWith.y - ItemToMove.h which can be very low and cause itemToMove to jump over an intermediate item. If I always set fakeItem.y to itemToMove.y it seems to fix this problem.

I also had to change when moving element away from collision to take into account element height and not just add +1 to y.
https://github.com/grafana/react-grid-layout/blob/grafana/lib/utils.js#L365

With all these changes I no longer see any strange behaviour of panels flowing or jumping through other panels. But unsure yet if I introduced other issues. Can open PR after more testing if the current behaviour (in v0.16.0) is not intended or desired.

[STRML]

Happy to review a PR, I've been aware that movement is sometimes wonky, although something in v0.16 seems to have made it worse.

[torkelo]

PR submitted