C'mon guys. This is a 5-liner! All you need to do is set $info['fields'], who's keys are the internal names, and values are external names. Then you can have different keys in $info['fill']

CommentFileSizeAuthor
#14 color_module_variable_fields_1.patch6.74 KBdmitrig01
#8 color_module_variable_fields_0.patch4.47 KBdmitrig01
color_module_variable_fields.patch2.14 KBdmitrig01
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented

Ok - more detailed report:
I am a themer. I want to colorize my theme. Color module is teh awesome. It does so much stuff. Everything is configurable. *However,* there is one thing that's not - this is a bug. There is *one* line of code preventing this. The fix is really short.

eaton’s picture

eaton CreditAttribution: eaton commented

dmitri, I've tested this and Garland continues to work, but I haven't had an opportunity to test the 'expanded features'.

Where WOULD one add these additional info keys to make additional configurable colors? this would cover just colors, not sliceable image chunks, correct?

I'm just trying to figure out what the final "use this" instructions are for themers, etc.

dvessel’s picture

dvessel CreditAttribution: dvessel commented

Interesting patch. This would mean each of the extra colors defined would need an exact match inside the style sheet. That's cool but it can get complicated real fast. What about the UI for handling the scheme? Adding that one extra color for that odd color shift is nice but try handing too many and it starts to get ugly.

There also may be an issue with the functions handling the image slicing. It depends on the pallet. Haven't tested, just posting what I understand about it.

dmitrig01, how about a patch to group chunks of the style sheets based on //comments then based on each color of the scheme have each group do a relative color shift. Would be easier than micromanaging each color change but still give more flexibility. :)

eaton’s picture

eaton CreditAttribution: eaton commented

dvessel, I'm guessing that a scope of that change would be outside the scope of a post-freeze fix. dmitri, can you expound a bit on where a themer would define these extra bits of info, and what would happen if they defined more than the default keys?

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented

This patch doesn't do any color shifting, or affect anything in that matter. What it does affect is which colors a user can choose, and slices. For example, in my theme, I want the header to have a gradient, the background to be changeable, and the body color to be changeable. The win for other themes is the ability to have other slices. For example, in my theme, I would specify $info like this:

$info = array(
  // ...
  'fields' => array(
    'base' => t('Base color'),
    'link' => t('Link color'),
    'top' => t('Header top'),
    'bottom' => t('Header bottom'),
    'text' => t('Text color'),
    'body' => t('Body color'), // <-- This line
  ),
  // Then, I could specify an additional fill area for the body
  'fill' => array(
    'base' => array(0, 0, 760, 568),
    // ...
    'body' => array(0, 200, 400, 300),
  ),
);
drewish’s picture

drewish CreditAttribution: drewish commented

that seems like a very reasonable change that would allow the module to be much more flexible. i'm not so certain on the "is it a bug fix or a feature" question though.

eaton’s picture

eaton CreditAttribution: eaton commented

So, to clarify, this would be set up in a theme's template.php file, right?

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented
FileSize
color_module_variable_fields_0.patch4.47 KB

New patch. 1: if there is no base color set, the color picker disappears. 2: If a link is about to be colored, but there is no link in the fields, fall back to text, and then to base.

dvessel’s picture

dvessel CreditAttribution: dvessel commented

Cool, just tried it out with Garland by defining the 'fields' array and adding an extra color to the end of each color scheme. The sixth color shows up in the color form.

+1 Considering this is a small change with more flexiblity. Doesn't look like it's breaking anything else.

And dmitrig, it does color shift. If a color in your style.css matches your 6th color inside your scheme exactly, that color *will* change.

btw, I placed a proposal for the other idea.

dvessel’s picture

dvessel CreditAttribution: dvessel commented
Status: Needs review » Needs work

Since you can define the fields from the theme, the base and anything else essential should not be changeable from color.inc. Maybe do an array merge from the module so those fields are always returned?

After that, I think this would be RTBC.

I can see this being very useful now. Awesome dmitrig!

dvessel’s picture

dvessel CreditAttribution: dvessel commented
Status: Needs work » Needs review

Hrm, I'll let others decide if that's really an issue.

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented

I wouldn't consider this an issue... I *like* that feature, because what if I wanted to re-name base to something else, themes don't have hook_form_alter ;)

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Needs work

I like this feature, and especially digged into this issue in discussion with the active core theme SoC-er guy, who found it impossible to work with color module due to the limitations of header top and header bottom and the color field names wired in. BUT I am not entirely sure such a change this late in the cycle is good.

Anyway, some comments:

- I don't understand: "Only themes with field as a base get to be colorized - big errors if not" - what are the big errors? what does "field as a base" mean?
- both of the "if" calls introduced lack whitespace after the "if" keyword, which does not conform to coding style

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented
FileSize
color_module_variable_fields_1.patch6.74 KB

Added a check that removed various nasty errors with no gradients, changed coding style, and changed that comment

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented
Status: Needs work » Needs review
klaasvw’s picture

klaasvw CreditAttribution: klaasvw commented

