Make recolorable, like Garland

I had a quick look at how garland does this. It has a directory garland/color/ which has the required data for colorability. I guess color.module looks for this folder and jumps in on theme settings if it exists. (Though I've heard color.module is difficult to use for anything other than Garland).

The preview.* files in that directory are just for the UI – which is non-trivial. However most of the backend support settings are in color.inc which is grockable. Generating the base image would require assistance from original visual designers though.

Steph, could you help out with that? Or refer me to who can?

Good description of the process of making a theme recolorable here
http://drupal.org/node/108459

Oh, Awesome! Thanks for that. I also found http://drupal.org/project/color_soc08, which provides more features and is more flexible than core's color.module. It doesn't seem to be stable though.

I'd love to create a finalized syntax and help colorize acquia marina. It looks like a good candidate.

Will be back here v-very soon.

skiquel! Great! I have some time available to help out with this, though not too much. Let me know how you think it would be most useful for me to use it. Also, do you have an idea of when you think you might get this feature working, either in a patch or CVS? A day? A week? A month? A year?

help steer you guys in the right direction

cvs -z6 -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib checkout -d color_soc08-DRUPAL-6--1 -r DRUPAL-6--1 contributions/modules/color_soc08/

http://kuler.adobe.com/

Peek at style.css, template.php (hook_color()), .info file (color = true), the blank files in images/color

That's about it.

Edit: updated CVS. I'll stay subscribed.

Awesome! This is a large part of the way there! I've taken some screenshots to demo this, and turned skiquel's work into a patch for DRUPAL-6--1 so that we can start working on this as a contribution to acquia marina. The patch doesn't include the additional files in acquia_marina/images/color/. These are in the zip file skiquel attached. I tweaked whitespace in the patch considerably, and added some @todos in the inline comments. Oh and I created two UI/usability issue nodes for color_soc08; #446054: Long color tags break layout on schema-create and #446056: Local tasks are lost on schema create/edit.

Yay!! :)

This may require standardization of Marina's CSS color usage (E.g. no color shorthand like white or #000.

I think color module is compatible with shorthand code:

http://api.drupal.org/api/function/_color_unpack/6

Bevan, et al. I am pretty busy during weekdays. But I'm going to take a peak and cleaning code + implementing some of those fixes (which seem relatively easy).

Great! It might be a while till I get a round to it. Thanks for the update!

OK. Today I am going to try to address those bugs. If you find anything else...

Just taken care of one.

Both those bugs are taken care of.

I was wondering if you could perhaps, after RTL support is committed, give this thing some public exposure in a -dev release so we can perfect it? I'll gladly act as a "sub-maintainer" to the color support.

I'll be uploading a new zip in a bit. I'm going to simplify this one a bit.

Skiquel, Are you fixing the bugs in color_soc08 that I linked to in #8, or the bugs in colorizable Marina that I annotated in the screenshot attached to #8?

