- @xamount opened merge request.
- Status changed to Needs review
almost 2 years ago 1:19pm 15 March 2023 - ๐น๐นTrinidad and Tobago xamount
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.
- ๐น๐นTrinidad and Tobago xamount
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?
- Status changed to Needs work
almost 2 years ago 1:43pm 16 March 2023 - ๐ฌ๐งUnited Kingdom catch
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.
- ๐ฌ๐งUnited Kingdom catch
Now ๐ Move system/base component CSS to respective libraries where they exist Fixed 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.
- ๐ฌ๐งUnited Kingdom catch
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.
- Status changed to Needs review
9 months ago 12:15pm 9 April 2024 - Status changed to Needs work
9 months ago 11:35am 10 April 2024 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.
- Status changed to Needs review
8 months ago 10:15pm 16 April 2024 - ๐ฌ๐งUnited Kingdom catch
This is a solid 2kb reduction in CSS payload for most pages now (uncompressed).
- ๐ฌ๐งUnited Kingdom catch
@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
- Status changed to Needs work
8 months ago 3:04pm 17 April 2024 - ๐ฎ๐ณIndia pradhumanjainOSL
pradhumanjain2311 โ made their first commit to this issueโs fork.
- Status changed to Needs review
8 months ago 9:40am 18 April 2024 - Status changed to Needs work
8 months ago 10:34am 18 April 2024 - ๐ซ๐ทFrance nod_ Lille
With the patch in the current version (using claro):
- item list style missing
/admin/reports/fields
, the new library is never attached anywhere - 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. - no style on status page
/admin/reports/status
- There are image paths inside the CSS files that needs to be updated since the path of the css file changed.
- Textarea resize styles not applied
/admin/structure/types
- 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
- item list style missing
- ๐บ๐ธUnited States kentr
container-inline.module.css
could also be moved, and then attached somewhere in thecontainer
chain.