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.
| 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 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 commentedComment #7
rominronin commentedHi, I'm working with @burkeker on this at drupalcon barcelona (first time sprinters, please be gentle).
Comment #8
burkeker 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 commentedAccidentally included the local .gitignore settings. A revised patch attached. Sorry for the (potential) confusion!
Comment #10
meramo 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 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 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 commentedRTBC as mentioned by Emma above :)
Comment #24
catchCommitted/pushed to 8.1.x, thanks!