fix(uiGridAutoResize): Replaced timeout checker with watcher

[tfilo]

In uiGridAutoResize there was cyckle which checked every 250ms for width or height change. I have implemented this same feature but based on watcher. So it shouldn't be affecting the performance of your site or application negatively.

[mportuga]

Can you add unit tests for your change?

[tfilo]

There are some unit test for original autoresize and that pass succesfully. But I will look at that later and I will try to add some meaningful test for this.

[tfilo]

There was already two tests here: src/features/auto-resize-grid/test/auto-resize-grid.spec.js
It was to check if grid is adjusted after grid element width change to greater width and if grid adjust when grid container change to greater width.

I added other test cases, when element or container became narrower. And I added test cases for height change too.

[falsyvalues]

Guys @tfilo, @mportuga creating watcher on $elm[0].clientWidth have a big implication causing multiple layout reflows. 100 rows kills both Chrome and IE 11.

[cybermerlin]

$watch is dirty code
use events or window.setTimeout()

this pull request added more throttles

[tfilo]

@cybermerlin @falsyvalues What if I add some "denounce" functionality into watcher? It will fire on every event but it will resize grid only when you stop resizing window.

https://stackoverflow.com/questions/21088845/can-i-debounce-or-throttle-a-watched-input-in-angularjs-using-lodash

I didn't want to make it worst. On our project it didn't have impact on performance and we get rid of timeout cycle every 250ms. But we don't have so big data.

[falsyvalues]

@tfilo I agree with @cybermerlin $watch is just not a right solution here, even if you add a throttle it will trigger reflows every 250 ms and it still will be wrong for performance. $watch has to be unregistered after few cycles, for example 5 cycles throttled for 200ms then off.

[tfilo]

@falsyvalues If we get back to original source code with timeout cycle set to 250ms. It will be same performance as throttle with 250ms on watcher am I right ?

So after considering all facts and suggestions I can come up with this solution. It will watch for change, if change occure it will unregister watcher, wait 250ms in case you are resizing window, than it will resize grid and register watcher again. From my point of view it will have negative impact on performance same way like original code has. But we get rid of 250ms cycle triggering even when you don't resize.

        var watcher = null;

        var executeResize = function() {
            uiGridCtrl.grid.gridWidth = gridUtil.elementWidth($elm);
            uiGridCtrl.grid.gridHeight = gridUtil.elementHeight($elm);
            setTimeout(function(){
              $scope.$apply(function(){
                uiGridCtrl.grid.refresh();
              });
            },0);
            watcher = registerWatcher(); // after resize register watcher again
        };

        var registerWatcher = function() {
          return $scope.$watchGroup([
            function() {
              return gridUtil.elementWidth($elm);
            },
            function() {
              return gridUtil.elementHeight($elm);
            }], function(newValues, oldValues, scope) {
            if (!angular.equals(newValues, oldValues)) {
              watcher(); // this will unbind watcher
              setTimeout(executeResize, 250); // after 250ms will trigger resize with actual data
            }
          });
        };

        watcher = registerWatcher(); // register for first time

If you agree that is not sufficient can you @mportuga revert this merge to original code? In case you agree that we can use my suggested solution I will make commit with merge request.

[cybermerlin]

1. replace $scope.$apply to $scope.$digest
2. debounce is not bad choice

function _setWH(){
  uiGridCtrl.grid.gridWidth = gridUtil.elementWidth($elm);
  uiGridCtrl.grid.gridHeight = gridUtil.elementHeight($elm);
  //if realy need then        $scope.$digest();
  uiGridCtrl.grid.refresh();
  //if realy need then        $scope.$digest();
}
setTimeout(()=>{
  if (gridUtil.elementWidth($elm) != uiGridCtrl.grid.gridWidth
      ||  gridUtil.elementHeight($elm) != uiGridCtrl.grid.gridHeight){
    _setWH.debounce(250)
  }
}, 250);

But I recommend use
window.on('resize', ...
or
grid.container.on('resize', ...

[tfilo]

@cybermerlin window.on('resize', ... is bad idea because you can change grid container size by hiding side menu for example, and in this case there is nothing like windows resize event because windows size is still same. And with grid.container.on('resize'.. i am not sure if container emit any event on resize. But I am going look at that.

[tfilo]

@falsyvalues I tried today 16 columns with 2000 rows with my first suggested code and I don't have any issue with that in Chrome. How can I simulate this performance issue anyway ?

[tfilo]

I have commited another changes which should improve performance.
#6490

If you have any better solution feel free to replace it. But I am out of options. See comments there.

[falsyvalues]

@tfilo

If we get back to original source code with timeout cycle set to 250ms. It will be same performance as throttle with 250ms on watcher am I right ?

It's not the same, since timeout was called once and $watch is a continuous digest loop.

So after considering all facts and suggestions I can come up with this solution. It will watch for change, if change occure it will unregister watcher, wait 250ms in case you are resizing window, than it will resize grid and register watcher again. From my point of view it will have negative impact on performance same way like original code has. But we get rid of 250ms cycle triggering even when you don't resize.

With $watch you can use throttle function and that is all you can get, so it is not so good solution imho. Best option it to react on event (maybe not window resize, since its a main case for throttle 😄) then get size (reflow triggered) then call refresh on grid.

You can take a look on performance graph that I posted in #6499 and look for reflow triggers.

P.S. I have not chance to review code yet.

[tfilo]

@falsyvalues thank for looking into it. Unfortunately I wasn't successful implementing this feature event based. So my last solution is still based on watcher. With some debounce like function which prevent from resizing grid while you are still resizing window or container. But it did not prevent from fireing watcher on every change. Now there is timeout 400ms on timer. If this will still cause performance problem. I can unbind watcher when timeout set. And bind it on timeout again. So it will prevent watcher to be triggered more often. But it will still based on watcher :(