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

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. ;)

Comments

jacine’s picture

Version: 7.x-dev » 8.x-dev

Yes. I agree this needs to happen, but I'd say this is probably D8 material for now.

jacine’s picture

Issue tags: +html5, +Front end

Tagging

rjgoldsborough’s picture

Status: Active » Needs review
StatusFileSize
new11.3 KB

Here'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.

jacine’s picture

Status: Needs review » Reviewed & tested by the community

This looks good to me! Thanks for the patch @rjgoldsborough. :)

catch’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +html5, +Front end
jacine’s picture

Status: Needs work » Needs review
StatusFileSize
new11.4 KB

Here's a re-roll.

rjgoldsborough’s picture

What 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

bleen’s picture

Status: Needs review » Reviewed & tested by the community

seems like a very strait forward patch ... and I agree 100% with Sun & Jacine that the two css files should be merged

catch’s picture

Status: Reviewed & tested by the community » Fixed

Looks good. Committed/pushed to 8.x.

Status: Fixed » Closed (fixed)
Issue tags: -html5, -Front end

Automatically closed -- issue fixed for 2 weeks with no activity.