ComboBox with sub menus in the JUCE code


#1

Hello guys !

I need a ComboBox which allows sub menus in my application. Something which looks like that at least. My purpose will be very original, it’s for handling presets and categories of presets.

I have seen there have been a few topics already created on the JUCE forum, with people talking about that issue but in my opinion, none really brought a viable solution. For example there :



So right now, I’m wondering how to do this for real the best possible way. I see three potential solutions :

  1. Creating a new component, totally independent of any of the standard JUCE classes (I’m not talking about the Component and PopupMenu classes of course)
  2. Creating a component heriting from the ComboBox class, using mainly the functions made already virtual there, which are showPopup() and addItemsToMenu (PopupMenu&) (strangely, the destructor hasn’t been made virtual in the ComboBox class however)
  3. Reworking the current ComboBox class to add this feature, and pushing it into the current JUCE code

I’ve been asking myself, what would be the best solution out of these three. The first solution is something I have done already in the past, but I’m not really fond of it myself. I am a fervent activist of the don’t repeat yourself (DRY) principle, so when I need something I want to be able to have it a way that will make its future use as simple and fast as possible. That’s why I like to propose code to the JUCE base once in a while. Moreover, all the look and feel classes are very convenient to use, and I’d like to be able to use the current ones to change the appeareance of my application, instead of tweaking manually the paint functions of custom components. Finally, if I had to do that from scratch properly, I would need to copy and paste a lot of things from the original ComboBox class code, even if I’m developing a custom independent component, and well I think that’s silly.

The second solution seemed to be the best at first glance. However, when I attempt to do so, I discovered a few things in the JUCE ComboBox class which prevented me to succeed. For example, there is absolutely nothing with the status “protected”, which means that whatever I decide to do with the two virtual functions available, I can’t rely on any of the ComboBox internal functions and variables. To summarize, it wouldn’t be possible to use most of the provided functions in the ComboBox class, and I would need to add a lot of code for adding items, my submenus, looking for the selected item etc. And most of the original ComboBox functions would be totally useless. Moreover, I have seen in the 2014 thread that @jules doesn’t want to make protected the class elements that would make that solution convenient. So it doesn’t seem that this solution would be good either.

And… what remains here is the third solution :slight_smile: Instead of starting coding something that might become useless if someone from the JUCE team says “no”, I thought I would try to sell my idea, give a proposal for an API and an implementation, and then code it and ask the JUCE team to include my changes in the base code. I understand also that the main worry here is that I would have to do something that can’t break other people code, and don’t make more difficult the adding of any potential future add-ons in the ComboBox class.

I would like also to say once again that such a change would be very useful to most of the JUCE library users, since that demand has already appeared numerous times in the past, and because it would be specifically very useful for preset management in plug-ins.


#2

So, here is my proposal.

  • Internally, the ItemInfo structure would still be used, so there are the least possible number of changes to do in the ComboBox class. I would just need to add two extra variables, one called subMenuHeaderID which is 0 or -1 for standard items, and any other value when it is a submenu header (which means a submenu is displayed when the cursor is on it) ; the other one would be called subMenuID, which is 0 or -1 when this item isn’t in any submenu, and something else when it is in a submenu, this something else being the value of the subMenuHeaderID in the associated item.

  • Such a change would be considered in the function addItemsToMenu, the PopupMenu to display would be created by looking sequentially into the items object and doing what is needed, and in a few other places to prevent the submenu header for being considered as a clickable / valid item

The API to create the submenus would be either

  • something that gives an access to the value of the subMenuIDs (such as an extension of the functions addItem-like such as addItem (const String& newItemText, int newItemId, int subMenuHeader) and other stuff)

  • something simpler and less error-prone like having two functions startSubMenu(const String& newSubMenuHeader) and closeSubMenu(), with all the addItem functions being between them allowing the addition of items inside the current submenu

On the user side, everything would work this way :

int n = 1; combo->addItem("default preset", n++); combo->addSeparator(); combo->startSubMenu("Bass"); combo->startSubMenu("Electric"); combo->addItem("Preset 1", n++); combo->addItem("Preset 2", n++); combo->closeSubMenu(); combo->startSubMenu("Synth"); combo->addItem("Preset 1", n++); combo->addItem("Preset 2", n++); combo->closeSubMenu(); combo->closeSubMenu(); combo->startSubMenu("Guitar"); combo->addItem("Preset 1", n++); combo->addItem("Preset 2", n++); combo->closeSubMenu();
What do you think of my very long post ? :slight_smile: Of course I can code that thing myself and share it there if we agree on the terms.

