Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Pegination float number problem #820

Closed
muhammedea opened this issue Aug 15, 2013 · 6 comments
Closed

Pegination float number problem #820

muhammedea opened this issue Aug 15, 2013 · 6 comments

Comments

@muhammedea
Copy link

In pagination if the num-pages property is float number (for example 10.2) and I click the last page (11) button the last page items don't seem. But if I go to the last page by "next" button manually, they seem. I solve this problem by this function. Because this function doesn't return float.
$scope.PageNumber = function() {
return Math.ceil($scope.items.length / $scope.item_per_page);
}
<pagination num-pages="PageNumber()" ....>

I think, we should be able to use like this:
<pagination num-pages="(items.length / item_per_page) + 1" ....>
or
<pagination num-pages="items.length / item_per_page" ....>

@hallister
Copy link

Actually I think the way you did it is exactly how it should be. num-pages shouldn't be able to take a float value because a page is or isn't, you can't have a partial page. Furthermore, it's usually a bad idea for a reusable directive to assume anything about input. In this case, you want pagination to assume that a float value is always going to need a ceil. I can't imagine a case where you would want to floor it, but that doesn't mean someone can't imagine it.

@pkozlowski-opensource
Copy link
Member

We are venturing into the tricky territory here... The harm was done already to a certain degree as we already do support page numbers as strings... But I agree with @hall5714 - the less magic and simpler components - the better. @bekos what is your take on this one?

@hall5714 thnx so much for helping us out on this project, you are doing awesome job. Would you be interested in becoming part of the core team in the longer run? We need all the helping (and skilled!) hands we can get :-)

@hallister
Copy link

@pkozlowski-opensource Absolutely! This is an awesome project and frankly, I'm amazed you guys keep up as well as you do! Anything I can do to help improve it, I'm all for.

When you say page numbers as strings do you mean as in $scope.numbers = "34" or are you speaking of strings that AnagularJS parses?

@bekos
Copy link
Contributor

bekos commented Aug 15, 2013

I agree that ceiling a number seems pretty magic, and totally less reasonable than supposing that "12" is actually 12.

I think I have seen this discussion before, but I would prefer pagination to support totalItems & itemsPerPage instead of numPages. The Math.ceil($scope.items.length / $scope.item_per_page); is something unavoidable and will save people's time. WDYT?

@hallister
Copy link

@bekos I like that idea much better. Then the ceil is absolutely expected and there's nothing magic about it.

@pkozlowski-opensource
Copy link
Member

@bekos 👍 for changing the interface of the pagination directive the way you've described. I was suggesting this some time ago as well but I think @petebacondarwin was saying that it would limit usability of this directive if people want to use it to move through an arbitrary collection (pages of size 1).

But I still think we should change the interface - this way we can spare people some calculations.

bekos added a commit to bekos/bootstrap that referenced this issue Aug 23, 2013
Closes angular-ui#820.

BREAKING CHANGE: API has undergone some changes in order to be easier to use.
 * `current-page` is replaced from `page`.
 * Number of pages is not defined by `num-pages`, but from `total-items` &
  `items-per-page` instead. If `items-per-page` is missing, default is 10.
 * `num-pages` still exists but is just readonly.

  Before:

  <pagination num-pages="10" ...></pagination>

  After:

  <pagination total-items="100" ...></pagination>
@bekos bekos closed this as completed in e55d906 Aug 24, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants