Make the theme recolorable
| Project: | Bluemarine |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
| Issue tags: | GHOP |
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
#1
This task has been claimed by justMatt.
#2
#3
Unfortunately, time ran out on this task so it's back up for grabs.
#4
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
#5
Information about creating patches is available at http://drupal.org/patch/create
#6
@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.
#7
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.
#8
How about zip up the images into a zip file and post that?
#9
I just uploaded it:
http://drupal.org/node/200115
#10
Hm. is there anyway you could add that to this issue here? It's a little confusing for people when things are spread around.
#11
Here come the files :)
#12
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
#13
Ok. Sorry by marking it without asking you before... I hope the other eyes like it ;)
#14
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?
#15
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
#16
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!
#17
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:
<?php$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.
#18
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.
#19
Yes, please make a patch to make the $names array changeable.
#20
Could you please use the attached alpha png as base for the icon in the header? (see http://drupal.org/node/200334)
#21
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
#22
Sorry, I didn't read the logo change before. Here are the new patches and images with this logo. :)
#23
Works, but the logo exceeds top header by one pixel, see screenshot
#24
@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
#25
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 themeThis 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.
#26
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.
#27
Ugh, crosspost and I forgot to set to CNW anyway. :p
#28
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 :)
#29
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.
#30
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)
#31
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.
#32
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.
#33
Sorry, I've posted twice and I can't delete. Read above :)
#34
I hope this works. I won't explain everything again, just read above. :D LOL
#35
First of all, that’s some awesome stuff you’ve done there.
I have two minor nitpicks:
#36
- 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.
#37
@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
#38
@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.
#39
color.inc have missing css array... see garland or minnelli about this change 2 days ago.
#40
@hass - I've searched the Garland issue queue and I don't see what you're referring to. Can you post a link, please?
#41
// CSS files (excluding @import) to rewrite with new color scheme.'css' => array(
'style.css',
),
#42
And this goes where?
#43
Into color/color.inc. Please take a look inside garlands' color.inc
#44
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.
#45
the theme was removed from Drupal core
#46
I think it should go to the (now) contributed theme Bluemarine theme issue queue.
#47
#48
FYI, I'm totally for this being added to bluemarine-in-contrib.