I'm the SoC-guy working on the core theme :-)

I was basically writing the same patch but I think there are some additional things needed to make this work flawlessly:

  • Multiple regions/field: as a theme developer you can only assign one region to one field. It would be better if you could assign an array of regions to each field. This gives both more flexibility for the theme designer and it's not needed to define a new field for each region that needs to be colored (you can use one field for more regions).
  • Preview: this is only a patch for the backend part, it won't work with the preview feature of color.module. So this definetely needs some work imho.
  • Gradient: The least that is needed is that the gradient is optional. But to make color.module even more "modular" for theme developers allowing multiple gradients would be great. But perhaps this is something for the future.

Once I figured out how all the previewing works I'll post an updated patch.

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented

Maybe we should make a color-advanced.module that does this. I was planning on it and have some concept code:

  $info['fill'] = array(
    array(
      '#type' => 'flood',
      '#x' => 0,
      '#y' => 0,
      '#color' => 'this',
    ),
    array(
      '#type' => 'gradient',
      '#x' => 0,
      '#y' => 0,
      '#width' => 500,
      '#height' => 300,
      '#color' => 'that',
      '#color2' => 'this',
    ),
   
    array(
      '#type' => 'gradient',
      '#x' => 0,
      '#y' => 200,
      '#width' => 500,
      '#height' => 200,
      '#color' => 'this',
      '#color2' => 'that',
    ),
  );
  $info['color'] = array(
    'this' => '#ff2200',
    'that' => '#00ff00',
  );
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Version: 6.x-dev » 7.x-dev
Category: bug » feature
Priority: Critical » Normal

I agree. color-advanced module sounds like a good way to handle this, esp. given how late we are in the release cycle. If it proves itself in contrib, let's get it into core next release! :)

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented
Status: Needs review » Closed (won't fix)
beginner’s picture

beginner CreditAttribution: beginner commented
Status: Closed (won't fix) » Active

Why not for D7?

Meanwhile, please add a link to the color_advanced module here. I didn't find it.

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented

I'm sorry, I haven't made it yet. No time :(

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Definitely, go, go! Color module has so much potential in it, which bright minded people like you can bring forward!

darumaki’s picture

darumaki CreditAttribution: darumaki commented

I agree with you all rock on drupalonians, these new improvements will be very nice, how soon can this super color.module be ready to download :)

hass’s picture

hass CreditAttribution: hass commented

Subscribe :-)

JurriaanRoelofs’s picture

JurriaanRoelofs CreditAttribution: JurriaanRoelofs commented

no the colors should be set up in the theme's color/color.inc

JurriaanRoelofs’s picture

JurriaanRoelofs CreditAttribution: JurriaanRoelofs commented

Im looking forward to see improvements in the color.module, because as it is now in drupal5 it's very limiting, these are some improvements I'd like to see in order of importance (high to low):

1. support for multiple gradient squares to be drawn with top and bottom color
2. support for more 'default colors', as per the modules use of 'default colors', this will also create new ranges of target colors, meaning you can use more contrasting colors in your design.
3. support for multiple unique gradients. (instead of multiple gradients with 1 color set, as in #1)
4. support for multiple fill-squares per color. Note that while only 1 square per color is allowed now, this does not limit your design in any way. You just need to be smart about placing your slices in a grid of squares.

darumaki’s picture

darumaki CreditAttribution: darumaki commented

I'm having an epiphany re: color module, it would seem that the if i can set this up in photoshop why use the color module in the first place ? Initially i thought it was great to use but then if i can do the same with more control in photoshop why not just create my own images and template. Web designers need total control and I don't see that happening with color module.

With the color module you are very limited to the default templates and/or if you want a custom job you have to create your own base file which is fine but then you can do all that in photoshop and be done with it. Now if you need any extra shades or gradients they are at your fingertip saving you extra time. But then again, one advantage of creating a custom base file template is that you no longer have to open up photoshop instead you just apply the color variant from within drupal.

Still, I'm finding things a lot easier doing it with photoshop. If you have to choose between using one less module or having to open up photoshop I think I might choose photoshop.

hass’s picture

hass CreditAttribution: hass commented

Is someone working on color_advanced.module for D6? I would be happy if i could use it... :-)

cwgordon7’s picture

cwgordon7 CreditAttribution: cwgordon7 commented
Title: Color.module: themes can only have 5 color options » DROP Task: Color.module: themes can only have 5 color options
Project: Drupal core »
Version: 7.x-dev »
Component: color.module » Oddball DROP task
Assigned: dmitrig01 » Unassigned

This is now a DROP task. Moving off the Drupal project issue queue because this will first be in contrib; if successful, will be in 7.x core (hopefully).

jibninjas’s picture

jibninjas CreditAttribution: jibninjas commented

I cannot get this to work for the life of me. I have applied the patch like 5 times and I just cannot get an extra field to show up. I can adjust the names and delete one of the fields from the color.inc file so so it seems that it is pulling from it properly, but when I add a field nothing happens. Any suggestions?