Part of the CSS Cleanup: http://drupal.org/node/1089868

Overview of Goals

  1. Make it easy to remove unwanted design assumptions in the theme layer, while maintaining critical functionality (such as functional JavaScript widgets).
  2. Prevent uneeded administrative styles from loading on the front end.
  3. Give modules the ability to include a generic design implementation with their module, without burdening themers.
  4. Make CSS and related markup more efficient and less intrusive to improve the themer experience.

The CSS Clean-up Process

Use the following guidelines when writing patches for the core issues listed below.

  1. Put CSS is in the appropriate file: CSS should be moved to separate files, using the following guidelines extracted from CSS file organization (for Drupal 8):

    CSS files for Drupal modules

    All of a module's styles should be placed in a css/ sub-directory and broken into one or more of the following files:

    module_name.module.css: This file should hold the minimal styles needed to get the module's functionality working. This includes layout, component and state styles. Any needed RTL styling would go in a file named module_name.module-rtl.css.

    module_name.skin.css: This file should hold extra styles to make the module's functionality aesthetically pleasing. This usually just consists of skin styles. Any needed RTL styling would go in a file named module_name.skin-rtl.css.

    module_name.admin.css: This file should hold the minimal styles needed to get the module's admin screens working. This includes layout, component and state styles. On admin screens, the module may choose to load the *.module.css in addition to the *.admin.css file. Any needed RTL styling would go in a file named module_name.admin-rtl.css.

    module_name.admin.skin.css: This file should hold extra styles to make the module's admin screens aesthetically pleasing. This usually just consists of skin styles. Any needed RTL styling would go in a file named module_name.admin.skin-rtl.css.

    Note: Modules should never have any base styles. Drupal core's modules do not have any base styles. Instead Drupal core uses the Normalize.css library augmented with a drupal.base.css library.

    If a module attaches a CSS file to a template file, the CSS file should be named the same as the template file, e.g. the system-plugin-ui-form.html.twig CSS file should be named system-plugin-ui-form.css

  2. Remove Assumptions: Styles that make too many assumptions, introduce superflous margins, padding and add things like font settings are not necessary and don't belong in core module CSS files. In cases where core themes depend on these properties, they should be moved to the CSS stylesheet of the respective theme.
  3. Reduce Selector Specificity: CSS code that resides in modules should be written in a way that's easily overridable in the theme layer. To improve the Themer Experience and make core CSS more efficient, CSS selectors should be made as general and short as possible. For example:
    • Use .style {} over div.style {} where possible.
    • Use .module .style {} over div.module div.somenestedelement .style where possible.
  4. Don't use IDs in selectors: Use of ID's in core CSS selectors requires more specificity in the theme layer, making it harder and more annoying to deal with. It makes achieveing consistency in complex design implementations much harder than it needs to be. We need to stop making life hard for theme developers.
  5. Don't be afraid to change markup: There's lots of overlap between using proper and semantic markup and doing CSS right. If you come across a case where CSS is being applied where using a more semantic elements would solve the problem, then change the markup in your patch to make it right. For more information, see the Drupal 8 Markup Gate rules.
  6. Start with Stark and cross-browser test.
    1. "Design" markup and CSS for the Stark theme.
    2. If applicable, adapt the styles to match the core themes afterward.
    3. Finally, test the changes in all supported browsers and ensure no regressions are introduced.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

opi’s picture

Hi there,
I'd like to help the CSS Cleanup, but I'm not used to contrib to drupal core, so I pick up an easy one: taxonomy module

So here is my first patch ever, splitting taxonomy.css into taxonomy.admin.css and taxonomy.theme.css (also update taxonomy.admin.inc and taxonomy.module css inclusion)

I wonder if we still need the css rule from taxonomy.theme.inc : .taxonomy-term-description { margin: 5px 0 20px; }
because the description is already wrapped up into a <p>, with default browser margins...

droplet’s picture

Status: Active » Needs review
FileSize
2.64 KB

and tr.taxonomy-term-preview is not needed. table bg are that color

jyve’s picture

FileSize
1.66 KB

Just tested the patch in #2. Here's my feedback:

- Totally agree with removing margin on .taxonomy-term-description and thus the entire taxonomy.theme.css.
- As far as I can tell, all css in taxonomy.admin.css can be removed since they are overwrites for things that have not been defined in this stylesheet anyway, which is wrong:

tr.taxonomy-term-divider-top {
  border-bottom: none;
}
tr.taxonomy-term-divider-bottom {
  border-top: 1px dotted #CCC;
}

The new patch attached removed all css altogether.

