Some changes to work around MacOS menu issues

- ID's are used instead of pointers to identify menu items
  vs. QAction's. This is a weak measure to enhance predictability.
- The file menu is built from abstract menu items instead with
  native Qt objects. This way the bug fix applies both to
  file menu items and the other menu entries

The main fix is:
- A menu sync is forced by emitting a focusWindowChanged event
  from the application object. This forces the QCocoaMenuBar
  implementation to update the system menu.
This commit is contained in:
Matthias Koefferlein 2018-02-08 16:23:59 -08:00
parent fb69dfd866
commit 396d0263d4
5 changed files with 143 additions and 97 deletions

View File

@ -1380,6 +1380,17 @@ GuiApplication::notify (QObject *receiver, QEvent *e)
return ret; return ret;
} }
void
GuiApplication::force_update_app_menu ()
{
#if defined(__APPLE__)
// This is a workaround for a bug in the MacOS native menu integration:
// this signal forces the menu to become updated. Without this, any
// new menu items stay disabled.
emit focusWindowChanged (focusWindow ());
#endif
}
int int
GuiApplication::exec () GuiApplication::exec ()
{ {

View File

@ -432,6 +432,12 @@ public:
return mp_mw; return mp_mw;
} }
/**
* @brief Forces update of the application menu
* This function is used for work around a MacOS issue.
*/
void force_update_app_menu ();
protected: protected:
virtual void setup (); virtual void setup ();
virtual void shutdown (); virtual void shutdown ();

View File

