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
Comment | File | Size | Author |
---|---|---|---|
#36 | Drupal Logo @ Gimp | 139.08 KB | tmadeira |
#34 | bluemarine.patch (the last version, I hope :) | 10.81 KB | tmadeira |
#32 | bluemarine.patch | 10.18 KB | tmadeira |
#28 | Newer bluemarine.patch | 10.2 KB | tmadeira |
#24 | New bluemarine.patch | 10.89 KB | tmadeira |
Comments
Comment #1
webchickThis task has been claimed by justMatt.
Comment #2
webchickComment #3
webchickUnfortunately, time ran out on this task so it's back up for grabs.
Comment #4
tmadeira CreditAttribution: tmadeira commentedHey,
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
Comment #5
webernet CreditAttribution: webernet commentedInformation about creating patches is available at http://drupal.org/patch/create
Comment #6
webchick@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.
Comment #7
tmadeira CreditAttribution: tmadeira commentedI'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.
Comment #8
webchickHow about zip up the images into a zip file and post that?
Comment #9
tmadeira CreditAttribution: tmadeira commentedI just uploaded it:
http://drupal.org/node/200115
Comment #10
webchickHm. is there anyway you could add that to this issue here? It's a little confusing for people when things are spread around.
Comment #11
tmadeira CreditAttribution: tmadeira commentedHere come the files :)
Comment #12
webchickGreat. :) 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
Comment #13
tmadeira CreditAttribution: tmadeira commentedOk. Sorry by marking it without asking you before... I hope the other eyes like it ;)
Comment #14
webchickOk, 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:
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?
Comment #15
webchickOh, 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
Comment #16
aclight CreditAttribution: aclight commentedI 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!
Comment #17
tmadeira CreditAttribution: tmadeira commentedThe 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:
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.
Comment #18
kkaefer CreditAttribution: kkaefer commentedCool, awesome work already. Please make sure to set this issue to "patch (code needs review)" when you think it's ready for being reviewed.
Comment #19
kkaefer CreditAttribution: kkaefer commentedYes, please make a patch to make the $names array changeable.
Comment #20
kkaefer CreditAttribution: kkaefer commentedCould you please use the attached alpha png as base for the icon in the header? (see http://drupal.org/node/200334)
Comment #21
tmadeira CreditAttribution: tmadeira commentedI 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
Comment #22
tmadeira CreditAttribution: tmadeira commentedSorry, I didn't read the logo change before. Here are the new patches and images with this logo. :)
Comment #23
meba CreditAttribution: meba commentedWorks, but the logo exceeds top header by one pixel, see screenshot
Comment #24
tmadeira CreditAttribution: tmadeira commented@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
Comment #25
aclight CreditAttribution: aclight commentedThe 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.
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.
Comment #26
add1sun CreditAttribution: add1sun commentedHey 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.
Comment #27
add1sun CreditAttribution: add1sun commentedUgh, crosspost and I forgot to set to CNW anyway. :p
Comment #28
tmadeira CreditAttribution: tmadeira commentedThere 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 :)
Comment #29
snufkin CreditAttribution: snufkin commentedThe 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.
Comment #30
tmadeira CreditAttribution: tmadeira commentedSorry, 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)
Comment #31
snufkin CreditAttribution: snufkin commentedyeah 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.
Comment #32
tmadeira CreditAttribution: tmadeira commentedFixed 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.
Comment #33
tmadeira CreditAttribution: tmadeira commentedSorry, I've posted twice and I can't delete. Read above :)
Comment #34
tmadeira CreditAttribution: tmadeira commentedI hope this works. I won't explain everything again, just read above. :D LOL
Comment #35
kkaefer CreditAttribution: kkaefer commentedFirst of all, that’s some awesome stuff you’ve done there.
I have two minor nitpicks:
Comment #36
tmadeira CreditAttribution: tmadeira commented- 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.
Comment #37
tmadeira CreditAttribution: tmadeira commented@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
Comment #38
aclight CreditAttribution: aclight commented@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.
Comment #39
hass CreditAttribution: hass commentedcolor.inc have missing css array... see garland or minnelli about this change 2 days ago.
Comment #40
NancyDru@hass - I've searched the Garland issue queue and I don't see what you're referring to. Can you post a link, please?
Comment #41
hass CreditAttribution: hass commentedComment #42
NancyDruAnd this goes where?
Comment #43
hass CreditAttribution: hass commentedInto color/color.inc. Please take a look inside garlands' color.inc
Comment #44
NancyDruThat's what I thought, but not everyone has ever looked at that code so they wouldn't have a clue. Now they do. Thanks.
Comment #45
Pasquallethe theme was removed from Drupal core
Comment #46
frank0987 CreditAttribution: frank0987 commentedI think it should go to the (now) contributed theme Bluemarine theme issue queue.
Comment #47
PasqualleComment #48
JohnAlbinFYI, I'm totally for this being added to bluemarine-in-contrib.
Comment #49
TWA CreditAttribution: TWA commentedI'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.
Comment #50
JohnAlbinColor 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.
Comment #51
oadaeh CreditAttribution: oadaeh commentedI 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.
Comment #52
webchickAwesome! Thanks, oadaeh!
Comment #54
per-olav CreditAttribution: per-olav commentednice! I bet a lot care about it :D