Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Follow-up to #2395853: Split system.module.css and system.theme.css files into SMACSS style components
After splitting the system.module.css and system.theme.css there are still some references to it in core, particularly in tests. Those need to be changed/removed.
Beta phase evaluation
| Issue category | Task because this is mostly clean up. |
|---|---|
| Issue priority | Not critical. |
| Unfrozen changes | Unfrozen because it only changes CSS and tests. |
| Prioritized changes | This is not a prioritized change for the beta phase. |
| Disruption | This only affects CSS files. There should be little or no disruption. |
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | remove_references_to-2553553-15.patch | 3.96 KB | alvar0hurtad0 |
| #9 | drupal_core-remove_obsolete_css_2553553-9.patch | 3.95 KB | burkeker |
| |||
| #8 | drupal_core-remove_obsolete_css_2553553-8.patch | 4.11 KB | burkeker |
| #4 | drupal_core-remove_obsolete_css_2553553-4.patch | 4.54 KB | meramo |
Comments
Comment #2
davidhernandezComment #3
Wim LeersComment #4
meramo CreditAttribution: meramo as a volunteer and at Bright Solutions GmbH commentedFirst take.
References replaces with existing files in tests, also noticed that apart from system.module.css and system.theme.css, the system.base.css was also removed but it's referenced in tests, so took care of it as well. Just correct me if I'm wrong and I'll redo.
Comment #5
LewisNyman@meramo Hi, thanks for the work. Do you think we could use a file that was split out of system.module.css instead of the other big CSS files? I think they are also going to get split up at some point so it would break tests again at some point. I think
js.module.cssis a good example.Comment #6
burkeker CreditAttribution: burkeker commentedComment #7
rominronin CreditAttribution: rominronin as a volunteer commentedHi, I'm working with @burkeker on this at drupalcon barcelona (first time sprinters, please be gentle).
Comment #8
burkeker CreditAttribution: burkeker as a volunteer commentedWe've
1) changed all the references from system.admin.css to js.module.css;
2) reverted the references to system.base.css
Hope it works!
Comment #9
burkeker CreditAttribution: burkeker as a volunteer commentedAccidentally included the local .gitignore settings. A revised patch attached. Sorry for the (potential) confusion!
Comment #10
meramo CreditAttribution: meramo as a volunteer and at Bright Solutions GmbH commentedApplies very nice against the head, also does what it should according to the issue description. Let's wait for someone else review and set to rtbc if all fine.
Comment #11
LewisNymanTests pass and I'm happy with the direction.
Comment #14
LewisNymanComment #15
alvar0hurtad0Here is the reroll.
Comment #16
alvar0hurtad0I forgot changing the status
Comment #17
j2r CreditAttribution: j2r commentedPatch apply successfully and Searched for any reference of the system.module.css and system.theme.css and its not there.
Moving it to RTBC
Comment #18
star-szrComment #20
Peacog CreditAttribution: Peacog as a volunteer commentedIt seems to me that this test is not doing what it purports to do. I'm not very familiar with the test subsystem, so I may be missing something, but it looks like this is testing that the
stylesheets-removekey in .info.yml successfully prevents a CSS file from being added to the page. However the function names and comment blocks all talk about testing that CSS overrides work properly, which I would take to mean testing thestylesheets-overridekey. If I've misunderstood I think it would be useful to clarify the wording in the comment blocks because it's quite confusing.Comment #21
emma.mariaSetting this back to RTBC as the patch applies and it has been set to RTBC many times already. Also moved to 8.1 version as this issue is a Task.
Comment #22
ChandeepKhosa CreditAttribution: ChandeepKhosa at 2Toucans commentedRTBC as mentioned by Emma above :)
Comment #24
catchCommitted/pushed to 8.1.x, thanks!