Fix for bugs when no compaction is on. #766
Conversation
lib/utils.js
Outdated
| @@ -192,6 +192,8 @@ function resolveCompactionCollision( | |||
| const otherItem = layout[i]; | |||
| // Ignore static items | |||
| if (otherItem.static) continue; | |||
| // Don't resolve collision if with itself | |||
| if (otherItem.i === item.i) continue; | |||
There was a problem hiding this comment.
This shouldn't be possible given that we iterate through the list starting past this item. Have you seen it compare with itself before?
There was a problem hiding this comment.
Yes, I have seen it happen. I checked again, and it is happening because itemIndex is being calculated incorrectly. It is trying to use Array.prototype.indexOf() to find the index of an object, which will not work. I have updated the fix to address the root issue instead.
I just noticed a similar issue is happening in collides(). It will never detect that it is checking whether an item is colliding with itself, as === can't be used to check object equality. I've changed it to compare the i key instead.
lib/utils.js
Outdated
| @@ -441,7 +443,8 @@ export function moveElementAwayFromCollision( | |||
| cols: number | |||
| ): Layout { | |||
| const compactH = compactType === "horizontal"; | |||
| const compactV = compactType === "vertical"; | |||
| // Compact vertically if not set to horizontal | |||
| const compactV = compactType !== 'horizontal'; | |||
There was a problem hiding this comment.
Good call, we need to resolve the collision somehow.
brian-eightsolutions
force-pushed
the
fix-for-no-compaction
branch
2 times, most recently
from
34924b8
to
ab90d33
Compare
brian-eightsolutions
force-pushed
the
fix-for-no-compaction
branch
from
7ee2e58
to
e768e47
Compare
|
Thanks! |
|
Hi! Sorry to bring this up again but were these "bugs" solved in a previous version ? I'm seeing the behavior in latest. Thanks! |
Both of these fixes are for when there is no compaction. See #750
For
moveElementAwayFromCollision(), the issue is that if there is no compaction,compactHandcompactVare both false, and no collision is resolved at all. The solution to this is to apply the vertical collision logic ifcompactHis not true ie. in all other instances. This is valid as there are other instances in the code that make the same assumption.For
resolveCompactionCollision(), the code was detecting an item's collision with itself, thus moving everything down too far. This isn't a noticable problem when there is compaction, as it will resolve all the items moving down too far afterwards by compacting them back up, but the logic is still incorrect. When compaction is off though, the tiles get pushed down too far and remain there, leaving empty blank space.Thanks!
Brian