-
Notifications
You must be signed in to change notification settings - Fork 65
perPage & defaultPerPage #17
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is clean implementation and should be bug-free. If @thepill can test this out for us on his environment to make sure, as I am unable to right now.
README.md
Outdated
printable => Add printing functionality => true (default) // Whether printable | ||
customButtons => Custom buttons thingy => [ // Array of objects | ||
perPage => Numbers of rows per page => [10, 20, 30, 40, 50] (default) // Results per page | ||
defaultperPage => Default rows per page => 10 (default) // Default results per page, otherwise it will be the first value of perPage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaultperPage
should become defaultPerPage
@@ -255,6 +253,36 @@ | |||
}, | |||
|
|||
computed: { | |||
perPageOptions: function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be very significantly shortened with arrow-functions, and the first few lines can just be
var options = (Array.isArray(this.perPage) && this.perPage) || [10, 20, 30, 40, 50];
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, I edited the readme for the "typo" and used the shorter way for options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I replaced the longer functions with arrow functions. The rest looks good to me and works in my demo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @danilopolani
Fix #16
perPage
will accept an array of values: the one will be the default and then the values are sorted.Otherwise, you can use
defaultPerPage
to set the default value. (Example in the README)