Thanks to anybody who has been able to read my posts up to the end !


#3

Hi Ivan,

as always, thank you for your great suggestions. I like your idea and it’s probably the best approach, however, don’t feel that you need to suggest something which tries to minimises changes to JUCE.

I’m not sure about the “statefull-ness” of your solution, i.e. that addItem's behaviour will depend on previous calls, for example if startSubMenu was called or not.

It seems to me that there is a lot of code duplication in ComboBox and PopupMenu. Wouldn’t a better interface be that we have a “getRootMenu()” method in ComboBox which returns a PopupMenu? addItem, addItemList, addSeparator and friends will then simply become one-line inline wrappers which call the equivalent method on the getRootMenu instance.

What do you think?


#4

Hello Fabian !

You’re right about the fact that the “statefull-ness” of my solution might be a problem, and I think it would be a great idea to have a direct access to the PopupMenu that is associated with the ComboBox. This way, a lot of duplicated code might me deleted, it wouldn’t change anything for the users of the ComboBox previous iteration thanks to the wrappers, and it will help to provide a lot of extra power to the ComboBox class.

The only downside would be the need to delete the virtual function addItemsToMenu (PopupMenu& menu), but I’m not sure anyway a lot of people was using classes herited from ComboBox in their base code…

I can do that this afternoon if we’re fine on the terms :slight_smile:

EDIT : in fact keeping the function addItemsToMenu would be very convenient for the tick/enabled property !


#5

By the way, is there any particular reason why the “const Item& PopupMenu::MenuItemIterator::getItem () const” isn’t declared as a “Item& PopupMenu::MenuItemIterator::getItem () const” instead ?


#6

OK, so I have my implementation working well, but I’d like to test it a little more, and maybe to optimize it a little bit.

I have two questions :

  • Can I make the “const Item& PopupMenu::MenuItemIterator::getItem () const” function “Item& PopupMenu::MenuItemIterator::getItem () const” instead ? It would solve a lot of issues on my side…

  • Can I change the “bool PopupMenu::MenuItemIterator::next()” function into a “bool PopupMenu::MenuItemIterator::next(bool iterateIntoSubMenus = false)” for… iterating also into submenus items when the boolean is true ? I could also change the constructor of the MenuItemIterator instead, to make that behaviour change defined only once.

Thanks !


#7

OK, here is the result :

I have provided two different implementations, all of them being depending on the answer “yes” for my first previous question. The second implementation needs also the change in the MenuIterator constructor, and is quite better and faster.

Again, I have already run numerous tests, but feel free to try the different functions to see if I have not forgotten anything, and give me your opinion on the code :wink:


#8

Thank you I’ll have a look.


#9

OK this is on develop now. Thank you @IvanC!


#10

You’re welcome ! Now I can put presets in my plug-ins so much easily :slight_smile:


#11

Hi guys !

Thanks you very much for this fantastic work on ComboBox enhancements.
I do not have enough time to participate to the brainstorm. Sorry for that.

Our software Smode use some tricks to support this feature since more than two year from now.
I’m very happy that the Juce code base will implement it natively allowing me to switch to
your nice implementation.

Just some feedback before this was merged in Master Juce branch:

  1. Your changes made the noChoicesMessage (empty popup menu custom text) feature disappeared.
    If you consider to remove this feature, you should mark the two following methods as deprecated: CombBox::setTextWhenNoChoicesAvailable
    ComboBox::getTextWhenNoChoicesAvailable
    Maybe something should be added to help user understand that clicking on the dropdown button of the ComboBox will do nothing in this special case. (gray it ?)

  2. If a widget own many ComboBox and if ComboBox’s menus are huge to construct (many submenu with icons etc…) you can reach some performances issues in certain case:

This can happen for instance if you have a TableBox or a TreeView with a column full of ComboBox.
Similar preset menu for each ComboBox do not helps because they are constructed for each ComboBox.

To avoid this issue, we add an optional custom callback showPopupWanted in the ComboBox::Listener.
This to allow lazy menu filling (not at widget construction but when the user click on it)

class JUCE_API  Listener
{
public:
    /** Destructor. */
    virtual ~Listener() {}
    /** Called when a ComboBox has its selected item changed. */
    virtual void comboBoxChanged (ComboBox* comboBoxThatHasChanged) = 0;
    /** D/LABS callback allow items modification just before displaying popup menu. */
    virtual void showPopupWanted(ComboBox* ) {} 
};

