From 6a762db29b8fe50409f639beffa1f3d9163409a3 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 10 Feb 2018 18:40:24 +0100 Subject: [PATCH] Reworked bookmark menu implementation to avoid MacOS menu issue Without this commit, bookmark menus got grayed out on MacOS with Qt5 sometimes. Now, the implementation of recent file menu, bookmark menu, macro menu and the static main menu use the same framewkork which includes a workaround for the disabled menu issue. --- src/lay/lay/layMainWindow.cc | 85 +++------------------- src/lay/lay/layMainWindow.h | 5 +- src/laybasic/laybasic/layAbstractMenu.cc | 11 +++ src/laybasic/laybasic/layAbstractMenu.h | 7 ++ src/laybasic/laybasic/layLayoutView.cc | 68 +++++++++++++++++ src/laybasic/laybasic/layLayoutView.h | 17 +++++ src/laybasic/unit_tests/layAbstractMenu.cc | 11 ++- 7 files changed, 127 insertions(+), 77 deletions(-) diff --git a/src/lay/lay/layMainWindow.cc b/src/lay/lay/layMainWindow.cc index dd2d594f0..37be91928 100644 --- a/src/lay/lay/layMainWindow.cc +++ b/src/lay/lay/layMainWindow.cc @@ -609,11 +609,6 @@ MainWindow::MainWindow (QApplication *app, const char *name) cp_frame_ly->addWidget (mp_cpy_label); cp_frame_ly->insertSpacing (-1, 6); - // connect to the menus to provide the dynamic parts - QMenu *bookmark_menu = mp_menu->menu ("bookmark_menu"); - tl_assert (bookmark_menu != 0); - connect (bookmark_menu, SIGNAL (aboutToShow ()), this, SLOT (bookmark_menu_show ())); - // select the default mode select_mode (lay::LayoutView::default_mode ()); @@ -1798,6 +1793,12 @@ MainWindow::edits_enabled_changed () } } +void +MainWindow::menu_needs_update () +{ + lay::LayoutView::update_menu (current_view (), *mp_menu); +} + void MainWindow::libraries_changed () { @@ -2426,57 +2427,6 @@ MainWindow::cm_redo () END_PROTECTED } -void -MainWindow::bookmark_menu_show () -{ - if (mp_menu->is_valid ("bookmark_menu.goto_bookmark_menu")) { - - Action goto_bookmark_action = mp_menu->action ("bookmark_menu.goto_bookmark_menu"); - bool has_bookmarks = current_view () && current_view ()->bookmarks ().size () > 0; - - if (has_bookmarks && edits_enabled ()) { - - goto_bookmark_action.set_enabled (true); - - QMenu *goto_bookmark_menu = goto_bookmark_action.qaction ()->menu (); - if (goto_bookmark_menu) { - - goto_bookmark_menu->clear (); - - if (current_view ()) { - const lay::BookmarkList &bookmarks = current_view ()->bookmarks (); - for (size_t i = 0; i < bookmarks.size (); ++i) { - QAction *action = goto_bookmark_menu->addAction (tl::to_qstring (bookmarks.name (i))); - action->setObjectName (tl::to_qstring (tl::sprintf ("bookmark_%d", i + 1))); - gtf::action_connect (action, SIGNAL (triggered ()), this, SLOT (goto_bookmark ())); - action->setData (QVariant (int (i))); - } - } - - } - - } else { - goto_bookmark_action.set_enabled (false); - } - - } -} - -void -MainWindow::goto_bookmark () -{ - BEGIN_PROTECTED - - QAction *action = dynamic_cast (sender ()); - tl_assert (action); - size_t id = size_t (action->data ().toInt ()); - if (current_view () && current_view ()->bookmarks ().size () > id) { - current_view ()->goto_view (current_view ()->bookmarks ().state (id)); - } - - END_PROTECTED -} - void MainWindow::cm_goto_position () { @@ -2580,19 +2530,7 @@ MainWindow::cm_bookmark_view () BEGIN_PROTECTED if (current_view ()) { - while (true) { - bool ok = false; - QString text = QInputDialog::getText (this, QObject::tr ("Enter Bookmark Name"), QObject::tr ("Bookmark name"), - QLineEdit::Normal, QString::null, &ok); - if (! ok) { - break; - } else if (text.isEmpty ()) { - QMessageBox::critical (this, QObject::tr ("Error"), QObject::tr ("Enter a name for the bookmark")); - } else { - current_view ()->bookmark_view (tl::to_string (text)); - break; - } - } + current_view ()->bookmark_current_view (); } END_PROTECTED @@ -3359,6 +3297,7 @@ MainWindow::select_view (int index) clear_current_pos (); edits_enabled_changed (); clear_message (); + menu_needs_update (); m_disable_tab_selected = dis; @@ -3738,6 +3677,7 @@ MainWindow::clone_current_view () connect (view, SIGNAL (title_changed ()), this, SLOT (view_title_changed ())); connect (view, SIGNAL (dirty_changed ()), this, SLOT (view_title_changed ())); connect (view, SIGNAL (edits_enabled_changed ()), this, SLOT (edits_enabled_changed ())); + connect (view, SIGNAL (menu_needs_update ()), this, SLOT (menu_needs_update ())); connect (view, SIGNAL (show_message (const std::string &, int)), this, SLOT (message (const std::string &, int))); connect (view, SIGNAL (current_pos_changed (double, double, bool)), this, SLOT (current_pos (double, double, bool))); connect (view, SIGNAL (clear_current_pos ()), this, SLOT (clear_current_pos ())); @@ -4041,6 +3981,7 @@ MainWindow::close_view (int index) clear_current_pos (); edits_enabled_changed (); + menu_needs_update (); clear_message (); update_dock_widget_state (); @@ -4184,10 +4125,7 @@ MainWindow::do_update_file_menu () if (m_mru.size () > 0 && edits_enabled ()) { // rebuild MRU menu - std::vector items = mp_menu->items (mru_menu); - for (std::vector::const_iterator i = items.begin (); i != items.end (); ++i) { - mp_menu->delete_item (mru_menu + "." + *i); - } + mp_menu->clear_menu (mru_menu); for (std::vector >::iterator mru = m_mru.end (); mru != m_mru.begin (); ) { --mru; @@ -4349,6 +4287,7 @@ MainWindow::do_create_view () connect (view, SIGNAL (title_changed ()), this, SLOT (view_title_changed ())); connect (view, SIGNAL (dirty_changed ()), this, SLOT (view_title_changed ())); connect (view, SIGNAL (edits_enabled_changed ()), this, SLOT (edits_enabled_changed ())); + connect (view, SIGNAL (menu_needs_update ()), this, SLOT (menu_needs_update ())); connect (view, SIGNAL (show_message (const std::string &, int)), this, SLOT (message (const std::string &, int))); connect (view, SIGNAL (current_pos_changed (double, double, bool)), this, SLOT (current_pos (double, double, bool))); connect (view, SIGNAL (clear_current_pos ()), this, SLOT (clear_current_pos ())); diff --git a/src/lay/lay/layMainWindow.h b/src/lay/lay/layMainWindow.h index 8e9c68a42..092ac57a4 100644 --- a/src/lay/lay/layMainWindow.h +++ b/src/lay/lay/layMainWindow.h @@ -689,7 +689,6 @@ public slots: void tab_close_requested (int); void enable_all (); void disable_all (); - void goto_bookmark (); void open_recent (); void view_selected (int index); void view_title_changed (); @@ -858,8 +857,9 @@ public slots: protected slots: void menu_changed (); void message_timer (); - void bookmark_menu_show (); void edits_enabled_changed (); + void menu_needs_update (); + void file_changed_timer (); void file_changed (const QString &path); void file_removed (const QString &path); @@ -874,7 +874,6 @@ private: // Main menu AbstractMenu *mp_menu; - QMenu *mp_goto_bookmark_menu; QTabBar *mp_tab_bar; QToolBar *mp_tool_bar; QDockWidget *mp_navigator_dock_widget; diff --git a/src/laybasic/laybasic/layAbstractMenu.cc b/src/laybasic/laybasic/layAbstractMenu.cc index dc6c62f79..dccd1ea33 100644 --- a/src/laybasic/laybasic/layAbstractMenu.cc +++ b/src/laybasic/laybasic/layAbstractMenu.cc @@ -1354,6 +1354,17 @@ AbstractMenu::insert_menu (const std::string &path, const std::string &name, con insert_menu (path, name, create_action (title)); } +void +AbstractMenu::clear_menu (const std::string &p) +{ + typedef std::vector::iterator > > path_type; + path_type path = find_item (p); + if (! path.empty () && ! path.back ().second->children.empty ()) { + path.back ().second->children.clear (); + emit changed (); + } +} + void AbstractMenu::delete_item (const std::string &p) { diff --git a/src/laybasic/laybasic/layAbstractMenu.h b/src/laybasic/laybasic/layAbstractMenu.h index f86c3a674..c5a6ade53 100644 --- a/src/laybasic/laybasic/layAbstractMenu.h +++ b/src/laybasic/laybasic/layAbstractMenu.h @@ -784,6 +784,13 @@ public: */ void insert_menu (const std::string &path, const std::string &name, const std::string &title); + /** + * @brief Deletes the children of the item with the given path + * + * If the item does not exist or is not a menu, this method does nothing. + */ + void clear_menu (const std::string &path); + /** * @brief Delete the item given by the path * diff --git a/src/laybasic/laybasic/layLayoutView.cc b/src/laybasic/laybasic/layLayoutView.cc index d4f291c5a..7a4c2b5c6 100644 --- a/src/laybasic/laybasic/layLayoutView.cc +++ b/src/laybasic/laybasic/layLayoutView.cc @@ -42,6 +42,7 @@ #include "tlString.h" #include "tlLog.h" #include "tlAssert.h" +#include "tlExceptions.h" #include "layLayoutView.h" #include "layViewOp.h" #include "layViewObject.h" @@ -74,6 +75,7 @@ #include "rdbMarkerBrowserDialog.h" #include "tlXMLParser.h" #include "gsi.h" +#include "gtf.h" #include @@ -772,6 +774,37 @@ LayoutView::init_menu (lay::AbstractMenu &menu) lay::HierarchyControlPanel::init_menu (menu); } +void +LayoutView::update_menu (lay::LayoutView *view, lay::AbstractMenu &menu) +{ + std::string bm_menu = "bookmark_menu.goto_bookmark_menu"; + + if (menu.is_valid (bm_menu)) { + + menu.clear_menu (bm_menu); + + Action goto_bookmark_action = menu.action (bm_menu); + + if (view && view->bookmarks ().size () > 0) { + + goto_bookmark_action.set_enabled (true); + + const lay::BookmarkList &bookmarks = view->bookmarks (); + for (size_t i = 0; i < bookmarks.size (); ++i) { + Action action; + gtf::action_connect (action.qaction (), SIGNAL (triggered ()), view, SLOT (goto_bookmark ())); + action.set_title (bookmarks.name (i)); + action.qaction ()->setData (QVariant (int (i))); + menu.insert_item (bm_menu + ".end", tl::sprintf ("bookmark_%d", i + 1), action); + } + + } else { + goto_bookmark_action.set_enabled (false); + } + + } +} + void LayoutView::set_drawing_workers (int workers) { @@ -3650,6 +3683,24 @@ LayoutView::cancel () clear_selection (); } +void +LayoutView::bookmark_current_view () +{ + while (true) { + bool ok = false; + QString text = QInputDialog::getText (this, QObject::tr ("Enter Bookmark Name"), QObject::tr ("Bookmark name"), + QLineEdit::Normal, QString::null, &ok); + if (! ok) { + break; + } else if (text.isEmpty ()) { + QMessageBox::critical (this, QObject::tr ("Error"), QObject::tr ("Enter a name for the bookmark")); + } else { + bookmark_view (tl::to_string (text)); + break; + } + } +} + void LayoutView::manage_bookmarks () { @@ -3663,6 +3714,7 @@ void LayoutView::bookmarks (const BookmarkList &b) { m_bookmarks = b; + emit menu_needs_update (); } void @@ -3670,6 +3722,22 @@ LayoutView::bookmark_view (const std::string &name) { DisplayState state (box (), get_min_hier_levels (), get_max_hier_levels (), m_cellviews); m_bookmarks.add (name, state); + emit menu_needs_update (); +} + +void +LayoutView::goto_bookmark () +{ + BEGIN_PROTECTED + + QAction *action = dynamic_cast (sender ()); + tl_assert (action); + size_t id = size_t (action->data ().toInt ()); + if (bookmarks ().size () > id) { + goto_view (bookmarks ().state (id)); + } + + END_PROTECTED } void diff --git a/src/laybasic/laybasic/layLayoutView.h b/src/laybasic/laybasic/layLayoutView.h index 0cc3d63ff..5d5d2e4c3 100644 --- a/src/laybasic/laybasic/layLayoutView.h +++ b/src/laybasic/laybasic/layLayoutView.h @@ -1557,6 +1557,11 @@ public: */ void bookmark_view (const std::string &name); + /** + * @brief Asks for a bookmark name and bookmark the current view under this name + */ + void bookmark_current_view (); + /** * @brief Show the bookmark management form */ @@ -1603,6 +1608,12 @@ public: */ static void init_menu (lay::AbstractMenu &menu); + /** + * @brief Updates the menu for the given view + * If the view is 0, the menu shall be updated to reflect "no view active" + */ + static void update_menu (lay::LayoutView *view, lay::AbstractMenu &menu); + /** * @brief Query the default mode */ @@ -2530,6 +2541,7 @@ public slots: private slots: void active_cellview_changed (int index); + void goto_bookmark (); signals: /** @@ -2562,6 +2574,11 @@ signals: */ void edits_enabled_changed (); + /** + * @brief This signal is sent when the view wants to update the menu + */ + void menu_needs_update (); + protected: /** * @brief Establish the view operations diff --git a/src/laybasic/unit_tests/layAbstractMenu.cc b/src/laybasic/unit_tests/layAbstractMenu.cc index 5edf78f8f..46bd3b3ff 100644 --- a/src/laybasic/unit_tests/layAbstractMenu.cc +++ b/src/laybasic/unit_tests/layAbstractMenu.cc @@ -124,5 +124,14 @@ TEST(1) menu.delete_item ("n1.c2"); EXPECT_EQ (menu_to_string (menu), "(n2)"); -} + menu.clear_menu ("n1"); + EXPECT_EQ (menu_to_string (menu), "(n2)"); + + menu.insert_menu ("end", "n1", lay::Action ("title:n1")); + menu.insert_item ("n1.begin", "c1", lay::Action ("title:c1")); + menu.insert_item ("n1.end", "c2", lay::Action ("title:c2")); + EXPECT_EQ (menu_to_string (menu), "(n2,n1(n1.c1,n1.c2))"); + menu.clear_menu ("n1"); + EXPECT_EQ (menu_to_string (menu), "(n2,n1)"); +}