This is split from http://drupal.org/node/194590 - particularly #70 points 1 and 2. On that long issue, we fixed the most critical problems with tableheader.js (thanks all for great work), but however, a few bugs are still unsolved, my follow-up on that got buried under some other stuff, and finally I was told to better open a new issue. So, here it is ;)
The problems fixed (hopefully) by the attached patch:
1. The new version of tableheader.js does clone the whole table, then remove everything excepting the header, to get a pure header in the end. Unfortunately, the current version fails to remove TFOOT part of the table. I know that current code of theme_table() doesn't render footers, and that 3rd party tables will be out of question once the point 2. below gets fixed, but still TFOOT is a valid HTML construct. We have no good reason to *not* support that, especially that it only costs us one short line in tableheader.js, executed once per page only. It makes the JS more safe, as TFOOT is entirely possible to be introduced by custom theme_table() implementations (or future changes to the default one - it's really sad that we're not supporting one of valid elements now).
2. tableheader.js is processing *all* tables on the page (old version did that, too). This is inconsistent, at the very least: If you have any not-drupal-themed tables, such as hardcoded HTML tables in your content, theme (either tabled layout, or just some kind of footer info and such...), or 3rd party embeds - all these tables will be subjected to stickyness too. Somewhere it may be nice, elsewhere less nice... But the point is that some tables may break, and even worse, this behavior is unreliable: tableheader.js is only added when Drupal itself renders a table with header. So for example, if you have a list of executives as a table in your sidebar, it'll be sticky on "Recent posts" page (Drupal renders another table and adds the JS), but not sticky on frontpage and nodes.
I added a new class "sticky-enabled" to the relevant Drupal-rendered tables, and restricted the JS to only process such marked tables, and not others. If you still like to apply sticky headers to some other tables, you can do so, by just specifying the "sticky-enabled" class (which is simple, and implies that you know what you're doing - you need to also add tableheader.js to the page).
How to test/reproduce: Fresh 6.x-dev install. Open the themes/garland/page.tpl.php file, and paste the test-data.txt file to the bottom, between <!-- /layout --> and print $closure . This gives you a non-drupal table (with all standard parts). Now visit some Drupal page with a table, and see how both drupal- and non-drupal tables are sticky, and how the table footer hangs on the sticky header. The patch should fix that - to see the footer fix in action, add the "sticky-enabled" class to the extra table.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | sticky-tableheader-block-admin.patch | 689 bytes | dvessel |
| #3 | sticky-table-targetting-2b.patch | 2.13 KB | dvessel |
| #2 | sticky-table-targetting-2.patch | 2.03 KB | JirkaRybka |
| test-data.txt | 1.57 KB | JirkaRybka | |
| sticky-table-targetting.patch | 2.49 KB | JirkaRybka |
Comments
Comment #1
JirkaRybka commentedNow that http://drupal.org/node/208179 is committed, the above point 1. is gone entirely. Point 2. is still valid, but I've no time to re-roll today. Going to update the patch as soon as possible.
Comment #2
JirkaRybka commentedUpdated for head. Contains only point 2. from the above - limit tableheader.js only to Drupal-themed tables. (The footer part is no more a problem, as the logic of tableheader.js changed recently.)
Comment #3
dvessel commentedWorks for me. Re-rolled with cleaner comments. No functional changes.
Comment #4
gábor hojtsyCommitted, thanks!
Comment #5
dvessel commentedMinor addition. Forgot about the block configuration page. theme_table was separated into a .tpl file so it didn't affect it there.
Comment #6
JirkaRybka commentedOh, I didn't even know about this one. Good catch, thanks. RTBC indeed.
Comment #7
gábor hojtsyThanks, committed.
Comment #8
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.