Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements pagination with page numbers #12

Closed
wants to merge 12 commits into from

Conversation

israelss
Copy link

@israelss israelss commented Oct 6, 2016

As requested by @caiquecastro on #11.

Also, included two more options: previous_button_icon and next_button_icon.

When using the pagination without page_numbers prop, the [previous|next]_button_text will have priority over [previous|next]_button_icon. When page_numbers is true, the icons have priority.
But in any case, the user still can override icon or text by passing a value on options.

The default icons are glyphicon-chevron-left and glyphicon-chevron-right.

I wrote some additional tests, but to be honest, until today I didn't have any experience with any kind of automatized tests, so if they are poorly written or if there are need for some more tests, please tell me!

- Added icon support on next/previous buttons
- Added a page_number mode, i.e., with buttons for each page
- Included tests for the added features
Include simple instructions to use the paginator with page number buttons instead of "Page x of y"
@hootlex
Copy link
Owner

hootlex commented Oct 8, 2016

Great job @israelss
Though, I think that the page_number property should be a key within config. Why did you choose this way?

previous_button_icon 👍

@israelss
Copy link
Author

israelss commented Oct 8, 2016

I think that it's simpler and easier to use it that way, but on a second thought I agree that it should be included within config and passed through options, specially because I plan to improve it, and include a max_buttons option, to include an ellipsis button when given too many pages, showing only (max_buttons - 3) + ellipsis button + the two last buttons, aside previous and next buttons.

E.g., with that options:

options: {
  page_numbers: true,
  max_buttons: 7
}

And with 13 pages in total, the result will be something like that (yes, i took that example from Foundation 6 pagination):

captura de tela de 2016-10-08 11 02 25

I will change it to be passed through options and fix a small bug that I just found out 😅
The max_buttons feature will be implemented later, probably next week.

* page_numbers now has to be passed in options. It is false by default.
* Icons always have priority over text, being default with page_numbers, but not without it.
* Fix a small bug where data would always be fetched on button click, even when the page is the same. Now data will only be fetched when the page change.
++TESTS ARE NEEDED++
* Added the ellipsis support when the given number of pages are bigger than the maximum defined by the user
* Added some basic testing for the page numbers
* changed default `max_buttons` from 5 to 7
* Tweaked some tests from utils.spec.js, and removed duplicate tests
** First commit with ellipsis (ae98586)

* Added the ellipsis support when the given number of pages are bigger than the maximum defined by the user

** Included utils.js tests (bba6be1)

** Changes in tests && more (	8b4756a)

* Added some basic testing for the page numbers
* changed default `max_buttons` from 5 to 7
* Tweaked some tests from utils.spec.js, and removed duplicate tests
* Included `ellipses` option (default: true), which can be used to turn on/off the ellipsis buttons creation
* Limited the range which can be used in `max_buttons` option. Now has to be at least 1.
* Added more tests
@israelss
Copy link
Author

Ok, so ellipsis buttons are now available! 😃

A new option was created, max_buttons, and it defaults to 7, i.e., when using page_numbers: true the default maximum number of page buttons will be 7, but using the max_buttons one can define any number to it (minimum recommended 5, minimum enforced 1). When passing a number lower than 1, the default value will be applied.

Another new option is ellipses, which defaults to true, and can be used to indicate if the paginator will use ellipsis buttons when the total of pages is greater than the max_buttons option.

What are your thoughts on these changes, @hootlex?

@nmfzone
Copy link

nmfzone commented Oct 14, 2016

When will this be merged? I think it would be nice. Btw, is this support Vue 2?

@israelss
Copy link
Author

@nmfzone it works fine with Vue 2 👍

* Added the `page_button_text` option, which makes possible to have custom text before the number of page on the buttons. So instead of just ['1', '2', '3', etc] on the buttons, we can now have ['Page 1', 'Page 2', 'Page 3', etc]
* Included some additional tests and refactored some old ones
@israelss
Copy link
Author

One more addition, the page_button_text option. One can pass a string and it will be attached before the number of the page.

@hootlex
Copy link
Owner

hootlex commented Oct 22, 2016

Awesome work @israelss
I will try to review and merge it today.

When it is done, do you plan on updating the documentation?
If you don't, it's okay. I just want to know. :)

@hootlex
Copy link
Owner

hootlex commented Oct 22, 2016

@israelss

  1. I am working on the plugin and I see that some times it generates 2 ellipses buttons.
    What is the reason for this?
  2. The page button prefix is cool!
  3. Don't make any changes on this pr since I've already merged it and upated in a local branch.

@hootlex
Copy link
Owner

hootlex commented Oct 22, 2016

@israelss I think I'm done. Check page-numbers branch and feel free to open a pr there if you want to make any changes.

@hootlex
Copy link
Owner

hootlex commented Oct 29, 2016

@israelss could I have an answer on 1?

@israelss
Copy link
Author

Sorry, I was off the net the last two weeks.

Answering:

When it is done, do you plan on updating the documentation?

I can work on the docs. But I can only start it on next week, is that ok?

I am working on the plugin and I see that some times it generates 2 ellipses buttons. What is the reason for this?

Is that random/unexpected? When it is happening? 😕

The page button prefix is cool!

Thanks! 😃

Check page-numbers branch and feel free to open a pr there if you want to make any changes.

I'll take a closer look tomorrow 👍

@hootlex
Copy link
Owner

hootlex commented Oct 29, 2016

Hey @israelss, thanks for the quick update.

Regarding the ellipses, I dont think it is a bug since you are displaying them intentionally.
My question is, why? I've never seen a paginator with two ellipses.

I can work on the docs. But I can only start it on next week, is that ok?

Of course, it is okay. You can do it whenever you like to. I just need to know if you will do it.

@nmfzone
Copy link

nmfzone commented Nov 20, 2016

Is there any update for this?

@gsuxlzt
Copy link

gsuxlzt commented Sep 14, 2017

Hi. just wondering if this will be implemented? Thanks a lot.

@maqduni
Copy link

maqduni commented Jan 26, 2018

Yyyeah... is it happening? You guys don't seem to have been too active in the past 1.5 years :D. This is a good feature, so if anyone can finish the job it'd be awesome.

@hootlex
Copy link
Owner

hootlex commented Jan 26, 2018

#11

@hootlex hootlex closed this Jan 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants