tableheader.js should give sticky-header table the same class as original table

current code

      headerClone = $(headerClone)[0];

      // Store parent table.
      var table = $(this).parent('table')[0];
      headerClone.table = table;
      // Finish initializing header positioning.
      tracker(headerClone);
      // We hid the header to avoid it showing up erroneously on page load;
      // we need to unhide it now so that it will show up when expected.
      $(headerClone).children('thead').show();

      $(table).addClass('sticky-table');

Means sticky_table class is set :)!
Fixed!

#6: tableheader-class.patch queued for re-testing.

+    // Copy classes from originalTable, remove undesired classes, and add
+    // sticky-header. Any other classes added to originalTable by modules will
+    // exists in stickyTable to ensure consistent styling.
+    this.$stickyTable.attr('class', this.originalTable.attr('class'));
+    this.$stickyTable.removeClass('sticky-enabled tableheader-processed sticky-table');
+    this.$stickyTable.addClass('sticky-header');

This looks fragile to me: suppose we're at a table with tabledrag enabled, or tableselect. They both add "-processed" classes, and both wouldn't be desired to be on the sticky table. let alone any contrib module.

Either we need a good whitelist (instead of blacklisting classes), or we need to pass the CSS classes set in php as a setting?

@Kiphaas7,
can you explain more ? i don't get your meaning :)

@Liam,
No hurts I think and it should be.

I can't remember what my actual problem was there, but I think I was complaining about the blacklisting (=removing) of _just_ the classes mentioned in

this.$stickyTable.removeClass('sticky-enabled tableheader-processed sticky-table');

Contrib might do funky stuff to a table or table header as well, which might not be wanted on the sticky header. If the classes which are to be deleted are a javascript inline setting (passed in by php) contrib can overrule/extend it.

… Or contrib should write extra js to manually remove the class itself. However, that smells like a lot of event duplication in the javascript code. Basically I'd prefer if it was something like this

this.$stickyTable.removeClass(some.inline.setting.string.containing.all.the.classes);

Rerolled

Because of missing `$` this patch doesn't work and gives errors in log console.

+++ b/core/misc/tableheader.js
@@ -158,6 +158,13 @@
+      this.$stickyTable.attr('class', this.originalTable.attr('class'));

Should be:

+++ b/core/misc/tableheader.js
@@ -158,6 +158,13 @@
+      this.$stickyTable.attr('class', this.$originalTable.attr('class'));

I will fix #25 and provide a patch in next comment.

...and here it is.

This is clean D8:

<table class="sticky-header" style="visibility: visible; position: fixed; top: 39px; left: 272.515625px; width: 1039px;">...</table>
<table class="views-table views-view-table sticky-enabled cols-7 responsive-enabled tableresponsive-processed table-select-processed tableheader-processed sticky-table">...</table>

and this is D8 + patch #27:

<table class="views-table views-view-table cols-7 responsive-enabled tableresponsive-processed table-select-processed sticky-header" style="visibility: hidden; position: fixed; top: 39px; left: 272.515625px; width: 1039px;">...</table>
<table class="views-table views-view-table sticky-enabled cols-7 responsive-enabled tableresponsive-processed table-select-processed tableheader-processed sticky-table">...</table>

I attached screenshots from chrome inspector as well.

Reviewed, but changed so that the code is chained, which should give a small performance boost.

+++ b/core/misc/tableheader.js
@@ -149,12 +149,18 @@
+        .attr('class', this.$originalTable.attr('class'))

Use addClass for consistency.

Also why do we do document.createElement now? we can just keep that and get rid of the addClass("sticky-header") line.

Here's a patch and interdiff.

Applies, sticky table inherits same classes as main table.

And the comments in #18 - #23 are out of scope, or do these concerns no longer apply?

Did reroll of patch #32 against 8.2.x

Re-rolled against 8.5.x

Add a test and let it go.

- Reroll the patch from #45.

Reroll the patch on 8.6.x

Patch from #53 applies properly on 8.9.6 and works as advertised.

Patch updated to for 9.3.x-dev

Last patch had an indentation error. This one fixes it.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Patch #62 contains changes to tableheader.es6.js file. which is no longer part of 10.1.x
Re-rolled patch for 10.1.x. Please review

This seems like something that can have a test case to show the issue.

I recreated the patch for Drupal 10.2.x

I rerolled the patch for Drupal 11.x

tableheader.js creates a second table which holds the sticky headers

I noticed that for a normal table, there's no duplicate sticky header. Header is made sticky using css.

But for a tableselect table, we have duplicate header.
In this case too, ideally we should be able to make the header sticky using css.

Why do we have duplicate header? Is this remnant from some old issue or do we still need this?

Why do we have duplicate header? Is this remnant from some old issue or do we still need this?

To answer my own question, this was because IE11 didn't support `position: sticky`.
Now that Drupal >= 10 doesn't support IE11, we can use `position:sticky`.

It's already been done in https://www.drupal.org/project/drupal/issues/3362276 for some tables.

Let's do this for tableheader too.

Issue to deprecate drupal.tableheader: https://www.drupal.org/project/drupal/issues/3439580

Closing in favor of https://www.drupal.org/project/drupal/issues/3439580