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

preliminary, but fully working pagination plugin with demo #54

Merged
merged 2 commits into from Apr 16, 2013

Conversation

alonextou
Copy link
Contributor

This will need a bit of re-factoring. I'm not sure why I'm using data-page attributes on the nav links (I could just use the html value number), and I could probably drop or rename a few pageThis and pageThat variables.

Overall it works good though I think.

The main issue is that I'm not sure how to approach tying this with the filter plugin. It works but the results won't update the navigation. They both show and hide rows, so I need someway of knowing how many rows are being displayed after a filter is made, to update my pagination, I think.

Thanks for a great piece of software, I enjoyed and learned a lot from your methodology.

@alonextou
Copy link
Contributor Author

Just FYI: you can use <table class="footable" data-nav="#element"> to specify the <ul id="element"></ul> which is to be your navigation. There are also parameters for paginate: true / false, and increment: any integer..

Let me know if you have any thoughts on modifying this plugin and / or the filter plugin to make them work nice together. Thanks.

@bradvin
Copy link
Member

bradvin commented Apr 16, 2013

I have been playing with it quite a bit today and have already made some changes. This then also resulted in some changes to the sorting plugin which I think were needed in any case. I will merge your code soon

bradvin added a commit that referenced this pull request Apr 16, 2013
@bradvin bradvin merged commit 1d1a024 into fooplugins:master Apr 16, 2013
@bradvin
Copy link
Member

bradvin commented Apr 16, 2013

going to commit my changes soon and then will start on the filtering compatibility

@alonextou
Copy link
Contributor Author

Thanks a lot @bradvin . As for filtering compatibility, my thought so far:

Instead of using $(row).hide();, we add / remove a class "hidden", to make it easy to iterate each row which is displayed / hidden. I could separate the main logic of paginate's "footable_initialized" into a function, and re-paginate the rows with out class "hidden" after each filter and on init. We would need an event to fire AFTER the filtering is all done to call that paginate function.

@alonextou
Copy link
Contributor Author

Well that was actually so much easier than I expected. We don't need to use a class, and we don't need to modify the filter plugin. I got it working great and will have a pull request shortly =D

@bradvin
Copy link
Member

bradvin commented Apr 18, 2013

cool - I have also been refactoring the pagination, sortable and filtering extensions. Going to commit them shortly, so there might be conflict with what you have done

@bradvin
Copy link
Member

bradvin commented Apr 18, 2013

@awc737 I have committed my changes. Pagination now works with sorting and filtering. I added a few demos including:

demo-pagination.htm - which is a standalone pagination demo
demo-sorting-filtering-pagination.htm - all 3 working together

I have noticed there are still some issues when all 3 are used together and will work through them

Just wanted to let you know so that you can get the latest stuff and see if anything else needs to change.

A couple of noteworthy changes I made:

  1. I made all classes have the prefix of footable (to avoid conflicts with other CSS people might include)
  2. I renamed some functions and option names
  3. I get the pagesize from a data attrib data-page-size on the table

@bradvin
Copy link
Member

bradvin commented Apr 18, 2013

@awc737 all issues resolved now - working nicely

@alonextou
Copy link
Contributor Author

Awesome!!!!! After I had it working with filtering, I realized I broke everything for sorting. What you did here was perfect! I think a lot of people will love this.

@6eDesign
Copy link

Hey guys, great work on adding Footable. I don't want to sound too nit-picky and please excuse my ignorance if there would be a better place to put these issues.. however, I've noticed a couple things with the new pagination:

(1) This can be seen in the demos involving filtering, removing the filter (with a button or by deleting the characters in the filter box) seems to break the Footable/pagination...clicking a page link brings it back to life.

(2) Though the Footable seems to work quite well when implemented multiple times on a single page. The pagination, however, seems to be linked to all of the tables on the page while only actually affecting the last occurrence of a Footable. Perhaps there is a workaround to this that I am unsure of..

Thanks guys.

@bradvin
Copy link
Member

bradvin commented Apr 22, 2013

@6eDesign thanks for pointing these out - I will commit fixes soon

@6eDesign
Copy link

That's great news. Thanks.

@bradvin
Copy link
Member

bradvin commented Apr 22, 2013

I have committed a fix for this : 1a32c33

Please test it out on demo-pagination-multiple.htm to see this working with multiple tables on a page

Please let me know if you find any bugs

@6eDesign
Copy link

It's working well so far-will let you know if anything comes up-thanks for the update.

@6eDesign
Copy link

@bradvin - I've noticed that the pagination will break (show all rows) if the table is initially hidden on page load and then displayed. Once again, this seems to fix itself when activating a pagination button. Overall, though, it's working great.

Line 110 of footable.paginate.js seems to be the culprit:
tbody.find('> tr:visible').hide();

Removing 'visible', seems to work:
tbody.find('> tr').hide();

But obviously this is less efficient than how you had it before, hiding rows that are already hidden and such.

@bradvin
Copy link
Member

bradvin commented Apr 24, 2013

@6eDesign good catch! I have made the change 96e2b66

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.

None yet

3 participants