grid icon indicating copy to clipboard operation
grid copied to clipboard

Fix resizeGrid for vertical grids with no/bad initial item positions

Open raihle opened this issue 9 years ago • 2 comments

Changes resizeGrid to fetch the current item's position after placing the item, instead of before. findPositionForItem returns the item in horizontal mode (so this works without issue), but a newly created object in vertical mode. This causes the layout algorithm to break if the items had no initial position, or if the initial position had to be disregarded.

raihle avatar Sep 28 '15 13:09 raihle

Can you please provide reproduction steps for the bug you're trying to fix?

findPositionForItem returns the item in horizontal mode (so this works without issue), but a newly created object in vertical mode

What do you mean by a 'newly created object'? Sounds like the fix should be in that method, instead of outside of it. findPositionForItem is a public method so it should work correctly for both orientations.

andrei-picus-hs avatar Sep 29 '15 08:09 andrei-picus-hs

Sorry for the confusion, I mean _getItemPosition, not findPositionForItem. As for reproduction, one of the tests I wrote for vertical grids fails without the fix, as the items are stacked vertically instead of horizontally. If you want to see it, try the following:

  • Remove the pre-set positions in the DEMO fixture.
  • Run the demo (with a horizontal grid, as normal). The items are all out of position since GridList has not laid them out yet, but resize the grid once and they will fall into place.
  • Modify the demo to use a vertical grid (reusing the DEMO fixture is fine)
  • Run the demo. The items are all out of position. After resizing once they will all be vertically stacked, regardless of the number of lanes. Resizing again improves it since the items now have sort-of-valid positions, but still yields the wrong result because the position of each item is based on the pre-resize position of the previous item (instead of its post-resize position, as is the case in horizontal grids).

raihle avatar Sep 29 '15 08:09 raihle