http://code.google.com/p/google-highly-open-participation-drupal/issues/...

Since Drupal 5, color.module ships with Drupal. This module allows administrators to change the color of their website with a regular easy-to-use color picker known from your favorite image manipulation program. However, each theme that is to be recolored has to be compatible with it.

Unfortunately, only one out of four themes in the Drupal core package is currently recolorable. Your task is to take the "bluemarine" theme and make it recolorable. Despite bluemarine being a simple looking theme, it has proven quite solid and usable for the backend, yet many people would like to change the bluish colors (that's why it's called bluemarine, after all) to something they like better.

The deliverable for this task is a patch against Drupal HEAD to make Bluemarine compatible with the color.module submitted to the issue queue. The task is complete when the patch has been marked "Ready To Be Committed".

Resources:
* Integration with color.module: http://drupal.org/node/108459
* Theme Developer's Guide to Drupal 6: http://drupal.org/node/171179
* Color module API: http://api.drupal.org/api/file/modules/color/color.module

Estimated time:
3-4 days

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

This task has been claimed by justMatt.

webchick’s picture

Title: #16: Make the "bluemarine" theme in Drupal core recolorable » GHOP #16: Make the "bluemarine" theme in Drupal core recolorable
Project: Google Highly Open Participation Contest (GHOP) » Drupal core
Version: » 7.x-dev
Component: GHOP Task » theme system
webchick’s picture

Status: Postponed (maintainer needs more info) » Active

Unfortunately, time ran out on this task so it's back up for grabs.

tmadeira’s picture

FileSize
63.06 KB

Hey,
I claimed the task 47 hours ago and now it's done. The theme is working with Color Module and everything is OK, but I don't know how to create the patch against the Drupal HEAD. Is there a how-to or something to help me? Can't I just send a gzipped file with the theme?

Thanks,
Tiago

webernet’s picture

Information about creating patches is available at http://drupal.org/patch/create

webchick’s picture

Status: Active » Needs review

@tmadeira, if you have access to IRC, hopping by #drupal or #drupal-ghop allows one of us to work it out with you in real time, which is probably faster. Otherwise, the docs wwwebernet posted are great.

Marking for review.

tmadeira’s picture

I'm sorry, but I guess it's not possible to patch a PNG (binary) file. I asked it on IRC but nobody answered me. Should I submit the patch without the PNG modifications? Should I submit the PNGs packed with the patch? The Drupal docs don't specify that.
Thanks very much,
Tiago.

webchick’s picture

How about zip up the images into a zip file and post that?

tmadeira’s picture

I just uploaded it:
http://drupal.org/node/200115

webchick’s picture

Hm. is there anyway you could add that to this issue here? It's a little confusing for people when things are spread around.

tmadeira’s picture

Assigned: Unassigned » tmadeira
Status: Needs review » Reviewed & tested by the community
FileSize
9.93 KB
10.11 KB
11.62 KB

Here come the files :)

webchick’s picture

Status: Reviewed & tested by the community » Needs review

Great. :) Thanks a lot!

I'm marking this back for review because we need at least two sets of eyes on it, and you can't set your own tasks ready to be committed. :D

tmadeira’s picture

Ok. Sorry by marking it without asking you before... I hope the other eyes like it ;)

webchick’s picture

Status: Needs review » Needs work

Ok, I tried this, and it works! Wowza!! For anyone testing this patch, the images need to go in bluemarine's "color" directory, or you'll get a whole whack of errors. ;)

A few things:

a) When submitting the form, I get the following notices:

    * notice: Undefined index: gradient in /Users/webchick/Sites/head/modules/color/color.module on line 399.
    * notice: Undefined offset: 1 in /Users/webchick/Sites/head/modules/color/color.module on line 307.
    * notice: Undefined index: base in /Users/webchick/Sites/head/modules/color/color.module on line 327.

Make sure you're working against the latest HEAD in CVS (else the 6.x-dev tarball), not 6.0 beta4. It has these kinds of messages turned off.

b) Is it possible to rename the values on the form? For example, "Base color" is actually "Header color", and "Header top" is actually "Left sidebar" and "Header bottom" is actually "Footer." It makes things pretty confusing. (btw, it might be that this is NOT possible, since we already lost one student on this task, but if not, could you please report back with what you found while researching this, and recommendations on a fix?)

c) What resources did you use to do this? Were there any places you found them confusing?

webchick’s picture

Oh, two more things:

1. It doesn't seem to respect my custom logo in "Global settings" and blows it away with the default Druplicon. It does work if I set it in admin/build/themes/settings/bluemarine instead.

2. We also need the theme's description, "Table-based multi-column theme with a marine and ash color scheme," changed, since this is no longer strictly true. :D

aclight’s picture

I also tested this out and it's awesome. I also get the same errors webchick reported, and I'm using 6.x-dev.

I imagine that getting this all fixed up should be pretty simple--again, great job with this!

tmadeira’s picture

The few things answers... ;)

a) I fixed it with some modifications in /modules/color/color.module. I'll submit the patch when I finish the rest.

