Make drupal.tableheader only use CSS for sticky table headers

It's fascinating that `position:sticky` replaces ~450 lines of code.

The Needs Review Queue Bot → tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

Thanks mr. needs-review-queue-bot. MR created.

Don't think this is good to go. You can't remove and deprecate at the same time.

Don't think this is good to go. You can't remove and deprecate at the same time.

I've created an MR which should be better implemented than the patch.
The MR keeps the `drupal.tableheader` library while deprecating, but removes the associated JS files, PHP code & text, and adds the css for sticky position.

Eum, you already removed it? Also D11 is not yet released, so I believe it's possible to remove this in 11.0.0 and add the deprecations to D10.

That's not a bad idea.

Ready for review for 10.3.x

A new patch for 10.2.x:

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

The MR should target 11.x, it will be backported to 10.3.x from there. Couple of extra comments on the MR. This looks great in general, so much code to get rid of.

Binoli Lalani → made their first commit to this issue’s fork.

Small metadata change - putting this into JavaScript because it's mostly about getting rid of JavaScript, might be questionably but close to that than theme system. Also re-titling for the new proposed solution.

Will need an issue summary update to match the new solution as well as a change record.

Making the title easier to read

One comment on the MR - because stable9 is there for backwards compatibility, we should probably add the new class and retain the old class instead or replacing. Otherwise this looks great to me!

Just rerolled patch #15 for D10.2.5.

Patch re-rolled for Drupal 10.2.5

dipakmdhrm → changed the visibility of the branch 3439580-remove-drupal.tableheader-library to hidden.

dipakmdhrm → changed the visibility of the branch 3439580-remove-drupal.tableheader-library to active.

Tests for this should now hopefully pass for 11.x branch after the changes from this issue 📌 Set budgets rather than exact numbers of asset size assertions Downport were merged in the MR branch.

Tested with Drupal 11.x
The header is sticky on scroll(see the attached video).

+1 for RTBC

Reroll of patch in https://www.drupal.org/project/drupal/issues/3439580#comment-15549200 📌 Make drupal.tableheader only use CSS for sticky table headers Needs review for D 10.2.6

This looks good to go, I updated the summary to be clearer.

Tests are passing and the video confirms it is working as expected as well.

The feedback on the MR has been addressed as well so I think it is ready.

Forgot to mention I also reviewed the change record which looks good too!

why the class name change? looks like it wouldn't cause a problem to use the sticky-enabled class to apply the css instead of creating a new one.

The original js adds the sticky-header class, and this is what the existing CSS targets, so it's not actually creating a new class.

this.$stickyTable = $(
          '<table class="sticky-header" style="visibility: hidden; position: fixed; top: 0;"></table>',

core/modules/system/css/components/sticky-header.module.css

table.sticky-header {
  z-index: 500;
  top: 0;
  margin-top: 0;
  background-color: #fff;
}

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

The patch in #31 is retroactively made for a specific version of D10.
The MR's still work for the issue.

hiding patch to prevent the bot from messing with the issue

@nod_ @catch: We're now at beta for D11. Does that mean we now need to adjust the change record & deprecation to target 11.1 & 12 instead of 10.3 & 11?

It will either need to be 11.1 for removal in 12 or 10.4 for removal in 12 depending on whether we backport this to 10.4 or not. I am not entirely sure if we would or not - it might be useful for themes that want to override the component to backport this to keep things in sync, but it's otherwise going to be quite a transparent change for most sites I would think which could be an argument for or against a backport. Will get an opinion or two from other committers.

Any chance to do a d11-beta2 to avoid all this? :P

let's get this in, risk is low

nod_ → closed merge request !7383

nod_ → closed merge request !7381

nod_ → reopened merge request !7381

nod_ → closed merge request !7381

Committed and pushed to 11.x

Wooo wooo yay!!!

fixing issue credits

Hi,

Glad to see JS featured made available with CSS only!

Sorry to arrive after the party, but in https://git.drupalcode.org/project/drupal/-/blob/10.3.x/core/modules/vie...

    $form['sticky'] = [
      '#type' => 'checkbox',
      '#title' => $this->t('Enable Drupal style "sticky" table headers (JavaScript)'),
      '#default_value' => !empty($this->options['sticky']),
      '#description' => $this->t('(Sticky header effects will not be active for preview below, only on live output.)'),
    ];

Should the (JavaScript) be removed too?

@GrimReaper - good find, could you open follow-up issue for fixing that text?

Thanks @catch.

I am on it :)

Follow up issue created: 📌 Remove JavaScript from Views configuration form now that it is only CSS Needs review

MR created and pipeline green, ready for review.

Automatically closed - issue fixed for 2 weeks with no activity.

Hi,

After this change, a lot of people seems to run into this issue

InvalidArgumentException: The callable definition provided "[Drupal\claro\ClaroPreRender,tablePositionSticky]" is not a valid callable. in Drupal\Core\Utility\CallableResolver->getCallableFromDefinition() (line 69 of core/lib/Drupal/Core/Utility/CallableResolver.php).

Is there any solution for it ?

Thank you