erichexter / twitter.bootstrap.mvc

nuget package to make bootstrap easy with mvc4
Apache License 2.0
248 stars 134 forks source link

Paging issue with active page #80

Closed maroquio closed 11 years ago

maroquio commented 11 years ago

The Pager helper function have an error on line 61. The correct code should be:

if (i == (currentPage)) ...

But this is awsome! Regards!

serra commented 11 years ago

Thanks for reporting, I'll check it out.

serra commented 11 years ago

@maroquio - I think there is no error. The IPagedList model uses a zero-based PageIndex, so it was convenient for the currentPage argument of MvcHtmlString Pager to be zero-based too.

I admit that this might be a bit confusing, since the ToPagedList<T> takes a one-based page argument, so that we can immediately pass in the page parameter we get from the request.

I created a small sample project that adds paging to the sample app: 1 | 2 | 3 | 4 | 5

Please let me know if you still think there's a bu there.

maroquio commented 11 years ago

Hi, Serra. While I was testing, I've tried 0-based index using, but I had to do some arithmetics on the GUI to make it work correctly. It was really confusing. I'm not sure if it is a good idea to keep it like that, but if it works well as 0-based index, its everything allright. Anyway, thanks for your answer and for keeping this very useful project.

serra commented 11 years ago

Glad you worked it out, I agree it can be a bit confusing. Feel free to suggest some changes in a pull request. It would indeed be better if the full public api of the paged list implementation used the same base for the page index.

serra commented 11 years ago

@maroquio - could you close this bug report? Thanks.

maroquio commented 11 years ago

Done! Regards.

2013/3/12 Marijn van der Zee notifications@github.com

@maroquio https://github.com/maroquio - could you close this bug report? Thanks.

— Reply to this email directly or view it on GitHubhttps://github.com/erichexter/twitter.bootstrap.mvc/issues/80#issuecomment-14763067 .