Split tablesort.module.css out to its own library and attach it via tablesort

Possible duplicate of 📌 Refactor system/base library Needs work , as some work for tablesort has already been done in that issue.

Good point - I think I opened this before I found that one. Closing.

Actually no. We should split the change out to here since it's tricky.

I've adjusted the tablesort.module.css but I could not figure where is the best place to attach the new library.

Someone please review and guide me here.

catch → changed the visibility of the branch 3432244-split-tablesort.module.css-out to hidden.

@xamount this is a tricky one, I ended up opening a new MR because there's been some related changes in core since your branch was open. With attachments, I wasn't sure either but added a template_preprocess implementation which works and is at least compact.

Should be reviewable/testable now.

So the way I tested this one was by just created a few nodes with A,B,C in front of the title. Then on the content view verified the table sorting worked as expected.

If different types of sorting are needed let me know, else believe this is good.

Paths needs to be updated after css file move

Updated the icon paths, good spot, not sure how I missed it in manual testing, maybe claro overrides hid the bug?

Yes I don't think any core theme uses this file

Feedback appears to be addressed.

Oops my mistake.

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.

Rebased.

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.

Rebased

nod_ → closed merge request !11445

Committed 4166328 and pushed to 11.x. Thanks!

There's a regression with this change:

Working on it, MR shortly.

OK the problem is that when the tablesort CSS is moved to the library, Claro's classy tablesort.css, which specifies a td.is-active background color, is loaded after Claro's tables.css, which specifies background: none;

Adding an MR that just removes the declaration from classy.css - it makes no sense for Claro to define two different CSS rules for the same class, the classy one was just never getting used until the regression.

More generally, Claro has tablesort.css and tables.css, but defines various styles for tablesort inside tables.css. So it would be worth a follow-up to move all of that styling to tablesort.css to avoid unused CSS when regular, unsortable, tables are rendered.

Opened 📌 Clean up Claro's tables/tablesort/tableselect CSS Active .

Explanation is great, fix is simple, there is one test that looks like a random failure I cannot rerun.

If that passes I think this is rtbc.

Doubt the change didn’t break the build step