If the latter, please upload CVS patch files here in the issue queue. These will be much easier for maintainers and self to review, test and collaborate with. If there are any new files, such as images/color/*, please attach those directly or zip the new folder and attach it.

Are you working from my patch in #8? I made some minor tweaks to your new code there – mostly whitespace, comments and code style. I would like not to have to make these again each time you upload some new work. Working with and sharing patch files allows us to collaborate easier. Thanks!

Those bugs you mentioned in color.module, which you made issues to, were both fixed last week, unless you find otherwise. If they are for color module, lets keep that discussion inside that queue so we don't side track here.

Also, I am having difficulty performing the diff, can you tell me the command you use to create that patch? I've only created patches a few times.

I have worked on simplified version of the color file with less fields. It's better to introduce a simple version, and get feedback on that.

Edit: This patch is failing for me

I've tried it on my zip, my latest tar, and the DRUPAL-6--1 branch of acquia marina.

Something like;

Should allow you to apply my patch so we can collaborate more easily on this change-set.

Make your changes to the code base, and once you're ready to upload your patch; cvs diff -up > ~/convenient-path/appropriate-name.patch then upload the file here. See also http://drupal.org/node/60108 and http://drupal.org/patch/create.

Ideally patches should always be created from and applied to a CVS checkout of the module, not the downloaded tarball, and not an already-patched or modified version of the module. That's not always possible or practical, but nevertheless 'ideal'.

How does the tarball you uploaded relate to the previous tarball that I turned into a patch? Is it a more-developed/cleaner version of it? Is it a different implementation of the same feature?

skiquel,

I had a look at the tarball you uploaded in #15. I see you changed a bunch of color names, and added some default schemas. Nice.

I converted the changes in your tarball to a patch and attached it (cvs.may09.patch) however this version does not work

The screenshot was too big. I uploaded it to flickr instead http://www.flickr.com/photos/bevanr/3529140093/sizes/o/

subscribing, greet if colors would be supported! Greetings, Martijn

+1 vote for this

@Bevan & @skiquel

This is great! Thanks for taking this on!

I created a new branch - DRUPAL-6--2. I hope that you can continue working on this within the 2.x branch.

I took the baby step of adding the color directory (and images) to the theme's images dir. for the 2.x branch. Once I have a chance to fully play with what you've done, I will add it to the 2.x branch ASAP. I hope this new branch will help with further development of making Marina recolorable.

Hi folks,

I just wanted to express my support for this project. Making this theme's colours changeable would be superawesome. In general I like the direction of the 6x-2x brach very much. Do it!

I am willing to provide some practical feedback/testing of the devs as I cannot code a whiff.

I upgraded the patch from #8 for DRUPAL-6--2. It didn't work however. My existing schemas stopped working and new/modified schemas had no effect with no errors. I didn't try with NG-color HEAD or on a fresh instance of Drupal. I didn't get started with debugging this, but uploaded the patch in case someone else wants to debug or test it on a fresh drupal instance.

@Bevan - I can confirm that schemes stopped working.

I narrowed down the problem. We recently added to template.php a new way of loading stylesheets which fixes an issue w/ Internet Explorer that limited the number of stylesheets IE can handle. This new code breaks the way stylesheets are loaded for the ng.color module. Essentially, it does not load the stylesheet(s) for the color scheme(s).

We will work on a solution ASAP. In the meantime, for reference, below is the code from template.php, that was added to the DRUPAL-6--2 branch, which break color schemes - removing the following chunk will correctly load the color scheme stylesheet(s).

template.php line 300 - 323

Thanks for documenting this jwolf. I'm not sure if or when I'll be able to work on this further. At the moment time and funding is the issue. I don't have a project that can fund it further at this time. Donations, bounties or enquiries welcome.

Donations, bounties or enquiries welcome.

How bout heaps of kudos?! ;)

skiquel, Bevan, does the module add the colorized style.css stylesheet to the stylesheet queue using drupal_add_css() or does it only put it into the $vars['styles'] variable?

In order to add some custom theme setting stylesheets in template.php we grab the stylesheet queue and write it back out to $vars['styles']. So if the colorized stylesheet is in the queue then it should be included, otherwise it'll get wiped out if it's only in $vars['styles'].

If it's not currently being added to the queue, it'd be great if it could be. Adding stylesheets at the theme layer is not uncommon, so anyone using drupal_add_css() to add stylesheets and then using the result from drupal_get_css() to re-populate $vars['styles'] is going to end up overwriting the colorized stylesheet.

Of course, I could also grab $vars['css'], add our stylesheets directly to the array, and then call drupal_get_css() with it. I've tried it and it works.

But that's seems less Drupally and would be something that other themers (or module developers?) might miss when they try to use drupal_add_css()/drupal_get_css() to repopulate $vars['styles']. So the problem might crop up again.

In addition, manipulating the $vars['css'] array directly might (?) be bypassing the css aggregation system, but I'm not sure about that.

Also, I'm not familiar with the color_nextgen code base so maybe there is a technical reason why adding the stylesheet to the queue won't work.

Thanks!

I don't know how much the approach described here (http://www.alldrupalthemes.com/drupal-blog/omg-were-live (somewhere in the middle)) is applicable to the problem at hand. But it might be worth a look.

Chris (sociotech),

I'm not sure of the issue, though am under the impression it is related to AqM's own CSS file aggregation technique which is designed to work around IE6's 30-stylesheets limit, and may well be related to your suggestion.

A little off-topic, but related; I blogged about another approach to adding CSS stylesheets to the page that doesn't require you call drupal_get_css() from your theme – which has a performance hit due to duplicate processing which is an issue for large sites that need to scale.

Bevan,

The ie6 css aggregation technique is purely optional and only used when actively selected in the development section of the theme settings. It is not recommended for production use. Instead, aggregation should be used to get around the 32-stylesheet limit. The aggregation technique in template.php is solely for the convenience of theme developers during development.

So that code is actually tangential to the issue. Even if it wasn't there, I'd still be calling drupal_add_css()/drupal_get_css() in order to add the stylesheets for custom theme settings (e.g., for width, fonts, etc.).

I wasn't aware that using add/get would incur a significant performance hit. I assumed that caching would be used on larger sites, which would eliminate the added processing penalty after a page was cached.

Another option would be for me to add the custom theme setting stylesheets separately like the ie6/7-fixes.css stylesheets. They wouldn't get aggregated, but it would side-step the color_nextgen problem, at least for our themes. Would there be a net performance gain by not calling add/get?

The approach you linked to for adding css stylesheets was a very interesting approach. One question I had was where this technique would put your stylesheets in the queue. The comments state that it still occurs late in the rendering process, but would there be any chance that they could be put in front of module and/or core stylesheets? Hopefully not, as this would eliminate the certainty of overriding other stylesheets.

If there aren't any significant downsides to your approach and if it still works in template.php, then i may give it a shot.

Thanks!

Chris/sociotech, Note that I don't know how significant the performance hit is. For 99% of Drupal web-sites it's insignificant. If you have several base/sub themes that are doing it it become more significant (log(n)), but maybe not much.

The sample code I gave in Can't Add CSS, JS, RSS Icon Or Set Title Or Messages In Preprocess Page? will add the stylesheet LAST to the page, allowing it to override every other stylesheet on the page. If you add stylesheets in in hook_init() it will be at or near the beginning, meaning it is overidden by other stylesheets. A lot of modules do it this way. This approach has a performance impact for AJAX, JSON, RSS, XMLRPC and other non-HTML/page requests, though again probably insignificant for probably 99% of Drupal sites...

I've been busy with the surgery thing for the past 2 weeks, but this weekend I'm going to devote some time to getting this thing working. Just make sure the dev is updated

tony

/acquia_marina-DRUPAL-6--2

patching file style.css
Hunk #1 FAILED at 76.
Hunk #2 FAILED at 84.
Hunk #3 FAILED at 875.
Hunk #4 FAILED at 1141.
Hunk #5 FAILED at 1287.
Hunk #6 FAILED at 1926.
6 out of 6 hunks FAILED -- saving rejects to file style.css.rej
(Stripping trailing CRs from patch.)
patching file template.php
Hunk #1 FAILED at 674.
1 out of 1 hunk FAILED -- saving rejects to file template.php.rej

If someone can see if that patch works on a clean DRUPAL-6--2 checkout... I think I'll still be able to work on it.

@skiquel - I've created separate patch fies for each file needing to be patched in the hopes of easing your patching process. The following steps will get you acquia_marina-6.x-2.x-dev patched and working with the color:ng module:

1. cvs -z6 -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib checkout -r DRUPAL-6--2 -d acquia_marina contributions/themes/acquia_marina

2. place all attached patch files w/in the acquia_marina checkout dir.

3. patch -p0 < acquia_marina_info.patch

4. patch -p0 < style.patch

5. patch -p0 < template.patch

6. patch -p0 < theme-settings.patch

Please note that removing the following, from template.php, fixes the issue with the color:ng stylesheets not loading as I mentioned in the above post:

The template.patch removes the above mentioned else while also adding the function acquia_marina_color() section.
However, removing the else is not ideal.

Attached are the modified images.

I'll add these to CVS soon-ish. I still need to clean a few up.

How is removing the else not optimal?

Color.module's current way of overriding may not work with drupal_get_css because it's not using the global $var variable's stylesheets.

Still looking into a work-around on either end.. As for the else, what is its purpose?

Looking forward to this, can't code but am willing to test.

There some "clash" with Wysiwyg API module and associated editors using javascript. Don't know what it is yet.

@skiquel - removing the else, specifically the $vars['styles'] = drupal_get_css();, is not optimal because it disables the loading of styles for other sections of the theme, e.g., every feature within theme_settings dir wont work. To clarify, the "Custom settings" section w/in the theme-settings need $vars['styles'] = drupal_get_css();. Theme width & Theme fonts settings break w/out $vars['styles'] = drupal_get_css();

@gumdrop - Please file a separate issue. The WYSIWYG issue you're reporting has nothing to do w/ the recolorable efforts.

@jwolf - My apologies however I am using everything in #34 and thought it might be relevant to this topic as well.

Have the patches on #34 been added to the dev version yet?
Many thanks :):)

No

subscribe

volunteer to be beta tester for recolorable Marina. Thanks to you folks for doing this!
our acquia marina site: http://voiceseducation.org

As Aquia Marina is one of the best looking free Themes for Drupal it would be a great enhancement when it is colorable.

Is there any progress on this topic or any timeframe when you think it would be available as dev or release?

thnx in advance.

subscribe

subscribed

subscribe