b) /modules/color/color.module:139:

  $names = array(
    'base' => t('Base color'),
    'link' => t('Link color'),
    'top' => t('Header top'),
    'bottom' => t('Header bottom'),
    'text' => t('Text color')
  );

Actually it's not possible to change it, but I can write a patch to make the $names array changeable in /themepath/color.inc. I think it's simple. Shall I do that?

c) I've read a lot of docs from Drupal (integrating with color module, creating patches, submitting patches, theme developers guide, cvs - checking out from the main repository, ...). They're all pretty good.

1. This is actually the only thing from your items I don't know how to do. If you have any tip it's going to help a lot, but anyway I'm going to look for...

2. Fixed to "Table-based recolorable multi-column theme". I'm going to submit it in the next patch.

kkaefer’s picture

Cool, awesome work already. Please make sure to set this issue to "patch (code needs review)" when you think it's ready for being reviewed.

kkaefer’s picture

Yes, please make a patch to make the $names array changeable.

kkaefer’s picture

FileSize
3.46 KB

Could you please use the attached alpha png as base for the icon in the header? (see http://drupal.org/node/200334)

tmadeira’s picture

Status: Needs work » Needs review
FileSize
2.84 KB
10.67 KB

I had to separate the patches because "cvs diff" does not support new directories creation and I've created themes/bluemarine/color.

- bluemarine.patch will patch /themes/bluemarine
- colormodule.patch will patch /modules/color

Well... and these files are essential to make everything works fine:
- /themes/bluemarine/color/preview.png
- /themes/bluemarine/color/base.png

meba’s picture

Status: Needs review » Needs work
FileSize
4.84 KB

Works, but the logo exceeds top header by one pixel, see screenshot

tmadeira’s picture

Status: Needs work » Needs review
FileSize
10.89 KB

@meba, actually this is a bug bluemarine already had. The fix is just a "padding-bottom:1px;", so here I am posting the bluemarine.patch again with this modification to replace bluemarine.patch I've put in http://drupal.org/node/197243#comment-657315

aclight’s picture

Status: Needs review » Needs work

The actual recoloring functionality works well. I don't see any errors and now the description for the various recolorable aspects match what actually get recolored. This is using everything in comment #22 above with the exception that the blumarine patch is from #24.

Code/patch comments:
1.

+description = Table-based recolorable multi-column theme

This needs a period at the end.

2. I think you misunderstood webchick's point #2 in comment #15. You fixed this in the .info file but not on the themes page itself (at admin/build/themes).

My only other gripe is that I think the color schemes look really ugly. I suspect you just copied them from garland, but for some reason they look fine in garland but not so good in bluemarine. I don't think that the color schemes should hold up this task as far as GHOP goes, but before this gets committed to Drupal I think it would be best to have some more attractive color schemes.

Other than the two things above, I don't see anything else.

add1sun’s picture

Status: Needs work » Needs review

Hey tmadeira, looking good!

Only issue I had applying the patch is that the newest bluemarine.patch fails on HUNK 1, which is the $Id line of the .info file. Make sure that you don't remove that line.

Functionally it is working well for me except for the issue webchick raised about the Global settings Logo problem.

Also for your new files, make sure you add the CVS $Id$ at the top of the file.

add1sun’s picture

Status: Needs review » Needs work

Ugh, crosspost and I forgot to set to CNW anyway. :p

tmadeira’s picture

Status: Needs work » Needs review
FileSize
10.2 KB

There goes the new bluemarine.patch to use together with everything in comment #22 (except that old bluemarine.patch).

@aclight, I fixed the info and created three color schemes: Simple Red, Simple Black and Simple Green (yeah, I'm very creative :D)

@add1sun, I added back the "id" line, but I can't fix the logo problem because it's a bug from Color Module (garland has this issue too)

So, I think the job is ready. I hope you like :)

snufkin’s picture

Status: Needs review » Needs work

The set-up screen works well, when i change colours it shows up on the preview too etc. But when i tried to save it i got these errors in my face:

* warning: getimagesize(themes/bluemarine/color/base.png) [function.getimagesize]: failed to open stream: No such file or directory in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 222.
* notice: Undefined index: copy in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 273.
* warning: Invalid argument supplied for foreach() in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 273.
* warning: imagecreatefrompng(themes/bluemarine//color/base.png) [function.imagecreatefrompng]: failed to open stream: No such file or directory in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 386.
* warning: imagesx(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 387.
* warning: imagesy(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 388.
* warning: imagecreatetruecolor() [function.imagecreatetruecolor]: Invalid image dimensions in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 391.
* warning: imagealphablending(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 392.
* warning: imagecolorallocate(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 509.
* warning: imagefilledrectangle(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 396.
* warning: imagecolorallocate(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 509.
* warning: imagefilledrectangle(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 396.
* warning: imagecolorallocate(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 509.
* warning: imagefilledrectangle(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 396.
* notice: Undefined index: gradient in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 400.
* warning: imagecopy(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 406.
* warning: imagedestroy(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 409.
* warning: imagecopy(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 425.
* warning: imagecopyresampled(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 420.
* warning: imagedestroy(): supplied argument is not a valid Image resource in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 441.
* notice: Undefined index: base in /home/balu/projects/drupal/d6-b4/modules/color/color.module on line 328.

I used a clean checkout.

Also the part of the patch that alters the info files is rejected.

tmadeira’s picture

Status: Needs work » Needs review

Sorry, but you probably didn't read #22. I wrote: There goes the new bluemarine.patch to use together with everything in comment #22 (except that old bluemarine.patch).
It won't work if you don't patch color.module and you don't download PNG files. (read #22)

snufkin’s picture

Status: Needs review » Reviewed & tested by the community

yeah true, sorry for that I just took the patch and tried without checking the whole issue. Using those pictures and the color module patch it works great! The .info part of the patch is still wrong though, but this is just a tiny issue. After that is corrected I say RTBC.

tmadeira’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
10.18 KB

Fixed the wrong bluemarine.info patch. :)

To explain to who didn't use this patch yet:
You must read the comment #22 to get the PNG files and colormodule.patch. This bluemarine.patch is just going to replace the bluemarine.patch in comment #22.

tmadeira’s picture

Sorry, I've posted twice and I can't delete. Read above :)

tmadeira’s picture

I hope this works. I won't explain everything again, just read above. :D LOL

kkaefer’s picture

First of all, that’s some awesome stuff you’ve done there.

I have two minor nitpicks:

  • The logo.png file you create is 46x53, whereas the original logo.png is 48x55
  • You should increase the line spacing on the live preview. If possible, make it look as close to a regular screenshot as possible.
tmadeira’s picture

FileSize
139.08 KB

- The logo size is different because there are some pixels completely transparent in the 48x55 that has no importance at all. I'm attaching a Gimp screenshot to show you ;)
- About line spacing, you're right. I'm going to correct it and I'll post a patch in the next 30 minutes.

tmadeira’s picture

@kkaefer,
Actually I've seen the page and the CSS and there's no difference between line spacing in homepage and live preview. If it's different there, could you send me a screenshot? Or if it doesn't, I think it's finally ready.

If anyone wants to review: http://drupal.org/node/197243#comment-658352

aclight’s picture

Status: Needs review » Reviewed & tested by the community

@kkaefer: tmadeira has promised me to work to fix the issues you point out in #35 if you aren't satisfied with his responses in #36 and #37. Therefore I'm marking this as RTBC so he can get credit for the task and move on to another. Feel free to change status if you aren't satisfied with his responses.

hass’s picture

Status: Reviewed & tested by the community » Needs work

color.inc have missing css array... see garland or minnelli about this change 2 days ago.

NancyDru’s picture

@hass - I've searched the Garland issue queue and I don't see what you're referring to. Can you post a link, please?

hass’s picture

  // CSS files (excluding @import) to rewrite with new color scheme.
  'css' => array(
    'style.css',
  ),
NancyDru’s picture

And this goes where?

hass’s picture

Into color/color.inc. Please take a look inside garlands' color.inc

NancyDru’s picture

Issue tags: +GHOP

That's what I thought, but not everyone has ever looked at that code so they wouldn't have a clue. Now they do. Thanks.

Pasqualle’s picture

Status: Needs work » Closed (won't fix)

the theme was removed from Drupal core

frank0987’s picture

I think it should go to the (now) contributed theme Bluemarine theme issue queue.

Pasqualle’s picture

Title: GHOP #16: Make the "bluemarine" theme in Drupal core recolorable » Make the theme recolorable
Project: Drupal core » Bluemarine
Version: 7.x-dev » 7.x-1.x-dev
Component: theme system » Code
Assigned: tmadeira » Unassigned
Category: task » feature
Status: Closed (won't fix) » Active
JohnAlbin’s picture

FYI, I'm totally for this being added to bluemarine-in-contrib.

TWA’s picture

I've done all the steps and it works well for the preview. But when I save it the only affected element is the logo and other parts remain the same !!! I didn't recieve any error messages !!!
Please help me.

JohnAlbin’s picture

Status: Active » Needs work

Color module has been beaten with a clue stick for D7. So the color.module patch won't be necessary anymore. But the bluemarine work will need to be updated to use D7 color module's API.

oadaeh’s picture

Status: Needs work » Fixed
Issue tags: -GHOP

I don't know if any of you still care about this or not, but I updated the patch to use the new version of the Color module and committed the changes, here: http://drupalcode.org/project/bluemarine.git/commit/7d37eb1

As an FYI, I couldn't get the image preview method to work correctly, so I quit trying that way and did it the way it's done in Bartik.

I'm marking this four year old issue as fixed. If there are any problems with what I committed, please create a new issue. I'm going to give it a couple of weeks to bake in dev before I create a new stable release, just in case anything comes up.

Thank you all for all the effort you put into making this happen.

webchick’s picture

Awesome! Thanks, oadaeh!

Status: Fixed » Closed (fixed)

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

per-olav’s picture

nice! I bet a lot care about it :D