I’m trying to make use of BurgerMenuComponent with ApplicationCommanManager, but can’t seem to get the menu to respond to any changes in the command statuses (e.g. enabled/disabled).
The command manager invokes the update correctly and the menu bar model responds by calling listeners with this as the model. So far so good.
void MenuBarModel::handleAsyncUpdate()
{
listeners.call ([this] (Listener& l) { l.menuBarItemsChanged (this); });
}
The burger menu handles this by calling setModel:
void BurgerMenuComponent::menuBarItemsChanged (MenuBarModel* menuBarModel)
{
setModel (menuBarModel);
}
…which ignores the update unless the model is actually different from current:
void BurgerMenuComponent::setModel (MenuBarModel* newModel)
{
if (newModel != model)
{
if (model != nullptr)
model->removeListener (this);
model = newModel;
if (model != nullptr)
model->addListener (this);
refresh();
listBox.updateContent();
}
}
This can’t be right? Surely the refresh and list box update should be called?
Have you tried the MenusDemo from the DemoRunner? I was able to get the BurgerMenu component to work in an app, with the Application Manager, by following that. In fact, I integrated that whole thing of being able to switch between Global Menu (Mac), MenuBar inside Window, and BurgerMenu for testing purposes.
burgerMenu.setModel (menuBarPosition == MenuBarPosition::burger ? getMenuModel() : nullptr);
And then I have my own MenuModel… I could take a more deep look if you need it…
MenuBarModel* MainComponent::getMenuModel()
{
return menuModel.get();
}
I also followed that to get the basics running. I can get the menu items to show and trigger commands, but no changes to the commands get updated. I don’t think the DemoRunner even attempts to show that functionality.
Yeah, you’re right. I just checked and my BurgerMenu doesn’t update enabled/disabled either, unless I change that function to:
void BurgerMenuComponent::setModel (MenuBarModel* newModel)
{
if (newModel != model)
{
if (model != nullptr)
model->removeListener (this);
model = newModel;
if (model != nullptr)
model->addListener (this);
}
refresh();
listBox.updateContent();
}
1 Like
Thanks! Getting closer. With the above modification the PopupMenu::Item states get updated in the burger menu, but it seems that the listBox.updateContent() is not sufficient, but needs to be followed by explicit listBox.repaint() for the changes to appear on screen.
Now i’m wondering why the burger menu items need to be clicked twice to trigger the commands. Having the menu open and after clicking elsewhere in the app, the first click on the menu only highlights the item, only the next click will trigger a command. This is very odd behaviour as well.
The commands are invoked in the mouseUp handler. The clicks aren’t working because the burger menu never gets the mouseUp event it needs. Right now i don’t fully understand why the list box methods aren’t used for the interactions. This seems like a weird way of doing it and it doesn’t seem to work properly either.
void BurgerMenuComponent::mouseUp (const MouseEvent& event)
{
auto rowIndex = listBox.getSelectedRow();
if (rowIndex == lastRowClicked && rowIndex < rows.size()
&& event.source.getIndex() == inputSourceIndexOfLastClick)
{
auto& row = rows.getReference (rowIndex);
if (! row.isMenuHeader)
{
listBox.selectRow (-1);
lastRowClicked = -1;
inputSourceIndexOfLastClick = -1;
topLevelIndexClicked = row.topLevelMenuIndex;
auto& item = row.item;
if (auto* managerOfChosenCommand = item.commandManager)
{
ApplicationCommandTarget::InvocationInfo info (item.itemID);
info.invocationMethod = ApplicationCommandTarget::InvocationInfo::fromMenu;
managerOfChosenCommand->invoke (info, true);
}
postCommandMessage (item.itemID);
}
}
}
Maybe someone from the JUCE devs could elaborate on the reasons behind the burger menu’s handling of user interaction in such awkward way?
I would need to get this working ASAP and right now the only way seems to involve ditching the current way on triggering the commands from the mouseUp handler. Is there danger of invoking unindented commands if the commands would be handled by the listBoxItemClicked method? Anything else to consider?
I took a look at this some more just for the heck of it. The problem is that the refresh() function always resets the lastRowClicked variable to -1. So by making the setModel() function always call refresh(), you click on a command and the mouse down causes a refresh() so by the time you get the mouse up, the lastRowClicked has been reset and nothing happens. But if you don’t call refresh() from setModel(), then the menu commands don’t get their enabled/disabled states updated.
So I came up with a hack that fixes it, at least it seems to. The idea is to call refresh(), but not reset the lastRowClicked in certain cases.
Of course, I never like hacking JUCE unless it’s unavoidable, but something is not working right here, and I’m not recommending these changes as a real fix, but until someone from the JUCE team such as @reuk or @attila can take a look at it,
In the juce_BurgerMenuComponent.h header, change the declaration of the refresh() function to:
void refresh(bool resetLastRow = true);
In juce_BurgerMenuComponent.cpp, change the first part of the refresh() function to:
void BurgerMenuComponent::refresh(bool resetLastRow)
{
if (resetLastRow)
lastRowClicked = inputSourceIndexOfLastClick = -1;
And make this modification to the setModel() function:
void BurgerMenuComponent::setModel (MenuBarModel* newModel)
{
if (newModel != model)
{
if (model != nullptr)
model->removeListener (this);
model = newModel;
if (model != nullptr)
model->addListener (this);
refresh();
listBox.updateContent();
}
// BURGER_FIX:
else
{
refresh(false);
listBox.updateContent();
}
}
I’m sure there’s probably a different/better way to fix it, but I would like to know what someone from JUCE thinks. And whether this works for you…
1 Like
That’s very interesting - thanks!. At this point the symptoms at your end seem to differ a bit from what i see here. Could it be that you’re testing on Windows or something. I’m on Mac.
IIRC the same kind of symptoms were caused by the mouseUp really not getting called at all on the first click after refresh and updateContent for some reason.
I fixed this by moving all logic from mouseUp to listBoxItemClicked as IMHO that’s how it should be. Got rid of all those state variables in the process. Of course i’m extremely curious to learn why it wasn’t implemented like that in the first place @fr810. So far my fix seems to work on Mac and Windows.
No, I’m on a Mac as well. That’s curious… but glad you found a workaround.
