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

Add option to remove pagination and display all rows. #106

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AWilco
Copy link

@AWilco AWilco commented Nov 29, 2013

Added a settings option to paginate the view (on by default). Setting this to off will draw all rows of the gantt chart, and will remove the paginate navigation options.

You could re-create similar behaviour by setting the itemsPerPage to a high number (however the navigation would remain) however this is a precursor to the ability to pan up and down the list as well as left and right.

@usmonster you seem to be the most active regarding pull requests?

@usmonster
Copy link
Collaborator

@AWilco, thanks for the pull request!

You're right that the only current way to do that is by setting itemsPerPage to some number (>= source.length if you're passing in an array), and then hiding the paging-related navigation controls, e.g. with a CSS rule:

.fn-gantt .navigate .nav-page-back,
.fn-gantt .navigate .nav-page-next,
.fn-gantt .navigate .page-number{
  display: none;
}

But yeah, this feels a little icky/hacky for something that is possibly a fairly common use case. I never actually use the paging feature anyway, so a nice, clean way to remove that concept from the plugin is definitely appreciated. (There is also a pretty big need for cleanup around the navigation rendering code, which is also on the roadmap.. see #85).

At first glance your approach seems straightforward and relatively minimal, though at this point I'm not sure I'd be comfortable making changes to the navigation construction code until The Great Refactoring. I'm also not sure if we'd want a second explicit paging-related option (e.g., paging), or if we'd instead want to have itemsPerPage fill that role. E.g., we could (and probably should) change the default behavior to show all rows if itemsPerPage is not specified or otherwise undefined|null|0|negative|falsey, instead of assigning the current rather arbitrary initial value of 7. Thoughts, @taitems?

I should be able to return to this within the next couple of weeks, so I'll leave it open for discussion. Hopefully by then I will have landed #85 and we can decide what/how to merge. Thanks again for contributing!

@AWilco
Copy link
Author

AWilco commented Dec 3, 2013

You're right, integrating with the current itemsPerPage (<=0 to disable) would be a neater solution. I chose a new variable mainly to make sure I didn't influence any current configurations.

@iamnader
Copy link

I like this option and would also like to see an option that allows for paging, but via ajax. ie so you can set the total numbers of items (so that 1/7 workjs) and fetch the new pages from a server

@usmonster usmonster added this to the n+2 milestone Dec 24, 2014
@reustle
Copy link

reustle commented May 10, 2015

Is "The Great Refactoring" happening at all? Any chance of this getting merged in?

@usmonster
Copy link
Collaborator

@reustle Oh, The Great Refactoring is coming. It's part of the milestone after the current one (which has been stalled a bit on week calculation sync-up with moment.js, but should be resolved soon). Sorry for the stall between versions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants