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

mdm’s picture

tjhellmann’s picture

Status: Active » Needs review
FileSize
5.19 KB

Here is a first run at it. I changed the css file name to follow the BAT naming conventions. Also removed id selectors and tried to clean up the rest of the css.

dcmouyard’s picture

Fixed coding style issues and added RTL style sheet.

Status: Needs review » Needs work

The last submitted patch, simpletest-css-cleanup-1217040-4.patch, failed testing.

dcmouyard’s picture

Updated the tests to check for simpletest.admin.css instead of simpletest.css.

dcmouyard’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, simpletest-css-cleanup-1217040-6.patch, failed testing.

dcmouyard’s picture

Status: Needs work » Needs review
FileSize
7.9 KB

I think the tests are fixed now...

sun’s picture

Status: Needs review » Needs work

This needs another re-roll, since the test classes have been moved to new directories.

webchick’s picture

Marked #1663164: Clean up css in simpletest module as a duplicate of this, but it may have more/different info.

droplet’s picture

+++ b/core/modules/simpletest/simpletest.admin-rtl.cssundefined
@@ -0,0 +1,5 @@
+.simpletest-test-label label {

.label class and then label tag ?

+++ b/core/modules/simpletest/simpletest.admin.cssundefined
@@ -0,0 +1,84 @@
+.simpletest-test-form th.select-all {

remove th ?

+++ b/core/modules/simpletest/simpletest.admin.cssundefined
@@ -0,0 +1,84 @@
+th.simpletest_test {

remove th ?

+++ b/core/modules/simpletest/simpletest.admin.cssundefined
@@ -0,0 +1,84 @@
+label.simpletest-group-label {

label and label class ?

+++ b/core/modules/simpletest/simpletest.admin.cssundefined
@@ -0,0 +1,84 @@
+.simpletest-test-label label {

same problem

+++ b/core/modules/simpletest/simpletest.admin.cssundefined
@@ -0,0 +1,84 @@
+tr.simpletest-pass.odd {
+  background-color: #b6ffb6;
+}
+tr.simpletest-pass.even {
+  background-color: #9bff9b;
+}
+tr.simpletest-fail.odd {
+  background-color: #ffc9c9;
+}
+tr.simpletest-fail.even {
+  background-color: #ffacac;
+}
+tr.simpletest-exception.odd {
+  background-color: #f4ea71;
+}
+tr.simpletest-exception.even {
+  background-color: #f5e742;
+}
+tr.simpletest-debug.odd {
+  background-color: #eee;
+}
+tr.simpletest-debug.even {
+  background-color: #fff;

remove tr ?

oresh’s picture

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

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

The last submitted patch, simpletest-css-cleanup-1217040-9.patch, failed testing.

oresh’s picture

Status: Needs work » Needs review
FileSize
2.7 KB

reducing the weight of selectors.
Small css cleanup

Status: Needs review » Needs work

The last submitted patch, 1217040-simpletest_module-css-clean-up-15.patch, failed testing.

droplet’s picture

Status: Needs work » Needs review
FileSize
3.4 KB
+++ b/core/modules/simpletest/simpletest.cssundefined
@@ -1,6 +1,6 @@
+.simpletest-form-table th.select-all {
   width: 1em;
 }

I removed it in favor to http://drupal.org/node/1252206

+++ b/core/modules/simpletest/simpletest.pages.incundefined
@@ -172,7 +172,7 @@ function theme_simpletest_test_table($variables) {
+    return theme('table', array('header' => $header, 'rows' => $rows, 'attributes' => array('id' => 'simpletest-form-table', 'class' => 'simpletest-form-table')));

class is array

and few simple changes:
- simplify .class
- add .simpletest-verbose-message to item-list, don't override all item-list globally.

oresh’s picture

@droplet, thx :)
Can you please add diff to previous patch?
If the test turns green - marking rtbc

droplet’s picture

FileSize
2.5 KB

@oresh,

sure

rteijeiro’s picture

Updated the css files names in the #17 patch.

rteijeiro’s picture

Issue summary: View changes

Updated issue summary.

droplet’s picture

Status: Needs review » Needs work

#20 doesn't include the new theme.css

rteijeiro’s picture

droplet, what do you mean with the "new theme.css"?

I can't find this file in the simpletest module.

droplet’s picture

@rteijeiro,

You make the patch #20...and removed all our changes

rteijeiro’s picture

Status: Needs work » Needs review
FileSize
4.45 KB

The patches #17 and #19 had different changes so I had to review them line by line.

Reviewed all the patches and redone the changes.

I guess this patch is right now.

droplet’s picture

@rteijeiro,

Thanks.

#19 is a interdiff patch, showing the diff between #15 & #17. (For some reason, it lost the "interdiff" text in the patch file name)..Sorry!!

#17 is my final patch.

#24 looks fine :)

Cheers.

rteijeiro’s picture

Re-rolled with the latest changes in 8.x branch.

pakmanlh’s picture

Status: Needs review » Closed (duplicate)

Tested and migrated to the sanbox here #2030419: Clean up the CSS for Simpletest module

pakmanlh’s picture

Issue summary: View changes

Updated summary to the latest CSS organization guidelines.