Content of cells is not re-rendered on data update after scrolling

[bertusvandepoll]

After scrolling the cells based on certain data, I am updating the data on which the cells are rendered. I see that it triggers a watch on which scope.$$updateData is called. However, the content of the cells is not re-rendered, therefore it still shows the old data. When I then scroll, the table is re-rendered and the new data object is shown.

Actually, it seems like on the change of data, it is re-rendering the cells outside the view but not the cells that you are viewing. If I havent used scrolling at all, everything is fine.

I am using the lastest version of ng-cells.

[kayhadrin]

It'd be great if you could make us a demo of this issue on codepen.io, jsfiddle, etc... Otherwise, it'd be pretty hard to reproduce your issue. ;-)

On Fri, 17 Apr 2015 at 22:17 bertusvandepoll notifications@github.com wrote:

After scrolling the cells based on certain data, I am updating the data on which the cells are rendered. I see that it triggers a watch on which scope.$$updateData is called. However, the content of the cells is not re-rendered, therefore it still shows the old data. When I then scroll, the table is re-rendered and the new data object is shown.

Actually, it seems like on the change of data, it is re-rendering the cells outside the view but not the cells that you are viewing. If I havent used scrolling at all, everything is fine.

I am using the lastest version of ng-cells.

— Reply to this email directly or view it on GitHub https://github.com/gdepourtales/ng-cells/issues/29.

[bertusvandepoll]

I have got an example on http://codepen.io/anon/pen/eNOVqE To reproduce:

1. open the link and check that it sgows values in the cells.
2. press "reset" (which sets all the data values to 0), nothing happens
3. Use scroll bar, result is that all values are set to 0. which I would have expected to happen at 2.

Your help is much appreciated, Bertus

On 17 April 2015 at 14:59, David notifications@github.com wrote:

It'd be great if you could make us a demo of this issue on codepen.io, jsfiddle, etc... Otherwise, it'd be pretty hard to reproduce your issue. ;-)

On Fri, 17 Apr 2015 at 22:17 bertusvandepoll notifications@github.com wrote:

After scrolling the cells based on certain data, I am updating the data on which the cells are rendered. I see that it triggers a watch on which scope.$$updateData is called. However, the content of the cells is not re-rendered, therefore it still shows the old data. When I then scroll, the table is re-rendered and the new data object is shown.

Actually, it seems like on the change of data, it is re-rendering the cells outside the view but not the cells that you are viewing. If I havent used scrolling at all, everything is fine.

I am using the lastest version of ng-cells.

— Reply to this email directly or view it on GitHub https://github.com/gdepourtales/ng-cells/issues/29.

— Reply to this email directly or view it on GitHub https://github.com/gdepourtales/ng-cells/issues/29#issuecomment-93980762 .

[kayhadrin]

Here's a modified version of your codepen where the problem is fixed. http://codepen.io/kayhadrin/pen/PqYazy

The ng-cells plugin expects you to update the data object at the outermost level. i.e. in your example, $scope.data must be updated directly. But the reset function you have is only updating the data of the nested arrays, so it won't be detected.

Before:

$scope.reset = function() {
    for (var row = 1; row < 1001; row++) {
        var rowContent = $scope.data[row];
        for (var col = 1; col < 1001; col++) {
            rowContent[col]=0;
        }
    }
}

After:

$scope.reset = function() {
    for (var row = 1; row < 1001; row++) {
        var rowContent = $scope.data[row];
        for (var col = 1; col < 1001; col++) {
            rowContent[col]=0;
        }
    }
    $scope.data = $scope.data.slice(); // equivalent to cloning the array
}

See this StackOverflow post for more information: http://stackoverflow.com/questions/19455501/watch-an-object-in-angular

[kayhadrin]

Having said that, I can see that there are refresh issues when the user had scrolled prior to updating the grid data model.

1. If the user had scrolled vertically, the vertical scroll offset is reset to 0.
   @gdepourtales could you advise whether the vertical scroll reset is expected?
2. If the user had scrolled horizontally, the horizontal scroll offset is not changed, but the grid cells (even header cells) are empty.
   This is most definitely a bug.

You can test it yourself here: http://codepen.io/kayhadrin/pen/PqYazy

[bertusvandepoll]

I guess that whether a vertical scroll reset to 0 is expected depends on the use case. In my case, the reload shows a different group with a possible different number of rows therefore I would like to scroll to the top. In my case, the columns are dates which will remain the same on reload and therefore I would prefer not to scroll to the left.

