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

Pager & Ember's #each loop generated table #638

Closed
mkrzempek opened this issue Jun 3, 2014 · 5 comments
Closed

Pager & Ember's #each loop generated table #638

mkrzempek opened this issue Jun 3, 2014 · 5 comments

Comments

@mkrzempek
Copy link

Pager addon isn't work properly with tables with <script> tags before within section (for example generated in Ember's handlebars {{#each}} loop). So data rows are always hidden.

In hideRows function there is missing tags filtering so I change line:
rows = c.$tbodies.eq(0).children(),
to:
rows = c.$tbodies.eq(0).children('tr'),

Please submit this simple fix or give me a reason why it cannot be implemented that way.

@Mottie
Copy link
Owner

Mottie commented Jun 3, 2014

Hi @mkrzempek!

Will a similar fix to the one mentioned for the thead in #479 help?

The script tags within the tbody are likely not associated with the rows after sorting the table, so I'm not sure if it would be best to just remove them? The fix you mentioned may work for the pager, but other widgets may also have an issue.

@mkrzempek
Copy link
Author

You're right - removing script tags is some kind of workaround.

But problem is more complex. Removing scripts make this table static (Ember's app loose ability to refresh table content for example when new row is added). So it cause to make many workaraunds to make table to work properly.

@Mottie
Copy link
Owner

Mottie commented Jun 3, 2014

Ok, I don't have a problem with making that change. I just wasn't sure if the script tags were required. I'll make the change and it will soon be available in the working branch.

@mkrzempek
Copy link
Author

Thanks a lot. Finally I don't have to make this change every upgrade from bower.

@Mottie
Copy link
Owner

Mottie commented Jun 18, 2014

This change is now available from the master branch in v2.17.2. Thanks for reporting the problem!

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

No branches or pull requests

2 participants