A question that remained open in #885228: CSS Files are in major need of clean up! is whether we can additionally remove system.messages.css and system.menus.css, as already outlined in the summary in #71:
The two additional files system.menus.css and system.messages.css are no longer needed now. They were introduced in #163785: Themes can't override core & module stylesheets and #193482: Styling status messages in system.css. Back then, that separation possibly made sense. However, we do not understand why you'd ever want to only override styles for messages or menus separately. Either you do custom theming, or you do not. I.e., either you override the entire sytem.theme.css file anyway, or you just expand on it. But you'd never override those two parts only.
One possible reason for wanting them separately might have been base themes. However. Given system.theme.css, base themes are free to decide whether they want to
- redo (override) the entire system.theme.css to achieve a certain base theme look
- override system.theme.css to just change a few details (keeping most of it the same)
- remove system.theem.css entirely (especially after #901062: Regression: Themes can no longer remove stylesheets by overriding them in .info files landed)
- just expand on the default system.theme.css by loading an additional CSS file
Reasons to consider merging system.menus.css and system.messages.css into system.theme.css:
- further aid in front-end performance: Two files less means two less HTTP requests.
- aid in avoiding the IE 31 files limit: Two files less means there is room for two other files. (Actually four files less on RTL sites)
- further aid in decreasing confusion, as that would leave us with simplicity: system.base.css, system.admin.css, system.theme.css.
It was not done as part of this patch, because we didn't want to derail this issue. Merging them would actually be possible in no time. We just need to know whether it's a complete waste of time to even think about it for D7. ;)
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 916424-7.patch | 11.4 KB | jacine |
| #3 | merge_system_messages_css_and_systems_menus_css_into_system_theme_css-916424-3.patch | 11.3 KB | rjgoldsborough |
Comments
Comment #1
jacineYes. I agree this needs to happen, but I'd say this is probably D8 material for now.
Comment #2
jacineTagging
Comment #3
rjgoldsborough commentedHere's a patch that simply appends the menus and messages including the rtl styling to the system.theme.css and system.theme-rtl.css files respectively and deletes the old files. This also removes any addition of the files since we are getting rid of them.
Comment #4
jacineThis looks good to me! Thanks for the patch @rjgoldsborough. :)
Comment #5
catch#3: merge_system_messages_css_and_systems_menus_css_into_system_theme_css-916424-3.patch queued for re-testing.
Comment #7
jacineHere's a re-roll.
Comment #8
rjgoldsborough commentedWhat exactly caused the re-test to fail? It says it was a -p0 but it wasn't. Looking at the details of the test, it looks like it couldn't find any of the system module?
***EDIT***
Perhaps this? :) http://xjm.drupalgardens.com/blog/rerolling-drupal-8-core-patches
Comment #9
bleen commentedseems like a very strait forward patch ... and I agree 100% with Sun & Jacine that the two css files should be merged
Comment #10
catchLooks good. Committed/pushed to 8.x.