Menu system: menu opening event, insert_menu variant with Action argument, clear_menu method, icon setter with QIcon

src/laybasic/laybasic/gsiDeclLayMenu.cc

@@ -36,7 +36,13 @@ public:
     if (triggered_cb.can_issue ()) {
       triggered_cb.issue<lay::Action> (&lay::Action::triggered);
     }
-    on_triggered_event ();
+  }
+
+  virtual void menu_opening ()
+  {
+    if (menu_opening_cb.can_issue ()) {
+      menu_opening_cb.issue<lay::Action> (&lay::Action::menu_opening);
+    }
   }
 
   virtual bool wants_visible () const
@@ -58,9 +64,9 @@ public:
   }
 
   gsi::Callback triggered_cb;
+  gsi::Callback menu_opening_cb;
   gsi::Callback wants_visible_cb;
   gsi::Callback wants_enabled_cb;
-  tl::Event on_triggered_event;
 };
 
 }
@@ -192,10 +198,29 @@ Class<lay::AbstractMenu> decl_AbstractMenu ("lay", "AbstractMenu",
     "@param name The name of the submenu to insert \n"
     "@param title The title of the submenu to insert\n"
   ) +
+  method ("insert_menu", (void (lay::AbstractMenu::*) (const std::string &, const std::string &, lay::Action *)) &lay::AbstractMenu::insert_menu, gsi::arg ("path"), gsi::arg ("name"), gsi::arg ("action"),
+    "@brief Inserts a new submenu before the item given by the path\n"
+    "\n"
+    "@param path The path to the item before which to insert the submenu\n"
+    "@param name The name of the submenu to insert \n"
+    "@param action The action object of the submenu to insert\n"
+    "\n"
+    "This method variant has been added in version 0.28."
+  ) +
+  method ("clear_menu", &lay::AbstractMenu::clear_menu, gsi::arg ("path"),
+    "@brief Deletes the children of the item given by the path\n"
+    "\n"
+    "@param path The path to the item whose children to delete\n"
+    "\n"
+    "This method has been introduced in version 0.28.\n"
+  ) +
   method ("delete_item", &lay::AbstractMenu::delete_item, gsi::arg ("path"),
     "@brief Deletes the item given by the path\n"
     "\n"
     "@param path The path to the item to delete\n"
+    "\n"
+    "This method will also delete all children of the given item. "
+    "To clear the children only, use \\clear_menu.\n"
   ) +
   method ("group", &lay::AbstractMenu::group, gsi::arg ("group"),
     "@brief Gets the group members\n"
@@ -360,6 +385,15 @@ Class<lay::Action> decl_ActionBase ("lay", "ActionBase",
     "\n"
     "Passing an empty string will reset the icon.\n"
   ) +
+#if defined(HAVE_QT) && defined(HAVE_QTBINDINGS)
+  method ("icon=", &lay::Action::set_qicon, gsi::arg ("qicon"),
+    "@brief Sets the icon to the given \\QIcon object\n"
+    "\n"
+    "@param qicon The QIcon object\n"
+    "\n"
+    "This variant has been added in version 0.28.\n"
+  ) +
+#endif
   method ("icon_text=", &lay::Action::set_icon_text, gsi::arg ("icon_text"),
     "@brief Sets the icon's text\n"
     "\n"
@@ -385,6 +419,18 @@ Class<lay::Action> decl_ActionBase ("lay", "ActionBase",
   ) +
   method ("trigger", &lay::Action::trigger,
     "@brief Triggers the action programmatically"
+  ) +
+  gsi::event ("on_triggered", &ActionStub::on_triggered_event,
+    "@brief This event is called if the menu item is selected.\n"
+    "\n"
+    "This event has been introduced in version 0.21 and moved to the ActionBase class in 0.28.\n"
+  ) +
+  gsi::event ("on_menu_opening", &ActionStub::on_menu_opening_event,
+    "@brief This event is called if the menu item is a sub-menu and before the menu is opened.\n"
+    "\n"
+    "This event provides an opportunity to populate the menu before it is opened.\n"
+    "\n"
+    "This event has been introduced in version 0.28.\n"
   ),
   "@hide\n"
   "@alias Action\n"
@@ -392,7 +438,18 @@ Class<lay::Action> decl_ActionBase ("lay", "ActionBase",
   
 Class<ActionStub> decl_Action (decl_ActionBase, "lay", "Action",
   gsi::callback ("triggered", &ActionStub::triggered, &ActionStub::triggered_cb,
-    "@brief This method is called if the menu item is selected"
+    "@brief This method is called if the menu item is selected.\n"
+    "\n"
+    "Reimplement this method is a derived class to receive this event. "
+    "You can also use the \\on_triggered event instead."
+  ) +
+  gsi::callback ("menu_opening", &ActionStub::menu_opening, &ActionStub::menu_opening_cb,
+    "@brief This method is called if the menu item is a sub-menu and before the menu is opened."
+    "\n"
+    "Reimplement this method is a derived class to receive this event. "
+    "You can also use the \\on_menu_opening event instead.\n"
+    "\n"
+    "This method has been added in version 0.28."
   ) +
   gsi::callback ("wants_visible", &ActionStub::wants_visible, &ActionStub::wants_visible_cb,
     "@brief Returns a value whether the action wants to become visible\n"
@@ -404,18 +461,13 @@ Class<ActionStub> decl_Action (decl_ActionBase, "lay", "Action",
     "This feature has been introduced in version 0.28.\n"
   ) +
   gsi::callback ("wants_enabled", &ActionStub::wants_enabled, &ActionStub::wants_enabled_cb,
-    "@brief Returns a value whether the action wants to become enabled\n"
+    "@brief Returns a value whether the action wants to become enabled.\n"
     "This is a dynamic query for enabled state which the system uses to dynamically show or hide "
     "menu items. This information is evaluated in addition "
     "to \\is_enabled? and contributes to the effective enabled status from "
     "\\is_effective_enabled?.\n"
     "\n"
     "This feature has been introduced in version 0.28.\n"
-  ) +
-  gsi::event ("on_triggered", &ActionStub::on_triggered_event,
-    "@brief This event is called if the menu item is selected\n"
-    "\n"
-    "This event has been introduced in version 0.21.\n"
   ),
   "@brief The abstraction for an action (i.e. used inside menus)\n"
   "\n"

src/laybasic/laybasic/layAbstractMenu.cc

@@ -541,8 +541,14 @@ Action::configure_from_title (const std::string &s)
 void
 Action::menu_about_to_show ()
 {
+  //  keeps a reference to self in case the action handler code removes actions
+  tl::shared_ptr<Action> self_holder (this);
+
   BEGIN_PROTECTED
 
+  on_menu_opening_event ();
+  menu_opening ();
+
   if (! mp_dispatcher || ! mp_dispatcher->menu ()) {
     return;
   }
@@ -565,7 +571,11 @@ Action::menu_about_to_show ()
 void
 Action::qaction_triggered ()
 {
+  //  keeps a reference to self in case the action handler code removes actions
+  tl::shared_ptr<Action> self_holder (this);
+
   BEGIN_PROTECTED
+  on_triggered_event ();
   triggered ();
   END_PROTECTED
 }
@@ -574,11 +584,15 @@ Action::qaction_triggered ()
 void
 Action::trigger ()
 {
+  //  keeps a reference to self in case the action handler code removes actions
+  tl::shared_ptr<Action> self_holder (this);
+
 #if defined(HAVE_QT)
   if (qaction ()) {
     qaction ()->trigger ();
   }
 #else
+  on_triggered_event ();
   triggered ();
 #endif
 }
@@ -589,6 +603,12 @@ Action::triggered ()
   //  .. no action yet, the reimplementation must provide some ..
 }
 
+void
+Action::menu_opening ()
+{
+  //  .. no action yet, the reimplementation must provide some ..
+}
+
 #if defined(HAVE_QT)
 QAction *
 Action::qaction () const
@@ -602,6 +622,76 @@ Action::menu () const
   return mp_menu;
 }
 
+static void
+configure_action (QAction *target, QAction *src)
+{
+  target->setShortcut (src->shortcut ());
+  target->setToolTip (src->toolTip ());
+  target->setCheckable (src->isCheckable ());
+  target->setChecked (src->isChecked ());
+  target->setEnabled (src->isEnabled ());
+  target->setIcon (src->icon ());
+  target->setIconText (src->iconText ());
+  target->setSeparator (src->isSeparator ());
+  target->setText (src->text ());
+  target->setVisible (src->isVisible ());
+}
+
+void
+Action::set_menu (QMenu *menu, bool owned)
+{
+  if (mp_menu == menu || ! lay::has_gui () || ! mp_action) {
+    return;
+  }
+
+  if (mp_menu && ! menu) {
+
+    QAction *new_action = new ActionObject (0);
+    configure_action (new_action, mp_action);
+
+    if (m_owned) {
+      delete mp_menu;
+    }
+    mp_menu = 0;
+
+    mp_action = new_action;
+    m_owned = true;
+
+  } else if (mp_menu && menu) {
+
+    configure_action (menu->menuAction (), mp_action);
+
+    if (m_owned) {
+      delete mp_menu;
+    }
+
+    mp_menu = menu;
+    m_owned = owned;
+    mp_action = menu->menuAction ();
+
+  } else if (! mp_menu && menu) {
+
+    configure_action (menu->menuAction (), mp_action);
+
+    if (m_owned) {
+      delete mp_action;
+    }
+
+    mp_menu = menu;
+    m_owned = owned;
+    mp_action = menu->menuAction ();
+
+  }
+
+  if (mp_menu) {
+    connect (mp_menu, SIGNAL (destroyed (QObject *)), this, SLOT (was_destroyed (QObject *)));
+    connect (mp_menu, SIGNAL (aboutToShow ()), this, SLOT (menu_about_to_show ()));
+  } else {
+    connect (mp_action, SIGNAL (destroyed (QObject *)), this, SLOT (was_destroyed (QObject *)));
+  }
+  connect (mp_action, SIGNAL (triggered ()), this, SLOT (qaction_triggered ()));
+}
+
 void
 Action::was_destroyed (QObject *obj)
 {
@@ -891,6 +981,17 @@ Action::set_icon (const std::string &filename)
   m_icon = filename;
 }
 
+#if defined(HAVE_QT)
+void
+Action::set_qicon (const QIcon &icon)
+{
+  if (qaction ()) {
+    qaction ()->setIcon (icon);
+  }
+  m_icon.clear ();
+}
+#endif
+
 std::string
 Action::get_tool_tip () const
 {
@@ -1072,13 +1173,10 @@ AbstractMenu::build_detached (const std::string &name, QFrame *mbar)
       menu_button->setText (tl::to_qstring (c->action ()->get_title ()));
 
       if (c->menu () == 0) {
-        QMenu *menu = new QMenu (mp_dispatcher->menu_parent_widget ());
+        c->set_menu (new QMenu (mp_dispatcher->menu_parent_widget ()), true);
-        menu_button->setMenu (menu);
-        c->set_action (new Action (menu), true);
-      } else {
-        menu_button->setMenu (c->menu ());
       }
 
+      menu_button->setMenu (c->menu ());
       build (c->menu (), c->children);
 
     } else {
@@ -1120,7 +1218,6 @@ static QAction *insert_action_after (QWidget *widget, QAction *after, QAction *a
 void
 AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
 {
-  m_helper_menu_items.clear ();
   if (tbar) {
     tbar->clear ();
   }
@@ -1153,14 +1250,15 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
 
         if (c->menu () == 0) {
           QMenu *menu = new QMenu (tl::to_qstring (c->action ()->get_title ()), mp_dispatcher->menu_parent_widget ());
-          // HINT: it is necessary to add the menu action to a widget below the main window.
+          c->action ()->set_menu (menu, true);
-          // Otherwise, the keyboard shortcuts do not work for menu items inside such a
+        }
-          // popup menu. It seems not to have a negative effect to add the menu to the
+
-          // main widget.
+        // HINT: it is necessary to add the menu action to a widget below the main window.
-          if (mp_dispatcher->menu_parent_widget ()) {
+        // Otherwise, the keyboard shortcuts do not work for menu items inside such a
-            mp_dispatcher->menu_parent_widget ()->addAction (menu->menuAction ());
+        // popup menu. It seems not to have a negative effect to add the menu to the
-          }
+        // main widget.
-          c->set_action (new Action (menu), true);
+        if (mp_dispatcher->menu_parent_widget ()) {
+          mp_dispatcher->menu_parent_widget ()->addAction (c->menu ()->menuAction ());
         }
 
         //  prepare a detached menu which can be used as context menus
@@ -1235,13 +1333,14 @@ AbstractMenu::build (QMenu *m, std::list<AbstractMenuItem> &items)
     if (c->has_submenu ()) {
 
       if (! c->menu ()) {
-        //  HINT: the action acts as a container for the title. Unfortunately, we cannot create a
+
-        //  menu with a given action. The action is provided by addMenu instead.
         QMenu *menu = new QMenu (mp_dispatcher->menu_parent_widget ());
         menu->setTitle (tl::to_qstring (c->action ()->get_title ()));
-        c->set_action (new Action (menu), true);
+        c->set_menu (menu, true);
         prev_action = insert_action_after (m, prev_action, menu->menuAction ());
+
       } else {
+
         //  Move the action to the end if present in the menu already
         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 ()) {
@@ -1254,6 +1353,7 @@ AbstractMenu::build (QMenu *m, std::list<AbstractMenuItem> &items)
         } else {
           prev_action = insert_action_after (m, prev_action, c->menu ()->menuAction ());
         }
+
       }
 
       build (c->menu (), c->children);
@@ -1288,23 +1388,16 @@ AbstractMenu::build (QToolBar *t, std::list<AbstractMenuItem> &items)
   for (std::list<AbstractMenuItem>::iterator c = items.begin (); c != items.end (); ++c) {
 
     if (! c->children.empty ()) {
-
       //  To support tool buttons with menu we have to attach a helper menu
       //  item to the QAction object.
-      //  TODO: this hurts if we use this QAction otherwise. In this case, this
+      if (! c->menu ()) {
-      //  QAction would get a menu too. However, hopefully this usage is constrained
+        c->set_menu (new QMenu (0), true);
-      //  to special toolbar buttons only.
+      }
-      //  In order to be able to manage the QMenu ourselves, we must not give it a parent.
+      build (c->menu (), c->children);
-      QMenu *menu = new QMenu (0);
-      m_helper_menu_items.push_back (menu); // will be owned by the stable vector
-      c->action ()->qaction ()->setMenu (menu);
-      t->addAction (c->action ()->qaction ());
-      build (menu, c->children);
-
-    } else {
-      t->addAction (c->action ()->qaction ());
     }
 
+    t->addAction (c->action ()->qaction ());
+
   }
 }
 
@@ -1437,6 +1530,10 @@ AbstractMenu::insert_separator (const std::string &p, const std::string &name)
 void
 AbstractMenu::insert_menu (const std::string &p, const std::string &name, Action *action)
 {
+  if (! action->menu ()) {
+    action->set_menu (new QMenu (), true);
+  }
+
   typedef std::vector<std::pair<AbstractMenuItem *, std::list<AbstractMenuItem>::iterator > > path_type;
   tl::Extractor extr (p.c_str ());
   path_type path = find_item (extr);

src/laybasic/laybasic/layAbstractMenu.h

@@ -271,6 +271,16 @@ public:
    */
   void set_icon (const std::string &filename);
 
+#if defined(HAVE_QT)
+  /**
+   *  @brief Sets the icon from a QIcon object
+   *
+   *  After using this function, "get_icon" will return an empty string as there
+   *  is no path for the icon file.
+   */
+  void set_qicon (const QIcon &icon);
+#endif
+
   /**
    *  @brief Set the icon's text
    *
@@ -306,10 +316,15 @@ public:
   void trigger ();
 
   /**
-   *  @brief Reimplement this method to implement a trigger handler
+   *  @brief Reimplement this method for a trigger handler
    */
   virtual void triggered ();
 
+  /**
+   *  @brief Reimplement this method for a handler called before a sub-menu is opening
+   */
+  virtual void menu_opening ();
+
   /**
    *  @brief Returns true, if the action is associated with a specific mode ID
    */
@@ -358,6 +373,11 @@ public:
    *  @brief Gets the QMenu object if the action is a menu action
    */
   QMenu *menu () const;
+
+  /**
+   *  @brief Sets the menu object
+   */
+  void set_menu (QMenu *menu, bool owned);
 #endif
 
   /**
@@ -368,6 +388,16 @@ public:
     return mp_dispatcher;
   }
 
+  /**
+   *  @brief This event is called when the action is triggered
+   */
+  tl::Event on_triggered_event;
+
+  /**
+   *  @brief This event gets called when the action is a sub-menu and the menu is opened
+   */
+  tl::Event on_menu_opening_event;
+
 #if defined(HAVE_QT)
 protected slots:
   void was_destroyed (QObject *obj);
@@ -564,6 +594,11 @@ struct LAYBASIC_PUBLIC AbstractMenuItem
   {
     return mp_action->menu ();
   }
+
+  void set_menu (QMenu *menu, bool owned)
+  {
+    return mp_action->set_menu (menu, owned);
+  }
 #endif
 
   void set_has_submenu ();
@@ -888,7 +923,6 @@ private:
   Dispatcher *mp_dispatcher;
   AbstractMenuItem m_root;
 #if defined(HAVE_QT)
-  tl::stable_vector<QMenu> m_helper_menu_items;
   std::map<std::string, QActionGroup *> m_action_groups;
 #endif
   std::map<std::string, std::vector<ConfigureAction *> > m_config_action_by_name;

testdata/ruby/layMenuTest.rb

@@ -440,47 +440,99 @@ RESULT
 
   end
 
-  class MyAction < RBA::Action
+  if RBA.constants.member?(:Action)
-    attr_accessor :dyn_visible, :dyn_enabled
+
-    def initialize
+    class MyAction < RBA::Action
-      self.dyn_visible = true
+      attr_accessor :dyn_visible, :dyn_enabled
-      self.dyn_enabled = true
+      def initialize
+        self.dyn_visible = true
+        self.dyn_enabled = true
+      end
+      def wants_visible
+        self.dyn_visible
+      end
+      def wants_enabled
+        self.dyn_enabled
+      end
     end
-    def wants_visible
+
-      self.dyn_visible
+    def test_7
-    end
+
-    def wants_enabled
+      a = MyAction::new
-      self.dyn_enabled
+
+      assert_equal(a.is_effective_visible?, true)
+      a.hidden = true
+      assert_equal(a.is_effective_visible?, false)
+      a.hidden = false
+      assert_equal(a.is_effective_visible?, true)
+      a.visible = false
+      assert_equal(a.is_effective_visible?, false)
+      a.visible = true
+      assert_equal(a.is_effective_visible?, true)
+      a.dyn_visible = false
+      assert_equal(a.is_effective_visible?, false)
+      a.dyn_visible = true
+      assert_equal(a.is_effective_visible?, true)
+
+      assert_equal(a.is_effective_enabled?, true)
+      a.enabled = false
+      assert_equal(a.is_effective_enabled?, false)
+      a.enabled = true
+      assert_equal(a.is_effective_enabled?, true)
+      a.dyn_enabled = false
+      assert_equal(a.is_effective_enabled?, false)
+      a.dyn_enabled = true
+      assert_equal(a.is_effective_enabled?, true)
+
     end
+
   end
 
-  def test_7
+  def test_8
 
-    a = MyAction::new
+    if !RBA.constants.member?(:Action)
+      return
+    end
 
-    assert_equal(a.is_effective_visible?, true)
+    action = RBA::Action::new
-    a.hidden = true
+    action.title = "title:n1"
-    assert_equal(a.is_effective_visible?, false)
-    a.hidden = false
-    assert_equal(a.is_effective_visible?, true)
-    a.visible = false
-    assert_equal(a.is_effective_visible?, false)
-    a.visible = true
-    assert_equal(a.is_effective_visible?, true)
-    a.dyn_visible = false
-    assert_equal(a.is_effective_visible?, false)
-    a.dyn_visible = true
-    assert_equal(a.is_effective_visible?, true)
 
-    assert_equal(a.is_effective_enabled?, true)
+    menu = RBA::AbstractMenu::new
-    a.enabled = false
+
-    assert_equal(a.is_effective_enabled?, false)
+    assert_equal(menu.action("s1.n1"), nil)
-    a.enabled = true
+    assert_equal(menu.action("s1"), nil)
-    assert_equal(a.is_effective_enabled?, true)
+
-    a.dyn_enabled = false
+    menu.insert_menu("end", "s1", "submenu1")
-    assert_equal(a.is_effective_enabled?, false)
+    menu.insert_menu("end", "s2", "submenu2")
-    a.dyn_enabled = true
+
-    assert_equal(a.is_effective_enabled?, true)
+    menu.insert_item("s1.end", "n1", action)
+    menu.insert_item("s1.end", "n2", action)
+    menu.insert_item("s2.end", "n1", action)
+
+    assert_equal(menu.action("s1.n1") == action, true)
+    assert_equal(menu.action("s1.n2") == action, true)
+    assert_equal(menu.action("s2.n1") == action, true)
+
+    assert_equal(menu.is_valid?("s1.n1"), true)
+    assert_equal(menu.is_valid?("s1.n2"), true)
+    assert_equal(menu.is_valid?("s2.n1"), true)
+
+    menu.clear_menu("s1")
+
+    assert_equal(menu.is_valid?("s1.n1"), false)
+    assert_equal(menu.is_valid?("s1.n2"), false)
+    assert_equal(menu.is_valid?("s2.n1"), true)
+
+    menu.clear_menu("s2")
+
+    assert_equal(menu.is_valid?("s1.n1"), false)
+    assert_equal(menu.is_valid?("s1.n2"), false)
+    assert_equal(menu.is_valid?("s2.n1"), false)
+
+    # proof of transfer of ownership
+    assert_equal(action._destroyed?, true)
+
+    menu._destroy
 
   end