Problem/Motivation

See: #2489460: [Meta] Move module.theme.css files to Classy or Seven

Proposed resolution

Move the CSS file to classy
Create a library for the CSS file
Add the library in the Classy twig template

Remaining tasks

User interface changes

None for Classy, Stark will be more Stark

API changes

None

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because CSS should live in the same place as the markup
Issue priority Not critical
Unfrozen changes Unfrozen because it only changes CSS
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

LewisNyman’s picture

Status: Active » Needs review
FileSize
2.94 KB

Here is an example patch

Manjit.Singh’s picture

FileSize
3.86 KB

removing css file from modules as well :)

lauriii’s picture

Status: Needs review » Needs work
Issue tags: +Needs screenshots
mortendk’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs screenshots
FileSize
2.21 KB
343.66 KB
354.98 KB

Rerolled patch to make it easier to read for review & added screenshots.

Before:

After:

LewisNyman’s picture

Issue summary: View changes

Added beta evaluation. I think we should update the Classy change record to cover all the child issues of #2489460: [Meta] Move module.theme.css files to Classy or Seven

lauriii’s picture

Status: Needs review » Reviewed & tested by the community

Thank you folks for working on this! I tested this manually and there's no visual change on Bartik which is good. The code changes what it should and stays in the scope of the issue and is clean against Drupal coding standards.

davidhernandez’s picture

Shouldn't the ".theme." bit be removed from the CSS file name, since it is redundant now being in the theme?

LewisNyman’s picture

Not in this issue, as we aren't removing the MAT concept until we've completed #2489666: [meta] Split module CSS files into SMACSS component CSS files

davidhernandez’s picture

I know that is the case with the system module CSS, but the book CSS seems fairly self-contained. Does it need breaking up?

LewisNyman’s picture

I'm trying to think back to the discussion we had at Drupalcon. I think the problem is just consistency, it would be better to have remove all references of *.module.css and *.theme.css in one go instead of having some with and some without. If we are being ambitious we could do it issue per issue, the file renames would be quite easy to do in one issue though.

davidhernandez’s picture

I think the problem is just consistency, it would be better to have remove all references of *.module.css and *.theme.css in one go

You're right. I do remember now. But, thinking about it, isn't the inconsistency leaving the .theme? We use it in modules, but none of the themes do and this is being added to a theme. So those file names will be living in Classy until they get changed, if they get changed.

Buuut, I'm satisfied that you're keeping track of it. I just wanted to make sure I understand and that this wasn't an oversight.

mortendk’s picture

lets just get all the files over then lewis can go banana's on renaming em all ;)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

I grepped the code base to ensure all the classes used in the css file are only in classy and it's sub themes - they are. So this is good to go. Committed f6b2354 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed f6b2354 on 8.0.x
    Issue #2489474 by mortendk, LewisNyman, Manjit.Singh, davidhernandez:...

Status: Fixed » Closed (fixed)

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