Anyways, you have a point that angular watches objects not values. The slice() methods fixes it, however, there is something else going on as well: if I add the timeout to the reset method (just like you did for the reload) it will "fix" the bug you last mentioned at http://codepen.io/kayhadrin/pen/PqYazy Moreover, the slice() is not needed anymore. See: http://codepen.io/anon/pen/aOoMOY

Bertus.

On 20 April 2015 at 08:32, David notifications@github.com wrote:

Having said that, I can see that there are refresh issues when the user had scrolled prior to updating the grid data model.

1. If the user had scrolled vertically, the vertical scroll offset is reset to 0. @gdepourtales https://github.com/gdepourtales could you advise whether the vertical scroll reset is expected?
2. If the user had scrolled horizontally, the horizontal scroll offset is not changed, but the grid cells (even header cells) are empty. This is most definitely a bug.

You can test it yourself here: http://codepen.io/kayhadrin/pen/PqYazy

— Reply to this email directly or view it on GitHub https://github.com/gdepourtales/ng-cells/issues/29#issuecomment-94370141 .

[selvach]

Horizontal Scroll Issue

I submitted pull request #32 that fixes the bug @kayhadrin describes:

If the user had scrolled horizontally, the horizontal scroll offset is not changed, but the grid cells (even header cells) are empty. This is most definitely a bug.

Vertical Scroll Issue

The code that causes the vertical scroll offset to reset to 0, is the resizing of the container div for the vertical scroll bar.

The code was added for some use case, so I didn't want to change it without investigating further.

If you were to remove the lines below, it prevents the vertical scroll form resetting when updating data.

table.js, Line 677 - 682

var elem = angular.element(scope.$$verticalScrollbarWrapperElement);
// we need to clear the scrollbar wrapper fixed height,
// otherwise it might cause the table size not to shrink to the minimum height properly
elem.css('height', 'auto');
var height = elem.parent()[0].offsetHeight;
elem.css('height', height + 'px');

[gdepourtales]

Hi all,

Thanks a lot for all suggestion. I have fixed the issue in release 0.3.11. I added 2 test files based on kayhadrin test.

scroll-with-data-changes.html shows the behaviour with changing the data content scroll-with-data-size-changes.html shows the behaviour of the scrollbars when the data array is changed. If you scroll down and far right and then click the reset button, the scrollbars should remain there but the content should be updated at [row=500,col=500].

As suggested by @selvach I also commented out the auto height. @kayhadrin : was it something related to Firefox ?

[kayhadrin]

I think it was done for all browsers rather than just Firefox. AFAIK, the goal was to force the ng-cells table to have a height dependent on its rows contents.

For example, I would have some cells become smaller due to having smaller text strings. So the table looked excessively tall, and that's why I put this "height=auto" change.

// we need to clear the scrollbar wrapper fixed height,
// otherwise it might cause the table size not to shrink to the minimum height properly

elem.css('height', 'auto'); // forces the vertical scrollbar to be resized based on the current contents
var height = elem.parent()[0].offsetHeight; // detect the new height after browser style update
elem.css('height', height + 'px'); // hard-set the height in px

[gdepourtales]

Thanks for your answer @kayhadrin. I see your point. Will try to find some good solution to this issue.

[gdepourtales]

Hi all,

The new version 0.4.0 has been published and it includes 2 examples which show how content is updated after a data update and also scrollbars position and size are updated after a change of the data matrix dimensions. See https://github.com/gdepourtales/ng-cells/blob/master/test/scroll-with-data-changes.html and https://github.com/gdepourtales/ng-cells/blob/master/test/scroll-with-data-size-changes.html for examples

[kayhadrin]

Hi Guy,

I've tried the new version and it didn't work well for my project. Something is causing a problem in the $digest cycle of Angular and it loops for a long time (not infinite!). When it stabilises, a side of the table is totally squished and the ng-scrollable bar does not appear or work.

I haven't got the time to fix this yet, so instead, I've made amendments to the 0.3.x code to fix the previous scrollbar issues. It works much better for me atm.

Feel free to have a look at it on this branch: https://github.com/kayhadrin/ng-cells/commits/fix/vertical-scroll-reset-issue

[gdepourtales]

Hi David

Too bad. Is there a way that I can test your use case and fix the 0.4 for it ? Because I think that the new scrolls are much better.

Thanks

[kayhadrin]

I'll try to create a test case for next week.

On 10:20pm, Fri, 11/09/2015 Guy de Pourtalès notifications@github.com wrote:

Hi David

Too bad. Is there a way that I can test your use case and fix the 0.4 for it ? Because I think that the new scrolls are much better.

Thanks

— Reply to this email directly or view it on GitHub https://github.com/gdepourtales/ng-cells/issues/29#issuecomment-139531953 .

[gdepourtales]

Great that would be helpful. Thanks