@ -450,6 +450,7 @@ MainWindow::MainWindow (QApplication *app, const char *name)
m_disable_tab_selected (false), m_disable_tab_selected (false),
m_exited (false), m_exited (false),
dm_do_update_menu (this, &MainWindow::do_update_menu), dm_do_update_menu (this, &MainWindow::do_update_menu),
dm_do_update_file_menu (this, &MainWindow::do_update_file_menu),
dm_exit (this, &MainWindow::exit), dm_exit (this, &MainWindow::exit),
m_grid_micron (0.001), m_grid_micron (0.001),
m_default_grids_updated (true), m_default_grids_updated (true),
@ -613,10 +614,6 @@ MainWindow::MainWindow (QApplication *app, const char *name)
tl_assert (bookmark_menu != 0); tl_assert (bookmark_menu != 0);
connect (bookmark_menu, SIGNAL (aboutToShow ()), this, SLOT (bookmark_menu_show ())); connect (bookmark_menu, SIGNAL (aboutToShow ()), this, SLOT (bookmark_menu_show ()));
QMenu *file_menu = mp_menu->menu ("file_menu");
tl_assert (file_menu != 0);
connect (file_menu, SIGNAL (aboutToShow ()), this, SLOT (file_menu_show ()));
// select the default mode // select the default mode
select_mode (lay::LayoutView::default_mode ()); select_mode (lay::LayoutView::default_mode ());
@ -1595,6 +1592,8 @@ MainWindow::configure (const std::string &name, const std::string &value)
} }
} }
dm_do_update_file_menu ();
return true; return true;
} else if (name == cfg_micron_digits) { } else if (name == cfg_micron_digits) {
@ -4173,30 +4172,31 @@ MainWindow::add_mru (const std::string &fn_rel, const std::string &tech)
} }
void void
MainWindow::file_menu_show () MainWindow::do_update_file_menu ()
{ {
if (mp_menu->is_valid ("file_menu.open_recent_menu")) { std::string mru_menu = "file_menu.open_recent_menu";
Action open_recent_action = mp_menu->action ("file_menu.open_recent_menu"); if (mp_menu->is_valid (mru_menu)) {
Action open_recent_action = mp_menu->action (mru_menu);
open_recent_action.set_enabled (true);
if (m_mru.size () > 0 && edits_enabled ()) { if (m_mru.size () > 0 && edits_enabled ()) {
open_recent_action.set_enabled (true); // rebuild MRU menu
std::vector<std::string> items = mp_menu->items (mru_menu);
QMenu *open_recent_menu = open_recent_action.qaction ()->menu (); for (std::vector<std::string>::const_iterator i = items.begin (); i != items.end (); ++i) {
if (open_recent_menu) { mp_menu->delete_item (mru_menu + "." + *i);
}
open_recent_menu->clear ();
for (std::vector<std::pair<std::string, std::string> >::iterator mru = m_mru.end (); mru != m_mru.begin (); ) {
--mru;
unsigned int i = std::distance (m_mru.begin (), mru);
QAction *action = open_recent_menu->addAction (tl::to_qstring (mru->first));
action->setObjectName (tl::to_qstring (tl::sprintf ("open_recent_%d", i + 1)));
gtf::action_connect (action, SIGNAL (triggered ()), this, SLOT (open_recent ()));
action->setData (QVariant (int (i)));
}
for (std::vector<std::pair<std::string, std::string> >::iterator mru = m_mru.end (); mru != m_mru.begin (); ) {
--mru;
unsigned int i = std::distance (m_mru.begin (), mru);
Action action;
gtf::action_connect (action.qaction (), SIGNAL (triggered ()), this, SLOT (open_recent ()));
action.set_title (mru->first);
action.qaction ()->setData (QVariant (int (i)));
mp_menu->insert_item (mru_menu + ".end", tl::sprintf ("open_recent_%d", i + 1), action);
} }
} else { } else {
@ -5016,6 +5016,10 @@ void
MainWindow::do_update_menu () MainWindow::do_update_menu ()
{ {
mp_menu->build (menuBar (), mp_tool_bar); mp_menu->build (menuBar (), mp_tool_bar);
lay::GuiApplication *app = dynamic_cast<lay::GuiApplication *> (qApp);
if (app) {
app->force_update_app_menu ();
}
} }
void void

View File

@ -859,7 +859,6 @@ protected slots:
void menu_changed (); void menu_changed ();
void message_timer (); void message_timer ();
void bookmark_menu_show (); void bookmark_menu_show ();
void file_menu_show ();
void edits_enabled_changed (); void edits_enabled_changed ();
void file_changed_timer (); void file_changed_timer ();
void file_changed (const QString &path); void file_changed (const QString &path);
@ -868,6 +867,7 @@ protected slots:
protected: protected:
void update_content (); void update_content ();
void do_update_menu (); void do_update_menu ();
void do_update_file_menu ();
private: private:
TextProgressDelegate m_text_progress; TextProgressDelegate m_text_progress;
@ -911,6 +911,7 @@ private:
bool m_disable_tab_selected; bool m_disable_tab_selected;
bool m_exited; bool m_exited;
tl::DeferredMethod<MainWindow> dm_do_update_menu; tl::DeferredMethod<MainWindow> dm_do_update_menu;
tl::DeferredMethod<MainWindow> dm_do_update_file_menu;
tl::DeferredMethod<MainWindow> dm_exit; tl::DeferredMethod<MainWindow> dm_exit;
QTimer m_message_timer; QTimer m_message_timer;
QTimer m_file_changed_timer; QTimer m_file_changed_timer;

View File

@ -217,7 +217,17 @@ class ActionObject
: public QAction : public QAction
{ {
public: public:
ActionObject (QObject *parent) : QAction (parent) { } ActionObject (QObject *parent)
: QAction (parent)
{
static size_t s_id = 0;
m_id = ++s_id;
}
size_t id () const
{
return m_id;
}
bool event(QEvent *e) bool event(QEvent *e)
{ {
@ -251,8 +261,18 @@ public:
return QAction::event(e); return QAction::event(e);
} }
private:
size_t m_id;
}; };
static size_t
id_from_action (QAction *action)
{
ActionObject *ao = dynamic_cast<ActionObject *> (action);
return ao ? ao->id () : 0;
}
ActionHandle::ActionHandle (QWidget *parent) ActionHandle::ActionHandle (QWidget *parent)
: mp_menu (0), : mp_menu (0),
mp_action (new ActionObject (parent)), mp_action (new ActionObject (parent)),
@ -1013,9 +1033,11 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
m_helper_menu_items.clear (); m_helper_menu_items.clear ();
tbar->clear (); tbar->clear ();
std::set<QAction *> present_actions; std::set<std::pair<size_t, QAction *> > present_actions;
QList<QAction *> a = mbar->actions (); QList<QAction *> a = mbar->actions ();
present_actions.insert (a.begin (), a.end ()); for (QList<QAction *>::const_iterator i = a.begin (); i != a.end (); ++i) {
present_actions.insert (std::make_pair (id_from_action (*i), *i));
}
for (std::list<AbstractMenuItem>::iterator c = m_root.children.begin (); c != m_root.children.end (); ++c) { for (std::list<AbstractMenuItem>::iterator c = m_root.children.begin (); c != m_root.children.end (); ++c) {
@ -1053,10 +1075,10 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
c->set_action (Action (new ActionHandle (menu)), true); c->set_action (Action (new ActionHandle (menu)), true);
} else { } else {
// Move the action to the end if present in the menu already // Move the action to the end if present in the menu already
std::set<QAction *>::iterator a = present_actions.find (c->menu ()->menuAction ()); std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.find (std::make_pair (id_from_action (c->menu ()->menuAction ()), c->menu ()->menuAction ()));
if (a != present_actions.end ()) { if (a != present_actions.end ()) {
if (s_can_move_menu) { if (s_can_move_menu) {
mbar->removeAction (*a); mbar->removeAction (a->second);
mbar->addMenu (c->menu ()); mbar->addMenu (c->menu ());
} }
present_actions.erase (*a); present_actions.erase (*a);
@ -1071,10 +1093,10 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
} else { } else {
// Move the action to the end if present in the menu already // Move the action to the end if present in the menu already
std::set<QAction *>::iterator a = present_actions.find (c->action ().qaction ()); std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.find (std::make_pair (id_from_action (c->action ().qaction ()), c->action ().qaction ()));
if (a != present_actions.end ()) { if (a != present_actions.end ()) {
if (s_can_move_menu) { if (s_can_move_menu) {
mbar->removeAction (*a); mbar->removeAction (a->second);
mbar->addAction (c->action ().qaction ()); mbar->addAction (c->action ().qaction ());
} }
present_actions.erase (*a); present_actions.erase (*a);
@ -1086,17 +1108,19 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
} }
// Remove all actions that have vanished // Remove all actions that have vanished
for (std::set<QAction *>::iterator a = present_actions.begin (); a != present_actions.end (); ++a) { for (std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.begin (); a != present_actions.end (); ++a) {
mbar->removeAction (*a); mbar->removeAction (a->second);
} }
} }
void void
AbstractMenu::build (QMenu *m, std::list<AbstractMenuItem> &items) AbstractMenu::build (QMenu *m, std::list<AbstractMenuItem> &items)
{ {
std::set<QAction *> present_actions; std::set<std::pair<size_t, QAction *> > present_actions;
QList<QAction *> a = m->actions (); QList<QAction *> a = m->actions ();
present_actions.insert (a.begin (), a.end ()); for (QList<QAction *>::const_iterator i = a.begin (); i != a.end (); ++i) {
present_actions.insert (std::make_pair (id_from_action (*i), *i));
}
for (std::list<AbstractMenuItem>::iterator c = items.begin (); c != items.end (); ++c) { for (std::list<AbstractMenuItem>::iterator c = items.begin (); c != items.end (); ++c) {
@ -1111,10 +1135,10 @@ AbstractMenu::build (QMenu *m, std::list<AbstractMenuItem> &items)
c->set_action (Action (new ActionHandle (menu)), true); c->set_action (Action (new ActionHandle (menu)), true);
} else { } else {
// Move the action to the end if present in the menu already // Move the action to the end if present in the menu already
std::set<QAction *>::iterator a = present_actions.find (c->menu ()->menuAction ()); std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.find (std::make_pair (id_from_action (c->menu ()->menuAction ()), c->menu ()->menuAction ()));
if (a != present_actions.end ()) { if (a != present_actions.end ()) {
if (s_can_move_menu) { if (s_can_move_menu) {
m->removeAction (*a); m->removeAction (a->second);
m->addMenu (c->menu ()); m->addMenu (c->menu ());
} }
present_actions.erase (*a); present_actions.erase (*a);
@ -1128,10 +1152,10 @@ AbstractMenu::build (QMenu *m, std::list<AbstractMenuItem> &items)
} else { } else {
// Move the action to the end if present in the menu already // Move the action to the end if present in the menu already
std::set<QAction *>::iterator a = present_actions.find (c->action ().qaction ()); std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.find (std::make_pair (id_from_action (c->action ().qaction ()), c->action ().qaction ()));
if (a != present_actions.end ()) { if (a != present_actions.end ()) {
if (s_can_move_menu) { if (s_can_move_menu) {
m->removeAction (*a); m->removeAction (a->second);
m->addAction (c->action ().qaction ()); m->addAction (c->action ().qaction ());
} }
present_actions.erase (*a); present_actions.erase (*a);
@ -1143,8 +1167,8 @@ AbstractMenu::build (QMenu *m, std::list<AbstractMenuItem> &items)
} }
// Remove all actions that have vanished // Remove all actions that have vanished
for (std::set<QAction *>::iterator a = present_actions.begin (); a != present_actions.end (); ++a) { for (std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.begin (); a != present_actions.end (); ++a) {
m->removeAction (*a); m->removeAction (a->second);
} }
} }