search

BTMorton / angular2-grid

A drag/drop/resize grid-based plugin directive for angular2
https://bmorton.co.uk/angular/
MIT License
354 stars 159 forks source link

Infinite loop when items have larger sizex than max cols #260

Closed sconix closed 7 years ago

sconix commented 7 years ago

In our project if saved grid item configuration happens to have larger sizex on some item than what is the max cols when items are loaded then the CPU usage goes to maximum and the tab has to be killed.

I will look into this during the weekend and if I can find the cause then I will submit a PR, but thought to make an issue about this if you happen to already know a fix / reason of this.

BTMorton commented 7 years ago

Are you using limit_to_screen or max_cols/auto_resize?

sconix commented 7 years ago

I am using limit_to_screen at the moment. Previously we were using auto_resize and I think it happened also then.

BTMorton commented 7 years ago

ok, thanks. I'll investigate.

What would be the ideal outcome for you? Currently, item sizes are treated as "sacred" and never adjusted, which means that even with limit_to_screen the most that would happen is that the item is pushed over to the first column and will still exceed the screen. Hence the pos.col == 1 check in isWithinBounds that was removed in a PR the other day. Obviously, that's not an ideal solution as it breaks other functionality. Is that acceptable behaviour for you, or would you rather that the grid reduce the size of the item?

Additionally, if you want the item to shrink, would you expect it to regrow when the screen gets bigger?

sconix commented 7 years ago

I would expect it to shrink so that grid always looks good and the programmer does not have to take care of sanitizing the values. But I do understand the problem and that its good to treat the sizes as sacred. In the end we plan to store the settings based on the current max column size so that user can have different layout for every screen size. Ideally I think there would be separate config for what is the current pos/size in the grid and what is the last user modified pos/size. Then grid could always try to set the user pos/size which would be updated on user actions only, but this might be too complex. So maybe having some config parameter how to handle the situation might be the best (i.e. weather tread the values sacred or not).

Btw that PR was from me and now I see what it was there for and I think it could be added back if you want it to behave like that, but at least it needs a change that when user is doing the resize then that pos.col == 1 check should not be done since it allows user to resize first item to be out of bounds.

BTMorton commented 7 years ago

I think there are better ways to handle it, so I'll leave it as-is for now. I'll do a fix for the infinite looping with the current behaviour for now and look to improve it later when I fix the same issue with position

sconix commented 7 years ago

Sounds good thats more than enough for me at this time.

sconix commented 7 years ago

Thanks, I can verify that the issue is fixed!