Problem/Motivation

#1921610: [Meta] Architect our CSS
The BAT (base-admin-theme) file organization we started to do in Drupal 8 was a fantastic idea. See http://drupal.org/node/1089868

It works really well, but its names conflict with the SMACSS categorization we're using in Drupal 8. "base" and "theme" means something else in SMACSS. So we just need to rename them.

Proposed resolution

contextual.base.css becomes css/contextual.module.css.
contextual.toolbar.css becomes css/contextual.toolbar.css
And contextual.theme.css becomes css/contextual.skin.css.

In addition, since our template files are now in a templates sub-directory of a module, we should do the same for the CSS. Note that the toolbar, tour and views modules already do that.

This is part of the CSS standard described at http://drupal.org/node/1887922

Remaining tasks

Test if with this patch the CSS is being added to Drupal from its new location with its new name

User interface changes

none

API changes

The contextual.module's CSS files will have new names.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jchin1968’s picture

Here's the patch file

jchin1968’s picture

Status: Active » Needs review

Please review above patch

jchin1968’s picture

Project: Drupal core » Drupal 8 Mobile Initiative
Issue summary: View changes

Update issue description

jchin1968’s picture

Project: Drupal 8 Mobile Initiative » Drupal core
FileSize
13.79 KB

New patch file.

Prefixed css filenames with 'css/' to match new css directory structure for a module

jchin1968’s picture

Project: Drupal core » Drupal 8 Mobile Initiative
Issue summary: View changes

Updated description to prefix css files to include "css/" in the filename

Shyamala’s picture

Project: Drupal 8 Mobile Initiative » Drupal core
Issue tags: +Novice, +mobile, +d8mux, +d8mux-css-cleanup

Tagging

Shyamala’s picture

Project: Drupal core » Drupal 8 Mobile Initiative
Issue summary: View changes

Added steps for testing

dcam’s picture

Project: Drupal 8 Mobile Initiative » Drupal core
Status: Needs review » Needs work
FileSize
13.66 KB

#3 needs work. The three image URLs shown below need to be updated like "../../../misc/edit.png".

+++ b/core/modules/contextual/css/contextual.skin.cssundefined
@@ -0,0 +1,96 @@
+.contextual .trigger {
+  background-attachment: scroll;
+  background-color: #fff;
+  background-image: url("../../misc/edit.png");
+  background-position: center center;
+  background-repeat: no-repeat;
+  background-size: 16px 16px;
+  border: 1px solid #ddd;
+  border-radius: 13px;
+  box-shadow: 1px 1px 2px rgba(0,0,0,0.3);
+  /* Override the .element-focusable height: auto */
+  height: 28px !important;
+  float: right; /* LTR */
+  margin: 0;
+  overflow: hidden;
+  padding: 0 2px;
+  position: relative;
+  right: 2px; /* LTR */
+  width: 28px;
+  text-indent: -9999px;
+  z-index: 2;
+  cursor: pointer;
+}
+++ b/core/modules/contextual/css/contextual.toolbar.cssundefined
@@ -0,0 +1,37 @@
+.icon-edit:before {
+  background-image: url("../../misc/edit.png");
+}
+.icon-edit:active:before,
+.active.icon-edit:before {
+  background-image: url("../../misc/edit-active.png");
+}

Otherwise, the patch looks good. I didn't find any additional uses of the old file names. The moved CSS files in the /css directory are being added to pages.

after.png

dcam’s picture

This is also going to need a reroll since we're dropping the change from "theme.css" to "skin.css". See http://drupal.org/node/1921610#comment-7366314.

Shyamala’s picture

Status: Needs work » Closed (duplicate)

Created a single issue to rename all css files at: #1987066: Rename files to match CSS file naming convention based on request by webchick to make review easier. Thanks everyone on this issue, looking to your continued participation in the new issue.

Refer: http://drupal.org/node/1921610#comment-7375894

JohnAlbin’s picture

Project: Drupal core » Drupal 8 Mobile Initiative
Version: 8.x-dev »
Component: CSS » CSS architecture
Status: Closed (duplicate) » Needs work

Sorry for the delay in reviewing these patches. My bronchitis flared up and I've been too sick until this week to get back into the issue queue.

Lots of discussions have happened in the interim. We just held a D8 Mobile Initiative meeting on Google+: https://plus.google.com/u/1/events/c0knva4lgh4vot0nun5lbfel9fc where we decided that we could make the CSS re-archicture work move faster by moving the work into a sandbox git repository. Then we could commit lots of little issues to the sandbox and roll larger, more-complete patches into Drupal 8’s issue queue. (per webchick's request)

So you're work is not lost! I'm moving this issue to the Mobile Initiative sandbox. :-)

chaz.chumley’s picture

Assigned: jchin1968 » chaz.chumley

Working on this at DrupalCon Sprint

chaz.chumley’s picture

Assigned: chaz.chumley » Unassigned

No change needed, unassigning for now

echoz’s picture

Assigned: Unassigned » echoz
echoz’s picture

Assigned: echoz » Unassigned
echoz’s picture

Status: Needs work » Needs review
FileSize
1.25 KB

Patch!

JohnAlbin’s picture

Status: Needs review » Fixed

Committed to the sandbox! Thanks!

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

Anonymous’s picture

Issue summary: View changes

Adding a link to the meta issue