lorenzofox3 / Smart-Table

Code source of Smart Table module: a table/grid for Angularjs
http://lorenzofox3.github.io/smart-table-website/
1.8k stars 513 forks source link

Possible issue with reinitialising the scope.pages collection in redraw function #737

Closed sannyjacobsson closed 7 years ago

sannyjacobsson commented 7 years ago

Angular version: 1.5.8 Smart table version: 2.1.8

https://github.com/lorenzofox3/Smart-Table/blob/master/src/stPagination.js

ng.module('smart-table')
  .directive('stPagination', ['stConfig', function (stConfig) {
....
        function redraw () {
....
          scope.pages = [];
....

When updating the st-items-by-page value a redraw() is triggered. In the case the new value is the length of the items in the backing collection the scope.pages collection is reinitialised.

It seems to me that we are loosing our referens to the scope.pages collection in the pagination.html template. See https://github.com/lorenzofox3/Smart-Table/blob/master/dist/smart-table.js

ng.module('smart-table', []).run(['$templateCache', function ($templateCache) {
    $templateCache.put('template/smart-table/pagination.html',
.....

If we instead of reinitialising the code>scope.pages collection in the redraw() function we set the length to zero scope.pages.length = 0; we will maintain our references. When changing the value from the length of the backing collection to some other value the pagination will work.

I discovered the issue when adding a "view all" option for a smart table. I tried with -1 to show all, however that caused ctrl.tableState().pagination.numberOfPages to become negative with all kinds of side effects.

I'm new to JavaScript and AngularJS so I may very well have missunderstod the issue.

MrWook commented 7 years ago

Helloo @sannyjacobsson,

When you update st-items-by-page in any kind of way it need to trigger a redraw(). Also when the redraw is triggered it need to destroy the scope.pages, because if you change the items per page the amount of pages change with it. You can see here: ng.module('smart-table') .directive('stPagination', ['stConfig', function (stConfig) { .... function redraw () { .... for (i = start; i < end; i++) { scope.pages.push(i); } .... it get reinitialize.

For Example: If you have 100 items and 20 items per page you have 5 pages. If you change the items per page to 10 you get 10 pages.

So if you want to be at a specific page when you update your st-items-by-page you need to do it yourself that is nothing the SmartTable could do.

To give you a deeper understanding for the scope.pages = []; vs scope.pages.length = 0;:

foo = [] creates a new array and assigns a reference to it to a variable. Any other references are unaffected and still point to the original array.

foo.length = 0 modifies the array itself. If you access it via a different variable, then you still get the modified array.

Read somewhere that the second one creates a new array by destroying all references to the existing array That is backwards. It creates a new array and doesn't destroy other references.

var foo = [1,2,3]; var bar = [1,2,3]; var foo2 = foo; var bar2 = bar; foo = []; bar.length = 0; console.log(foo, bar, foo2, bar2); gives:

[] [] [1, 2, 3] []

Source

If you want to show all items on one site you better change st-items-by-page to a very high number, i think that should work. Or you set it to the amount of items you got. But -1 is a very bad idea there are multiplications in the SmartTable code.