daniel-nagy / md-data-table

Material Design Data Table for Angular Material
MIT License
1.9k stars 519 forks source link

Add 'All' option in pagination dropdown #376

Closed kelleysy closed 8 years ago

kelleysy commented 8 years ago

I am trying to show an 'All' option in the select menu for pagination.

In the md-data-table.js, if I add $scope.$watch('$pagination.limit', function (newValue, oldValue) {

  if (isNaN(newValue)) {
    newValue = parseInt(self.total);
    self.limit = parseInt(self.total);
  } else if(isNaN(oldValue) || newValue === oldValue) {
    return;
  }
  // find closest page from previous min
  self.page = Math.floor(((self.page * oldValue - oldValue) + newValue) / (isPositive(newValue) ? newValue : 1));
  self.onPaginationChange();
});

and in nutritionController.js, I add $scope.limitOptions = [5, 10, 15, 'All'];

the total amount will show up and work.

But, when I actually click 'All', '5' still shows up in the select menu. All rows show up, its just the option text that is wrong. Can you show me how to fix the text in the select menu?

daniel-nagy commented 8 years ago

The problem is you're overwriting the limit variable with a value that is not in your limit options (the total number of items in this case). As a result, the pagination directive is using the first element in your limit options as a placeholder, which is why you see 5.

Here are the changes you can make to get this working,

self.min = function () {
  if(self.limit === 'All') {
    return isPositive(self.total) ? 1 : 0;
  }

  return isPositive(self.total) ? self.page * self.limit - self.limit + 1 : 0;
};

$scope.$watch('$pagination.limit', function (newValue, oldValue) {
  if(newValue === 'All') {
    newValue = parseInt(self.total, 10);
  } else if(isNaN(newValue) || isNaN(oldValue) || newValue === oldValue) {
    return;
  }

  // find closest page from previous min
  self.page = Math.floor(((self.page * oldValue - oldValue) + newValue) / (isPositive(newValue) ? newValue : 1));
  self.onPaginationChange();
});

I think this is a good idea, using meaningful text such as 'All', as limit options. I should find a way to map text to an expression so developers can do more of this kind of thing.

kelleysy commented 8 years ago

I submitted a pull request to add the 'All' Please take a look, thank you!

daniel-nagy commented 8 years ago

Supported in v0.10.8