Split from #2880237: [meta] Refactor system/base library.
This CSS is only used for item lists (of course) which may not be rendered on every page, leading to unused CSS and unnecessary page weight.
Move item-list to its own library. Then attach it from template_preprocess_item_list
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | 3512285-nr-bot.txt | 91 bytes | needs-review-queue-bot |
| #12 | 3512285-nr-bot.txt | 91 bytes | needs-review-queue-bot |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
catchCSS is added on https://drupal-dev.ddev.site/admin/reports/fields but doesn't actually work, maybe a weight issue?
Comment #4
catchFound the problem - claro wants to extend, not override, the library.
Comment #6
smustgrave commentedFixed a merge conflict but seems performance tests are failing.
Comment #7
catchUpdated the performance tests, but the new layout builder failures are consistent so something is going on here.
Comment #8
catchShould be green now. Looked like very complicated issues in LayoutBuilderUITest but was in fact a typo in the starterkit theme library definition.
Comment #9
borisson_It is green now, and we're decreasing the page weight, that's great.
I am not sure about moving the file though, people are not supposed to reference the file by path, but it seems like it increases the likelihood this will be a breaking change?
Comment #10
catch@borisson_ the
moved_filesdefinition supports libraries_override explicitly - it transparently switches the override and triggers a deprecation error. That leaves two situations where changing the file location can affect things:1. Someone modifying the file in hook_library_info_alter() - but data structures passed to alter hooks are excluded from backwards compatibility for this reason, we'd have the same problem changing the location in a major release too.
2. Someone directly referencing the file in a separate library definition, this feels very, very unlikely because until this MR it was loaded on every request anyway, so it would result in double-loading the same file.
We've also had #3432183: Move system/base component CSS to respective libraries where they exist in core since around 10.2 or 10.3, and not had any regressions reported that I'm aware of (this is one of many follow-ups to that issue to try to finish the meta).
Comment #11
borisson_Thanks for that information. Moving to rtbc based on #10.
Comment #12
needs-review-queue-bot commentedThe 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.
Comment #14
dcam commentedRebased
Comment #15
dcam commentedThis needed a rebase due to other commits that changed stylesheet performance metrics.
Comment #17
longwaveLooking at #3512236: Move fieldgroup CSS to its own library I think we need
moved_filesadding, as per #10?Comment #18
catchYes it did need that - added. Also needed a rebase.
Comment #19
catchComment #20
needs-review-queue-bot commentedThe 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.
Comment #21
catchRebased.
Comment #22
smustgrave commentedBelieve feedback for this one has been addressed
Comment #23
dcam commentedIt needed a rebase due to ongoing changes in the performance tests. I expected the tests to fail, but apparently the new performance metrics from 11.x are still good here.
Comment #24
alexpottWe need a CR and to link to it in the moved files section. Plus the deprecation version needs updating. - can be re rtbc'd once that is done.
Comment #25
dcam commentedI created a new CR for libraries moved in 11.3.0 and made the requested changes. I'm restoring the RTBC status per #24.
Comment #26
alexpottCommitted 5ee49cc and pushed to 11.x. Thanks!