Fixed bug that caused item position sometimes have NaN values

[sconix]

This fixes a bug where in some situations the item top value ended having NaN value and the item jumped to the first row.

[BTMorton]

Can you explain how this fixes that issue? It looks to me like it will just break cascading if c > itemDims.x

[sconix]

You are right that I am not hundred percent sure about the fix, but I do know that it can not break anything since if the loop goes longer then there is undefined values in the lowRow array which makes it anyway not work the way it was intended. Since the lowRow has only as many items as does the itemDims.x so if looping continues after c + i index then the Math.max starts to produce NaN numbers since lowRow[c+i] is undefined. So if the loop is supposed to continue longer than c+i then something is broken in setting the values into lowRow.

So at least in our application the amount of items in itemDim.x is the same as the columns (which is the length used for filling the lowRow and hence if something is added to the index it goes too large and then lowRow[c+i] is undefined which causes problems.

[BTMorton]

lowRow has as many items as _curMaxRow, not itemDims.x. That means that if itemDims.x = 3, and c = 4, if there is an item in column 5 that should cause this to not move up, it would not be caught, since immediately c > itemDims.x. Does that make sense?

I think I get the issue you're trying to solve, and even though I don't quite know how to reproduce it, I have a fix in mind already that should be committed soon.

[sconix]

At least looking at the code the lowRow is filled based on maxCols not Rows and this is the main reason why I can't follow the code :)

If you have a fix in mind I can test it very easily in our app. In my test case I only have two items on the grid one on each row and when I move the item on second row I get the NaN value and the item is moved to the first row even thought it should not be (of course the move only happens becase top does not have proper value so its basically same as 0).

[BTMorton]

Correct, lowRow is the lowest row per column, so it gets initialised by setting all columns to have 1 as it's lowest row. I've changed it to make it a Map, rather than an object and instead of initialising everything to 1, I do an existence check. It's arguably worse in terms of performance, as it's an extra check every time we access the lowRow property, but the whole item grid isn't great from that perspective and I have plans to remove it soon anyway.

[sconix]

That sounds good to me and makes sense.

[BTMorton]

See 51269ee. If you could test it with your setup, it would be appreciated.

[sconix]

Thanks i am back at my work desk after 2 hours I will test it as the first thing and report back.

[sconix]

The latest 2.0.7 version works for this! Thanks for the fast fix!