This is part of #1921610: [Meta] Architect our CSS
The comment module does not follow the guidelines in #1089868: CSS Cleanup
comment.theme.css becomes css/comment.skin.css
comment.theme-rtl.css becomes css/comment.skin-rtl.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
After this issue's completion, we still have all the other steps outlined at #1921610: [Meta] Architect our CSS.
none
The system.module's CSS files will have new names.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 1981026-comment-css-5.patch | 1.28 KB | nick_schuch |
| #2 | Screen Shot 2013-04-27 at 5.05.38 PM.png | 19.79 KB | johnalbin |
| #1 | 1981026-comment-css-1.patch | 2.31 KB | kim.pepper |
Comments
Comment #0.0
kim.pepperAdded problem
Comment #1
kim.pepperThis patch does the rename and move as per issue summary.
Comment #2
johnalbinI reviewed this patch and the comment CSS (comment indenting) is being added to Drupal from its new css/ location with its new name, comment.skin.css.
RTBC!
Comment #2.0
johnalbinadd css dir
Comment #3
shyamala commentedTagging
Comment #4
tim.plunkettPer #1921610-18: [Meta] Architect our CSS and further pushback against "skin", please do not rename .theme.css files to .skin.css.
Comment #5
nick_schuch commentedRerolled and removed the "skin" ref as per #4
Comment #6
dcam commented#5 looks good. It was rerolled so that the files are renamed instead of deleted/added, per #4. I found no instances of the old file locations in the code.
Comment #7
webchickI think we need to do these renames as a single patch, not in separate sub-issues. Because:
#1: It's clobbering up the RTBC queue with individual issues (each carrying their own roll/review/re-roll/re-review/rtbc/re-re-re-review/needs work/etc. overhead), with patches that could easily be scripted and reviewed as a whole.
#2: If they're not all done at once, we introduce inconsistencies in how files are named, which will be confusing to themers. We can't introduce more stuff to "clean up later" at this point in the release cycle. :\
#3: I still am not clear on whether or not this naming convention actually has community buy-in around it, and one place to discuss it would be much easier to determine this than several.
Can we please close these sub-issues as dupes and work at either #1921610: [Meta] Architect our CSS or some other "all in one rename" issue?
Comment #8
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.
Comment #9
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 #10
mtiftCommitted to the sandbox! :-)
Comment #11.0
(not verified) commentedupdated remaining tasks!