fix(gridList): reorder tiles when ngRepeat array changes #2553
Conversation
if the index of any tile changes then all tiles are rearranged by the gridList this way elements that happen to stay at the same index of the array are also rearranged (which was not the case before and caused weird sorting errors) - demo included Fixes #1663
rename functions and format code correctly
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
signed the CLA |
|
CLAs look good, thanks! |
|
@shyndman - Do you have time to review this PR. I approved let me know and I will merge. |
|
Yeah, I'll be able to take a look over the weekend. |
| if (newIdx === oldIdx) { | ||
| return; | ||
| } | ||
| gridCtrl.reorderTiles(); |
There was a problem hiding this comment.
Take the case where someone prepends a tile to the grid -- each of the tiles that already exists in the grid will have this watch trigger, and reorderTiles will be run as many times as there are tiles. This seems like a lot of work just to add a tile.
There was a problem hiding this comment.
I'll play around with a couple things. I want to roll in a fix for #2225, since it's related.
There was a problem hiding this comment.
yes, it is computation intensive, I didn't have time to go deeper into the codebase at the time, will probably try to find a better solution today, thanks for looking at it :)
ajoslin
added
the
in progress
ajoslin
removed
the
in progress
when I tried sorting an array bound to md-grid-list ngRepeat we found that items that didn't change place were displayed incorrectly - this here fixes it (created a demo to prove it - before fix the 4th item would be displayed on 2nd from beginning or end of the md-grid-list after sorting the underlying array)