[meta] Refactor system/base library

We could try to work on this during the 8.x life cycle since we could most likely create BC layer for this in Classy.

Wow...I did not realise this was happening after so many years of working on Drupal. I was trying to minimise unnecessary CSS on a site I was working on and I discovered all of this CSS was coming from system/base library which, as you as said, is being attached to every single page.

Here is a temporary work around, if you are are 100% sure that all of this CSS is not needed in your frontend theme:

function YOUTHEME_page_attachments_alter(array &$attachments) {
  if (in_array('system/base', $attachments['#attached']['library'])) {
    $system_base = array_search('system/base', $attachments['#attached']['library']);
    unset($attachments['#attached']['library'][$system_base]);
  }
}

Sorry but I don't have the time these days to write a patch. If I get the time, I will submit it.

Yes, I am agreed with @xamount,

function YOUTHEME_page_attachments_alter(array &$attachments) {
  if (in_array('system/base', $attachments['#attached']['library'])) {
    $system_base = array_search('system/base', $attachments['#attached']['library']);
    unset($attachments['#attached']['library'][$system_base]);
  }
}

I will try to implement this preprocess function in the patch. I will provide the patch soon.
Thanks

Please review this patch.

Fixed CCF for #15.
Please review.

Thanks for the attempt but the patches at #15 and #17 is probably not the way to go. My suggestion at #12 is just a temporary solution.

The better solution is as suggested in the issue description. That is, we need to move all the respective CSS to separate libraries and attach them to their respective templates. Once that is done, then my suggestion at #12 will no longer be needed.

I took a stab at it and refactored most of the css that should be refactored. The remainder of css were not specific to any templates and/or were too broad and used in multiple areas of Drupal.

One important thing I noticed is that some themes (core themes included) are using libraries-override to override some of the css in the system/base library. I don't think it will break anything, but the css it will try to find will not exist.

For e.g. in claro.info.yml:

libraries-override:
  system/base:
    css:
      component:
        css/components/ajax-progress.module.css: css/components/ajax-progress.module.css
        css/components/autocomplete-loading.module.css: css/components/autocomplete-loading.module.css
        css/components/system-status-counter.css: css/components/system-status-counter.css
        css/components/system-status-report-counters.css: css/components/system-status-report-counters.css
        css/components/system-status-report-general-info.css: css/components/system-status-report-general-info.css
        css/components/tabledrag.module.css: css/components/tabledrag.css

In the above code, all of the source system-status-...css will be missing if this MR were to be merged.

What should we do about these kinds of cases?

MR has failures.

From @nod_

Seems like we could move the details css file to the drupal.collapse library too no? it's a pretty big file in claro.
Same for sticky-header.module.css that could go into drupal.tableheader maybe?

Don't think these are already included here, so adding to make sure they get included.

Also bumping priority here because this is a significant front end performance improvement for any sites that hasn't manually excluded these files itself to get the same effect.

Now #3432183: Move system/base component CSS to respective libraries where they exist is in, trying to revive this one.

I've started a new MR because the approach is different. The original approach here used attach from templates, however template overrides would not get the library added so it seemed risky from a bc standpoint. Instead I've moved files between libraries where possible, or if there wasn't an appropriate library (system status et al) into a new library then using #attached in the render element.

Covered in the MR:

A new system status report library attached from the status report render element.

details.module.css moves to core/drupal.collapse

sticky-header.css moves to core/drupal.tableheader

A new core/drupal.item_list library attached from the item_list element.

A new core/drupal.resize library attached from the textarea element.

Starterkit's tabledrag styling hadn't moved in the previous MR, moved that here.

Extra things I found:

Claro's views styling should probably be a libraries-extend of one of the views UI libraries - should be a follow-up IMO mostly because I'm not sure which one is best.

edit: ResolvedLibraryDefinitionsFilesMatchTest is a lifesaver here.

We can re-use the system/base change record we already have as long as this lands in 10.3.x. Tagging for that, and I'll add the suggested text additions to the issue summary once tests are green.

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.

This is a solid 2kb reduction in CSS payload for most pages now (uncompressed). We already cut out ~3.5kb in the other issue. Overall 6/7ths reduction in the size of this library, which is loaded on every page.

I'm all for this, can a CR be written up?

@smustgrave there's an existing CR for the previous issue at https://www.drupal.org/node/3432346

Since this issue is very similar, I was planning to copy the API changes section of this issue over to that CR once this is committed, as long as it lands in 10.3.x

few typos in the library names

With the patch in the current version (using claro, aggregation off):

1. item list style missing /admin/reports/fields, the new library is never attached anywhere
2. tablesort indicator icon missing on claro /admin/content?title=&type=All&status=All&langcode=All&order=status&sort=asc this might need to be a follow-up, looks tricky.
3. no style on status page /admin/reports/status
4. There are image paths inside the CSS files that needs to be updated since the path of the css file changed.
5. Textarea resize styles not applied /admin/structure/types
6. Haven't found a test case for fieldgroup but that won't work either

The 2kb makes sense, there is a bunch of file that should be loaded that are not :D

container-inline.module.css could also be moved, and then attached somewhere in the container chain.

Given the amount of bugs found in #40, and the amount of merge conflicts here after a few months, I think we should split this into individual issues by destination library. Will be easier to verify that way.

Switching to a meta/plan.

First issue needs review: #3511938: Move status report assets to their own library

Next one: #3512194: Move resize CSS into its own library.

Since these all need manual testing and conflict with each other, definitely better to do library-by-library.

#3512236: Move fieldgroup CSS to its own library theoretically won't conflict with #3512194: Move resize CSS into its own library so added that too.

Added #3512262: Move details.module.css to the collapse library because that also shouldn't conflict hopefully.

From the original MR here, this leaves:

Sticky tableheaders - already done in #3439580: Make drupal.tableheader only use CSS for sticky table headers

Tablesort - needs work issue already at #3432244: Split tablesort.module.css out to its own library and attach it via tablesort

item-list - @todo.

That's everything split out to its own issue now.

Of the remaining files:

clearfix.module.css - class is used by multiple core demplates.
align.module.css - used various places in core, but we should remove the views duplicates: #3436855: Remove the views-align-* CSS classes
container-line.module.css - used in lots of places.
hidden.module.css - used a lot
js.module.css - used a lot
nowrap.module.css - can't see where this is used at all, maybe we can deprecate/drop it.
position-container.module.css - one usage in claro, could maybe be factored out.
reset-appearance.module.css - one usage in claro, could be factored out.

Getting close to 1kb of CSS once remaining issues are in, this is down from 7kb originally.

Getting close to 1kb of CSS once remaining issues are in, this is down from 7kb originally.

That is amazing. Nice work!

Opened #3515090: [meta] Olivero page weight improvements for some similar issues in Olivero and added a couple more child issues here.

Different file, same genre: #3565297: Move 'display link' CSS to its own views library.