So we modify the beginning of ComboBox::showPopup() function to trig this callback synchronously before displaying the menu like this:

void ComboBox::showPopup()
{
// D/LABS last chance to modify items list.
Component::BailOutChecker checker (this);
listeners.callChecked (checker, &ComboBoxListener::showPopupWanted, this);
if (checker.shouldBailOut())
return;
// then display the menu.

Hopping this could helps.
Thanks again to both of you.


#12

And here I am, the poor guy who relies on that virtual function for some obscure aspect. I am now forced to switch to the develop branch to bring in some changes that are not easily cherry-pickable to master (see this), and as a result I have to rework on this completely different area because of this breaking change.

It’s frustrating.

@fabian, I’m sorry to require your attention again, but I see it’s you who committed this (00c0671c6b2c938776bafbb3959d1ed5493d8578).
Why was the old function removed altogether, instead of just deprecating it?
How am I supposed to achieve the same functionality now?


#13

We were using this method internally at ROLI a lot too. But it really doesn’t make any sense anymore as you can just add the items to the root menu of the ComboBox. For example, at roli we had something like this:

class CustomItemComboBox   : public ComboBox
{
public:
     // children need to override this
     virtual PopupMenu::CustomComponent* createCustomItem (int itemIndex) const = 0;
     virtual int getNumCustomItems() const = 0;
private:
     // this won't compile with newer juce versions!
     void addItemsToMenu (PopupMenu& menu) const override
     {
         for (int i = 0; i < getNumCustomItems(); ++i)
              menu.addCustomItem (i + 1, createCustomItem (i + 1));
     }
};

However, this is not really necessary anymore as any subclass has access to the root menu and can simply add their custom items to the root menu. To, however, make code changes minimal, I altered the above code in the following way:

class CustomItemComboBox   : public ComboBox
{
public:
     // children need to override this
     virtual PopupMenu::CustomComponent* createCustomItem (int itemIndex) const = 0;
     virtual int getNumCustomItems() const = 0;
protected:
     // call this method from your child's constructor
     void addItemsToMenu (PopupMenu& menu) const
     {
         for (int i = 0; i < getNumCustomItems(); ++i)
              menu.addCustomItem (i + 1, createCustomItem (i + 1));
     }
};

and then any subclass of CustomItemComboBox needs to call addItemsToMenu (*getRootMenu()) from it’s constructor. Better, however, is to get rid of CustomItemComboBox altogether and simply have the subclasses add the custom items to the root menu.


Combo box issue when emtpy
#14

Hi.

I’m glad to see that the ComboBox with sub menus feature is finally available in Juce master branch.
Many thanks for this.

Here some notes:

  1. the unused variable ComboBox::noChoicesMessage is still here.
    The following method should be marked as deprecated:
    CombBox::setTextWhenNoChoicesAvailable
    ComboBox::getTextWhenNoChoicesAvailable

  2. I cannot edit my previous post on this forum thread, maybe because it’s too old.
    So just note that I abandon my Listener::showPopupWanted patch because ComboBox::showPopup is now virtual and do the job as lazy menu filling callback in my ComboBox child class. Thanks for this.

  3. ComboBox with sub menus is great but client should take extra care when adding sub items.
    A ComboBox have two different keys to read or modify the selected item:

  • the id getSelectedItemIndex()/setSelectedItemIndex()
  • or the text getText()/setText()

This design enforce that item text should be unique for all items.
It is normal to have text uniqueness in each level of ComboBox PopupMenu::Item.
But the client expects to have leaf with same text on different sub items:
submenu1/bob
submenu2/bob
(like in file system for instance)

Because the final ComboBox::getText key here is “bob” we cannot know which one is selected.
Same issue for ComboBox::setText. The iterator based implementation will select the first one.

May be a documentation warning about this could be welcome. Of course a native support of this feature in ComboBox could be nice.

To allow this behavior, I only add flatten item hierarchy with unique item text like “submenu1/bob” in ComboBox::currentMenu
My child class overload showPopup who construct and display a new hierarchical PopupMenu derived from the ComboBox::currentMenu by tokenizing the “submenu1/bob” texts. (little bit heavy)

My child class allow “/” separator customization or empty to rely on the default implementation.

The key point here is that the ComboBox::getText() must return a full unique key like “submenu1/bob”

Hopping this can helps.


#15

Hello ! 1) and 2) should be solved now (see there : Combo box issue when emtpy)

For 3), I’ll have a look and ask the JUCE guys around if I can add a solution.