drupalprojects / kalatheme

Mirror of http://drupal.org/project/kalatheme provided by hubdrop.
http://hubdrop.org/project/kalatheme
GNU General Public License v2.0
22 stars 29 forks source link

Kalatheme _table hook overwrites tablefield header classes #300

Open break9 opened 7 years ago

break9 commented 7 years ago

the core.inc file line 11

function kalatheme_table($variables) {... function overwrites the classes added by the tablefield module.

this means the table header rows aren't sticky. I am not sure if this is by design but it was pretty difficult to track down.

I am assuming I need to write a custom hook to re-insert the table-sticky classes, or am I missing something?

soniktrooth commented 7 years ago

@break9 The first couple of lines of that function are checking if any classes are being passed in and using them if they are there or setting a new array if they're not. In what way does this over write what table field is doing? If table field is adding classes in a preprocess then they should just come in and get added here.

break9 commented 7 years ago

Tablefield or more precisely, drupal function theme_table (sticky-header), turns the table into 2 tables including one that is the sticky header (this table contains only the header rows that will become "sticky" and is hidden at the top of the page waiting to be set to visible once the table header is scrolled beyond top:21

It looks like none of that pre-processing done by tablefield makes it to the rendered output if Kalatheme or more specifically, the kalatheme_table function, is called.

if you remove the kalatheme_table hook, then the table is rendered as expected.

rendered output without kalathem_table:

<table class="sticky-header" style="position: fixed; top: 21px; left: 494px; visibility: hidden;"><thead>
<tr>
  <th class="row_0 col_0" scope="col">R-Rod Ø (inch)</th>
  <th class="row_0 col_1" scope="col">G-Groove Ø (inch)</th>
  <th class="row_0 col_2" scope="col">L-Length, Groove (inch)</th>
  <th class="row_0 col_3" scope="col">S-Shoulder Ø (inch)</th>
  <th class="row_0 col_4" scope="col">0-ring Dash Number</th> 
</tr>
</thead>
</table>

<table id="tablefield-0" class="tablefield sticky-enabled tableheader-processed sticky-table">
<thead>
<tr>
  <th class="row_0 col_0" scope="col">R-Rod Ø (inch)</th>
  <th class="row_0 col_1" scope="col">G-Groove Ø (inch)</th>
  <th class="row_0 col_2" scope="col">L-Length, Groove (inch)</th>
  <th class="row_0 col_3" scope="col">S-Shoulder Ø (inch)</th>
  <th class="row_0 col_4" scope="col">0-ring Dash Number</th> 
</tr>
</thead>
<tbody>
 <tr class="odd">
  <td class="row_1 col_0">+.000/-.002</td>
  <td class="row_1 col_1">+.002/-.000</td>
  <td class="row_1 col_2">+.008/-.000</td>
  <td class="row_1 col_3">+.004/-.000</td>
  <td class="row_1 col_4"></td> 
</tr>
<tr class="even">
...
</tbody>
</table>

rendered table with kalatheme_table:

<table id="tablefield-0" class="tablefield table table-striped table-bordered table-hover">
<thead>
<tr>
  <th class="row_0 col_0" scope="col">R-Rod Ø (inch)</th>
  <th class="row_0 col_1" scope="col">G-Groove Ø (inch)</th>
  <th class="row_0 col_2" scope="col">L-Length, Groove (inch)</th>
  <th class="row_0 col_3" scope="col">S-Shoulder Ø (inch)</th>
  <th class="row_0 col_4" scope="col">0-ring Dash Number</th> 
</tr>
</thead>
<tbody>
 <tr>
  <td class="row_1 col_0">+.000/-.002</td>
  <td class="row_1 col_1">+.002/-.000</td>
  <td class="row_1 col_2">+.008/-.000</td>
  <td class="row_1 col_3">+.004/-.000</td>
  <td class="row_1 col_4"></td> 
</tr>
<tr>
...
</tbody>
</table>
break9 commented 7 years ago

It looks like you are overwriting the drupal theme_table function completely but you haven't included the code to handle sticky headers. I copied this code from the drupal theme_table function and pasted into the kaltheme_table function at line 24 to fix the issue.

Does this sound correct?

  $sticky = $variables['sticky'];
  $empty = $variables['empty'];

  +if (count($header) && $sticky) {
  +  drupal_add_js('misc/tableheader.js');
  +  // Add 'sticky-enabled' class to the table to identify it for JS.
  +  // This is needed to target tables constructed by this function.
  +  $attributes['class'][] = 'sticky-enabled';
  +}

  $output = '<table' . drupal_attributes($attributes) . ">\n";
soniktrooth commented 7 years ago

@break9 You are correct, this is indeed missing from the Kalatheme table implementation.

@pirog Do you recall any valid reason why we weren't supporting sticky headers? Perhaps some sort of clash with Bootstrap or something?

soniktrooth commented 7 years ago

@break9 Have you tested this out? Does it seem to hold together? No broken JS? Feel free to fire up a pull request and I will pull it down and test it.

break9 commented 7 years ago

No issues so far.

pirog commented 7 years ago

@soniktrooth no idea, might have been before bootstrap had sticky functionality

break9 commented 7 years ago

tried to push to a new branch but i am getting a permissions error

soniktrooth commented 7 years ago

Thanks @pirog. @break9 you will need to fork this repo, create a branch on your fork and then roll a PR between that and this.

pirog commented 7 years ago

jesus christ @break9!

break9 commented 7 years ago

Sup @pirog !!!

@soniktrooth done