However, I believe that a proper developer should have a look at taxonomy.admin.inc and take out the taxonomy-specific paging system that adds the 'taxonomy-term-divider-top', 'taxonomy-term-divider-bottom', 'taxonomy-term-preview' classes etc. Default paging functionality should be used here.

xjm’s picture

Status: Needs review » Needs work
Issue tags: +Novice

Tagging as novice for the task of rerolling the Drupal 8.x patch on account of #22336: Move all core Drupal files under a /core folder to improve usability and upgrades.

If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.

jyve’s picture

Status: Needs work » Needs review
FileSize
1.71 KB

Rerolled the patch against the new folder structure.

cosmicdreams’s picture

Looks like what is left is the removal of the paging system mentioned in #3 and allow the default paging system to function.

Jacine’s picture

Issue tags: -Novice

However, I believe that a proper developer should have a look at taxonomy.admin.inc and take out the taxonomy-specific paging system that adds the 'taxonomy-term-divider-top', 'taxonomy-term-divider-bottom', 'taxonomy-term-preview' classes etc. Default paging functionality should be used here.

I want to mark this RTBC just based on the code review and limited testing that I've already done, but I would like to make sure we're not unintentionally screwing something up, and TBH I cannot figure out how to get these classes to appear to test what this even does. I can honestly say I don't think I've ever seen it in action.

@jyve I assume you've figured this out? Can you tell me how I can test this?

ytsurk’s picture

Assigned: Unassigned » ytsurk

here was my confusion ..

ytsurk’s picture

not the man for this ;)

ytsurk’s picture

finally an adapted patch ;)

new verison of the oldest #5 (removing all the css file) - was no longer apply-able, lil outdated ..
plus additionally removed the links and inline classes on the ul for the dropbutton in a term overview. (like on the content overview). the dropbutton was broken.

still open is the pagination (which seems broken right now on d8 (too) - i get all terms only on the first page ..),
#292073: Long hierarchies not displayed on Taxonomy admin page
there's a patch, and even a nice module to solve this issue.
maybe the .js file could be removed too ...

those 4 are still set as classes, but no longer have a css-definition:

  • taxonomy-term-description
  • taxonomy-term-preview
  • taxonomy-term-divider-top
  • taxonomy-term-divider-bottom
Dragan Eror’s picture

I guess that is not enough to remove styles from module, those styles should be moved in every core theme (or at least in Seven).

Remove Assumptions: Styles that make too many assumptions, introduce superflous margins, padding and add things like font settings are not necessary and don't belong in core module CSS files. In cases where core themes depend on these properties, they should be moved to the CSS stylesheet of the respective theme.

ytsurk’s picture

Status: Needs work » Needs review

this style here taxonomy-term-description adds a margin-bottom, and probably should be moved to the themes ..

.taxonomy-term-description {
  margin: 5px 0 20px;
}

i'll create issues there as soon as this is commited.

ytsurk’s picture

@Jacine,

this can be tested by generating taxonomy terms, devel module can do that for you.
then you go to the Structure > Taxonomy > Your Vocabulary and check the list resp. pagination ..

oresh’s picture

Issue tags: -html5, -Front end

Status: Needs review » Needs work
Issue tags: +html5, +Front end

The last submitted patch, taxonomy-taxonomy_css-1217046-10.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

Updated issue summary.

ytsurk’s picture

Status: Needs review » Needs work
FileSize
1016 bytes

uploaded wrong file ..

ytsurk’s picture

ytsurk’s picture

Status: Needs work » Needs review
FileSize
1014 bytes

in my opinion the .css still can be removed.

this will create followup's

all themes reling on the margin etc. need to be informed.

here the removed css:

tr.taxonomy-term-preview {
  background-color: #EEE;
}
tr.taxonomy-term-divider-top {
  border-bottom: none;
}
tr.taxonomy-term-divider-bottom {
  border-top: 1px dotted #CCC;
}
.taxonomy-term-description {
  margin: 5px 0 20px;
}
andypost’s picture

This looks like related to admin.css

index 1f80d52..0000000
--- a/core/modules/taxonomy/taxonomy.css
+++ /dev/nullundefined

Maybe better to move it to bartik or seven?

oriol masjuan’s picture

Assigned: ytsurk » oriol masjuan

Starting to work on this.

oriol masjuan’s picture

Assigned: oriol masjuan » Unassigned
Status: Needs review » Fixed

After checking the patch, we can see that the changes have already been applied to the current version.

This can be closed.

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

Anonymous’s picture

Issue summary: View changes

Updated summary to the latest CSS organization guidelines.