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.
Comment | File | Size | Author |
---|---|---|---|
#13 | 1981024-13-rename-contextual-css-files.patch | 1.25 KB | echoz |
#5 | after.png | 13.66 KB | dcam |
#3 | 1981024-rename-contextual-css-files.patch | 13.79 KB | jchin1968 |
#1 | 1981024-rename-contextual-css-files.patch | 13.44 KB | jchin1968 |
Comments
Comment #1
jchin1968 CreditAttribution: jchin1968 commentedHere's the patch file
Comment #2
jchin1968 CreditAttribution: jchin1968 commentedPlease review above patch
Comment #2.0
jchin1968 CreditAttribution: jchin1968 commentedUpdate issue description
Comment #3
jchin1968 CreditAttribution: jchin1968 commentedNew patch file.
Prefixed css filenames with 'css/' to match new css directory structure for a module
Comment #3.0
jchin1968 CreditAttribution: jchin1968 commentedUpdated description to prefix css files to include "css/" in the filename
Comment #4
Shyamala CreditAttribution: Shyamala commentedTagging
Comment #4.0
Shyamala CreditAttribution: Shyamala commentedAdded steps for testing
Comment #5
dcam CreditAttribution: dcam commented#3 needs work. The three image URLs shown below need to be updated like "../../../misc/edit.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.
Comment #6
dcam CreditAttribution: dcam commentedThis 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.
Comment #7
Shyamala CreditAttribution: Shyamala commentedCreated 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
Comment #8
JohnAlbinSorry 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. :-)
Comment #9
chaz.chumley CreditAttribution: chaz.chumley commentedWorking on this at DrupalCon Sprint
Comment #10
chaz.chumley CreditAttribution: chaz.chumley commentedNo change needed, unassigning for now
Comment #11
echoz CreditAttribution: echoz commentedComment #12
echoz CreditAttribution: echoz commentedComment #13
echoz CreditAttribution: echoz commentedPatch!
Comment #14
JohnAlbinCommitted to the sandbox! Thanks!
Comment #15.0
(not verified) CreditAttribution: commentedAdding a link to the meta issue