MenuBarComponent::showMenu()


#1

I want key combinations Alt+L (where L is some letter) to show the first menu that begins with that letter.

So my DocumentWindow reacts to these keypresses by finding the MenuBarComponent (using dynamic_cast on child components) and calling its showMenu() method - the docs suggest this is the one to use for triggering a menu from the keyboard.

Well, this doesn’t work for me.

void MenuBarComponent::showMenu (int index) { if (index != currentPopupIndex) { if (inModalState) {

My app is not in modal state so this doesn’t get executed.

hideCurrentMenu(); indexToShowAgain = index; return; }

After that comes some administration, deletion watchers are created and a global mouse listener is installed - which is probably fine. Then I get to this loop:

for (;;) { const int x = getScreenX() + xPositions [itemUnderMouse]; const int w = xPositions [itemUnderMouse + 1] - xPositions [itemUnderMouse];

and more code I don’t understand in detail, but it seems like the idea is to use the mouse position to decide which item to display - and this ends up with a DummyMenuComponent being created because the mouse is elsewhere. The index passed to showMenu() is simply ignored.

Am I supposed to put my app in modal state before calling MenuBarComponent::showMenu(), or is the method broken or am I doing things the wrong way?


#2

Ok… I’d have to debug that to see what’s going wrong, but it all looks ok at a glance. There’s no need to do anything modal yourself, it should just work, and the mouse is used because you’d expect the menu to respond to the mouse, even if it wasn’t a mouse-click that opened it.


#3

I don’t have a “pure C++” sample (it happens in my PILS editor which is written in PILS using juce bindings, and debugging is a bit tricky), but I’ll prepare a sample if needed.

However, consider this:

void MenuBarComponent::showMenu (int index) { if (index != currentPopupIndex) { if (inModalState) { hideCurrentMenu(); indexToShowAgain = index;

The index parameter is used twice - first time, to decide if the menu is already shown, and second time, in a conditional block that doesn’t get executed unless the window is modal.

After that, it is not used.

Without using index, how would the remaining code be able to show the correct menu???


#4

Well, here is a sample. Replace the HelloWorldWindow class in Main.cpp of the juce_main sample with this class:

[code]class HelloWorldWindow : public DocumentWindow
, private MenuBarModel
{
public:
HelloWorldWindow()
: DocumentWindow (T(“JUCE Hello World!”),
Colours::lightgrey,
DocumentWindow::allButtons,
true)
{
// Create an instance of our main content component, and add it
// to our window.

    TextEditor* const contentComponent = new TextEditor();
    contentComponent->setMultiLine(true);
    contentComponent->setText("Watch the caret");
    contentComponent->setSize(300, 200);

    setContentComponent (contentComponent, true, true);

    menuNames.add("Fruits");
    popup[0].addItem(100, "Apple");
    popup[0].addItem(101, "Banana");
    menuNames.add("Vegetables");
    popup[1].addItem(200, "Carrot");
    popup[1].addItem(201, "Tomato");
    setMenuBar(this);
    for (int i = 0; i < getNumChildComponents(); i++)
    {
        menuBar = dynamic_cast<MenuBarComponent *>(getChildComponent(i));
        if (menuBar) break;
    }

    centreWithSize (getWidth(), getHeight());

    setVisible (true);
    contentComponent->grabKeyboardFocus();
}

~HelloWorldWindow()
{
    // (the content component will be deleted automatically, so no need to do it here)
    setMenuBar(NULL);
}

//==============================================================================
void closeButtonPressed()
{
    // When the user presses the close button, we'll tell the app to quit. This 
    // window will be deleted by our HelloWorldApplication::shutdown() method
    // 
    JUCEApplication::quit();
}

virtual const StringArray getMenuBarNames() {return menuNames;}
virtual const PopupMenu getMenuForIndex(int index, const String &name) {return popup[index];}
virtual void menuItemSelected(int, int) {}
virtual bool keyPressed(const KeyPress &key)
{
    if (key.getModifiers().isAltDown())
    {
        int altCode = key.getKeyCode();
        switch (altCode)
        {
        case 'F':
            menuBar->showMenu(0);
            return true;
        case 'V':
            menuBar->showMenu(1);
            return true;
        }
    }
    return false;
}

juce_UseDebuggingNewOperator

private:
StringArray menuNames;
PopupMenu popup[2];
MenuBarComponent *menuBar;
};
[/code]

This will add a menu bar with menus Fruits and Vegetables. I replaced the content with a TextEditor; the caret indicates the focus.

Pressing Alt-F and Alt-V will call showMenu() with the appropriate index.

The first press will cause the caret to disappear from the TextEditor, indicating it lost the focus - but no menu is shown. I think an invisible dummy menu is displayed, setting the menu bar in modal mode. The Esc key seems to have no effect in this state.

The 2nd Alt-press works - I think because the menu bar is now modal. The Esc key will restore focus to the TextEditor content.


#5

Well, this hack fixes it:

[code]void MenuBarComponent::showMenu (int index)
{
if (index != currentPopupIndex)
{
if (inModalState)
{
hideCurrentMenu();
indexToShowAgain = index;
return;
}

    indexToShowAgain = -1;
    currentPopupIndex = -1;
    itemUnderMouse = index; //hack[/code]

Now, the index arg controls which menu is shown in the first loop iteration, which must have been be the intention if I understand things right.

But it’s a hack; I hope you’ll find time for a proper fix.

I suspect this bug has gone unnoticed for long because users and programmers have gotten used to just press a bit harder when the first keypress doesn’t work, thinking their keyboard is worn.


#6

Hi there

Well, before I noticed your follow-up post, I had a look at this and came up with exactly the same fix! It’s not really a hack - that makes it work the way it was supposed to. All checked-in now.


#7

Thanks - I called it a hack because I was concerned about using itemUnderMouse for storing an index that wasn’t derived from the mouse position - but if we both think this is the way to fix it, I trust we’re right.


#8

I think the method could have been designed a bit more elegantly, but it looks pretty solid to me now.