Problem/Motivation

Part of #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.

The Tourmodule does not yet follow the guidelines.

Proposed resolution

css/tour.css to css/tour.module.css
css/tour-rtl.css to css/tour.module-rtl.css

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 tour.module's CSS files will have new names.

By akmalfikri and Gomez_in_the_South

CommentFileSizeAuthor
#15 1981074-tour-css-15.patch854 bytesmtift
#14 1981074-tour-css-14.patch410 byteschaz.chumley
#10 1981074-tour-css-fixed.patch854 bytesKevin Morse
#1 1981074-tour-css.patch3.5 KBGomez_in_the_South
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gomez_in_the_South’s picture

Gomez_in_the_South CreditAttribution: Gomez_in_the_South commented
Status: Active » Needs review
FileSize
1981074-tour-css.patch3.5 KB

Find patch attached.

akmalfikri’s picture

akmalfikri CreditAttribution: akmalfikri commented

I reviewed the patch in #1 and the CSS is being added to Drupal from its new location with its new name. It is RTBC from me, waiting for testbot to complete review.

Gomez_in_the_South’s picture

Gomez_in_the_South CreditAttribution: Gomez_in_the_South commented
Assigned: Gomez_in_the_South » Unassigned
Status: Needs review » Reviewed & tested by the community

Reviewed and tested.

Shyamala’s picture

Shyamala CreditAttribution: Shyamala commented
Issue tags: +Novice, +mobile, +Needs manual testing, +d8mux, +d8mux-css-cleanup

Tagging

Shyamala’s picture

Shyamala CreditAttribution: Shyamala commented

@Gomez_in_the_South thanks for the work!
The testing & change in status to RTBC cannot be done by the person who posted the patch.
Changing status to Needs Review to have someone else review it.

Shyamala’s picture

Shyamala CreditAttribution: Shyamala commented
Status: Reviewed & tested by the community » Needs review

changing status back to needs review

izus’s picture

izus CreditAttribution: izus commented
Status: Needs review » Reviewed & tested by the community

Hi,
I reviewd #1 and it lokks good for me.
Thanks

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

This needs to be rerolled with proper git configuration: http://drupal.org/documentation/git/configure

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Reviewed & tested by the community » Needs work
Kevin Morse’s picture

Kevin Morse CreditAttribution: Kevin Morse commented
Status: Needs work » Needs review
FileSize
1981074-tour-css-fixed.patch854 bytes

Should be good. I copied the git config and it uses rename instead of add and delete

Shyamala’s picture

Shyamala CreditAttribution: Shyamala commented
Status: Needs review » 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

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Project: Drupal core » Drupal 8 Mobile Initiative
Version: 8.x-dev »
Component: CSS » CSS architecture
Status: Closed (duplicate) » Needs review

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

chaz.chumley CreditAttribution: chaz.chumley commented
Assigned: Unassigned » chaz.chumley

Working on at DrupalCon Sprint

chaz.chumley’s picture

chaz.chumley CreditAttribution: chaz.chumley commented
FileSize
1981074-tour-css-14.patch410 bytes

Uploaded new patch based on SMACSS and file naming convention

mtift’s picture

mtift
he/him
Primary language English
Location Minnesota, USA
CreditAttribution: mtift commented
FileSize
1981074-tour-css-15.patch854 bytes

This was missing the tour.module.css namechange

echoz’s picture

echoz CreditAttribution: echoz commented
Assigned: chaz.chumley » Unassigned
Status: Needs review » Reviewed & tested by the community
JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Status: Reviewed & tested by the community » Fixed

Committed to the sandbox! Thanks!

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

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

formatting + steps for testing