Setting ComboBox's PopupMenu colors

Trying to set the colors for a ComboBox’s PopupMenu. I’m setting them in the constructor of a class derived from ComboBox, which works for other components (like setting the caret color in a TextEditor), but isn’t working here…

ThemedComboBox::ThemedComboBox()
{
    // Working
    setColour (ComboBox::backgroundColourId, Theme::mainBackground);
    setColour (ComboBox::textColourId, Theme::text);
    setColour (ComboBox::buttonColourId, Theme::mainBackground);
    setColour (ComboBox::arrowColourId, Theme::text);
    setColour (ComboBox::outlineColourId, Theme::border);
    setColour (ComboBox::focusedOutlineColourId, Theme::buttonHighlighted);  

    // Not working  
    setColour (PopupMenu::backgroundColourId, Theme::mainBackground);
    setColour (PopupMenu::textColourId, Theme::text);
    setColour (PopupMenu::headerTextColourId, Theme::text);
    setColour (PopupMenu::highlightedBackgroundColourId, Theme::buttonHighlighted);
    setColour (PopupMenu::highlightedTextColourId, Theme::text);
}

PopupMenu is not a component, so you cannot do:
popupMenu.setColour (PopupMenu::backgroundColourId, Colours::grey);
calling setColour (PopupMenu::backgroundColourId, ..); on your ComboBox won’t work either because the ComboBox does not handle the popupMenu painting and does not use the PopupMenu::backgroundColourId.

The only way to have those setColour (PopupMenu::backgroundColourId, ..) working is to call them on the popupMenu’s lookAndFeel.
In your case you have to call them on the ComboBox’s lookAndFeel (unless you have set a parent Component to your ComboBox’s PopupMenu, in which case the popupMenu will use that parent’s lookAndFeel).

1 Like

Reviving this thread, since I have a usecase which cannot be solved using the LookAndFeel:

I have a generic GUI engine, and the colours are controlled per Component in a hierarchical manner. There is a set of LookAndFeels, but creating a lookAndFeel for each Component is not an option.

It would be great, if the findColour() in the lookAndFeel could use the PopupMenu::Options::parentComponent* instead. This would lead to the correct behaviour.

EDIT: targetComponent is actually set, just the draw methods would need a const PopupMenu::Options& as argument.

Little bump, explanatory code:

Colour LookAndFeel::findPopupMenuColour (PopupMenu::ColourIDs colourId, const PopupMenu::Options& options)
{
    if (auto* target = options.getTargetComponent())
        return target->findColour (colourId);

    return findColour (colourId);
}

void LookAndFeel_V2::drawPopupMenuBackground (Graphics& g, int width, int height, const PopupMenu::Options& options)
{
    auto background = findPopupMenuColour (PopupMenu::backgroundColourId, options);
    // ...

needs a change of the PopupMenu::LookAndFeelMethods though, but now that we have the PopupMenu::Options that seems like the perfect use for it

1 Like

Any chance to get that in juce6 please?

Only users who have overridden the drawPopupMenu will have to add that argument to their function, they don’t even need to use it (but I guess, they will want to)

And it fixes the wrong behaviour of ComboBox not to use the defined colours for its PopupMenu.

EDIT: I took the liberty to edit this into a Feature Request, please vote :slight_smile:

EDIT 2:
Good morning to a fresh new week :slight_smile:

Any chance?

P.S. I am not allowed to post before anyone else says something. But I can edit :wink:

EDIT 3: Here is a PR to allow setting the PopupMenu colours on the ComboBox directly:

2 Likes

Why not passing the whole popupMenu::options to the popupMenu painting methods rather than just the target component?

Is a new LookAndFeel::findColour() method needed?
Couldn’t you use just use targetComponent->findColour() in your lnf ?

1 Like

That was my first thought as well. When I learned the PopupMenu won’t keep the PopupMenu::Options, I refactored to use only the targetComponent to limit the information exposed.

TBH. later I learned, that the MenuWindow stores a copy of the PopupMenu::Options, but I decided against changing it back to limit the exposed information and maybe future Components might use the same approach, which are not a PopupMenu.
But that is totally up for discussion.

Yes, that is possible. But if you look, I switched more than 10 times the findColour to use the targetComponent if available, so DRY justifies a second method IMHO.
I could have settled for a default argument, but that would impose a runtime cost on all other occasions, and only a few LookAndFeelMethods actually need to deflect to a targetComponent (so far only PopupMenu).

Little bump, can the PopupMenu::LookAndFeelMethods please make either the PopupMenu::Options or more generically the targetComponent* available?

Currently it is not possible to use the colours from the targetComponent, which I would argue is expected behaviour.

The PR above would make that possible, if it needs changing, please let me know.

N.B. the major version upgrade is a good time to change the PopupMenu::LookAndFeelMethods signature, which needs to happen eventually…

2 Likes

Still no feedback?

We’ll be adding this, along with some other PopupMenu features (manual columnbreaks, themed column separators) shortly after the JUCE 6 release.

7 Likes

I just tested this, seems there was no change. Is that still to do?
Or do I have to change something to make the PopupMenu reacting to the colours defined in ComboBox?