ContextMenu: Harmonize with menu bar

3 files changed, 148 insertions, 97 deletions

diff --git a/src/ui/contextmenu.cpp b/src/ui/contextmenu.cpp
index a3e9e9e78..0139f2444 100644
--- a/src/ui/contextmenu.cpp
+++ b/src/ui/contextmenu.cpp
@@ -25,14 +25,13 @@
 #include <glibmm/miscutils.h>
 
 #include <gtkmm/box.h>
+#include <gtkmm/cssprovider.h>
 #include <gtkmm/image.h>
 #include <gtkmm/separatormenuitem.h>
 
 #include "desktop.h"
 #include "document.h"
 #include "document-undo.h"
-#include "helper/action.h"
-#include "helper/action-context.h"
 #include "inkscape.h"
 #include "message-context.h"
 #include "message-stack.h"
@@ -40,6 +39,12 @@
 #include "selection-chemistry.h"
 #include "shortcuts.h"
 
+#include "helper/action.h"
+#include "helper/action-context.h"
+#include "helper/icon-loader.h"
+
+#include "include/gtkmm_version.h"
+
 #include "object/sp-anchor.h"
 #include "object/sp-clippath.h"
 #include "object/sp-image.h"
@@ -58,20 +63,23 @@ ContextMenu::ContextMenu(SPDesktop *desktop, SPItem *item) :
     MIGroup(),
     MIParent(_("Go to parent"))
 {
-// g_message("ContextMenu");
     _object = static_cast<SPObject *>(item);
     _desktop = desktop;
 
-    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_UNDO));
-    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_REDO));
+    Inkscape::Preferences *prefs = Inkscape::Preferences::get();
+    int show_icons_pref = prefs->getInt("/theme/menuIcons", 0);
+    bool show_icon = show_icons_pref >= 0 ? true : false;
+
+    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_UNDO), show_icon);
+    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_REDO), show_icon);
     AddSeparator();
-    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_CUT));
-    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_COPY));
-    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_PASTE));
+    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_CUT), show_icon);
+    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_COPY), show_icon);
+    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_PASTE), show_icon);
     AddSeparator();
-    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_DUPLICATE));
-    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_DELETE));
-    
+    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_DUPLICATE), show_icon);
+    AppendItemFromVerb(Inkscape::Verb::get(SP_VERB_EDIT_DELETE), show_icon);
+
     positionOfLastDialog = 10; // 9 in front + 1 for the separator in the next if; used to position the dialog menu entries below each other
     /* Item menu */
     if (item!=nullptr) {
@@ -162,6 +170,8 @@ ContextMenu::ContextMenu(SPDesktop *desktop, SPItem *item) :
             append(*miu);
         }
     }
+
+    signal_map().connect(sigc::mem_fun(*this, &ContextMenu::ShiftIcons));
 }
 
 ContextMenu::~ContextMenu(void)
@@ -186,7 +196,7 @@ void ContextMenu::LeaveGroup()
     _desktop->setCurrentLayer(_desktop->currentLayer()->parent);
 }
 
-void ContextMenu::LockSelected() 
+void ContextMenu::LockSelected()
 {
     auto itemlist = _desktop->selection->items();
     for(auto i=itemlist.begin();i!=itemlist.end(); ++i) {
@@ -253,11 +263,11 @@ context_menu_item_on_my_deselect(void */*object*/, SPAction *action)
 
 
 // TODO: Update this to allow radio items to be used
-void ContextMenu::AppendItemFromVerb(Inkscape::Verb *verb)
+void ContextMenu::AppendItemFromVerb(Inkscape::Verb *verb, bool show_icon)
 {
     SPAction *action;
     SPDesktop *view = _desktop;
-    
+
     if (verb->get_code() == SP_VERB_NONE) {
         Gtk::MenuItem *item = AddSeparator();
         item->show();
@@ -267,10 +277,33 @@ void ContextMenu::AppendItemFromVerb(Inkscape::Verb *verb)
         if (!action) {
             return;
         }
-        
-        auto const item = Gtk::manage(new Gtk::MenuItem(action->name, true));
-
+        // Create the menu item itself
+        auto const item = Gtk::manage(new Gtk::MenuItem());
+
+        // Now create the label and add it to the menu item (with mnemonic)
+        auto const label = Gtk::manage(new Gtk::AccelLabel(action->name, true));
+#if GTKMM_CHECK_VERSION(3,16,0)
+        label->set_xalign(0.0);
+#else
+        label->set_alignment(0.0, 0.5);
+#endif
         sp_shortcut_add_accelerator(GTK_WIDGET(item->gobj()), sp_shortcut_get_primary(verb));
+        label->set_accel_widget(*item);
+
+        // If there is an image associated with the action, then we can add it as an icon for the menu item
+        if (show_icon && action->image) {
+            item->set_name("ImageMenuItem");  // custom name to identify our "ImageMenuItems"
+            auto const icon = Gtk::manage(sp_get_icon_image(action->image, Gtk::ICON_SIZE_MENU));
+
+            // create a box to hold icon and label as GtkMenuItem derives from GtkBin and can only hold one child
+            auto const box = Gtk::manage(new Gtk::Box(Gtk::ORIENTATION_HORIZONTAL));
+            box->pack_start(*icon, false, false, 0);
+            box->pack_start(*label, true, true, 0);
+
+            item->add(*box);
+        } else {
+            item->add(*label);
+        }
 
         action->signal_set_sensitive.connect(sigc::mem_fun(*this, &ContextMenu::set_sensitive));
         action->signal_set_name.connect(sigc::mem_fun(*item, &ContextMenu::set_name));
@@ -284,6 +317,7 @@ void ContextMenu::AppendItemFromVerb(Inkscape::Verb *verb)
         item->signal_select().connect(sigc::bind(sigc::ptr_fun(context_menu_item_on_my_select),item,action));
         item->signal_deselect().connect(sigc::bind(sigc::ptr_fun(context_menu_item_on_my_deselect),item,action));
         item->show_all();
+
         append(*item);
     }
 }
@@ -326,7 +360,7 @@ void ContextMenu::MakeItemMenu ()
     append(*mi);//insert(*mi,positionOfLastDialog++);
 
     AddSeparator();
-    
+
     /* Select item */
     if (Inkscape::Verb::getbyid( "org.inkscape.followlink" )) {
         mi = Gtk::manage(new Gtk::MenuItem(_("_Select This"), true));
@@ -400,7 +434,7 @@ void ContextMenu::MakeItemMenu ()
     mi->set_sensitive(!SP_IS_ANCHOR(_item));
     mi->show();
     append(*mi);
-    
+
     bool ClipRefOK=false;
     bool MaskRefOK=false;
     if (_item){
@@ -409,14 +443,14 @@ void ContextMenu::MakeItemMenu ()
                 ClipRefOK=true;
             }
         }
-    }    
+    }
     if (_item){
         if (_item->mask_ref){
             if (_item->mask_ref->getObject()){
                 MaskRefOK=true;
             }
         }
-    }    
+    }
     /* Set mask */
     mi = Gtk::manage(new Gtk::MenuItem(_("Set Mask"), true));
     mi->signal_activate().connect(sigc::mem_fun(*this, &ContextMenu::SetMask));
@@ -427,7 +461,7 @@ void ContextMenu::MakeItemMenu ()
     }
     mi->show();
     append(*mi);
-    
+
     /* Release mask */
     mi = Gtk::manage(new Gtk::MenuItem(_("Release Mask"), true));
     mi->signal_activate().connect(sigc::mem_fun(*this, &ContextMenu::ReleaseMask));
@@ -445,7 +479,7 @@ void ContextMenu::MakeItemMenu ()
     mi->set_sensitive(TRUE);
     mi->show();
     append(*mi);
-    
+
     /* Set Clip */
     mi = Gtk::manage(new Gtk::MenuItem(_("Set Cl_ip"), true));
     mi->signal_activate().connect(sigc::mem_fun(*this, &ContextMenu::SetClip));
@@ -456,7 +490,7 @@ void ContextMenu::MakeItemMenu ()
     }
     mi->show();
     append(*mi);
-    
+
     /* Release Clip */
     mi = Gtk::manage(new Gtk::MenuItem(_("Release C_lip"), true));
     mi->signal_activate().connect(sigc::mem_fun(*this, &ContextMenu::ReleaseClip));
@@ -593,7 +627,7 @@ void ContextMenu::ActivateUngroup()
     sp_item_group_ungroup(static_cast<SPGroup*>(_item), children);
     _desktop->selection->setList(children);
 }
- 
+
 void ContextMenu::ActivateUngroupPopSelection()
 {
     _desktop->selection->popFromGroup();
@@ -603,19 +637,19 @@ void ContextMenu::ActivateUngroupPopSelection()
 void ContextMenu::MakeAnchorMenu()
 {
     Gtk::MenuItem* mi;
-    
+
     /* Link dialog */
     mi = Gtk::manage(new Gtk::MenuItem(_("Link _Properties..."), true));
     mi->signal_activate().connect(sigc::mem_fun(*this, &ContextMenu::AnchorLinkProperties));
     mi->show();
     insert(*mi,positionOfLastDialog++);
-    
+
     /* Select item */
     mi = Gtk::manage(new Gtk::MenuItem(_("_Follow Link"), true));
     mi->signal_activate().connect(sigc::mem_fun(*this, &ContextMenu::AnchorLinkFollow));
     mi->show();
     append(*mi);
-    
+
     /* Reset transformations */
     mi = Gtk::manage(new Gtk::MenuItem(_("_Remove Link"), true));
     mi->signal_activate().connect(sigc::mem_fun(*this, &ContextMenu::AnchorLinkRemove));
@@ -778,7 +812,7 @@ void ContextMenu::ImageEdit()
     for(auto i=itemlist.begin();i!=itemlist.end();++i){
         Inkscape::XML::Node *ir = (*i)->getRepr();
         const gchar *href = ir->attribute("xlink:href");
-        
+
         if (strncmp (href,"file:",5) == 0) {
         // URI to filename conversion
           name = g_filename_from_uri(href, nullptr, nullptr);
@@ -794,7 +828,7 @@ void ContextMenu::ImageEdit()
             fullname = Glib::build_filename(Glib::get_current_dir(), name);
         }
         if (name.substr(name.find_last_of(".") + 1) == "SVG" ||
-            name.substr(name.find_last_of(".") + 1) == "svg"   ) 
+            name.substr(name.find_last_of(".") + 1) == "svg"   )
         {
             cmdline.erase(0, bmpeditor.length());
             Glib::ustring svgeditor = getImageEditorName(true);
@@ -807,7 +841,7 @@ void ContextMenu::ImageEdit()
 
     //g_warning("##Command line: %s\n", cmdline.c_str());
 
-    g_spawn_command_line_async(cmdline.c_str(), &errThing); 
+    g_spawn_command_line_async(cmdline.c_str(), &errThing);
 
     if ( errThing ) {
         g_warning("Problem launching editor (%d). %s", errThing->code, errThing->message);
@@ -862,7 +896,7 @@ void ContextMenu::ImageExtract()
 void ContextMenu::MakeShapeMenu ()
 {
     Gtk::MenuItem* mi;
-    
+
     /* Item dialog */
     mi = Gtk::manage(new Gtk::MenuItem(_("_Fill and Stroke..."), true));
     mi->signal_activate().connect(sigc::mem_fun(*this, &ContextMenu::FillSettings));
@@ -888,7 +922,7 @@ void ContextMenu::MakeTextMenu ()
     mi->signal_activate().connect(sigc::mem_fun(*this, &ContextMenu::FillSettings));
     mi->show();
     insert(*mi,positionOfLastDialog++);
-    
+
     /* Edit Text dialog */
     mi = Gtk::manage(new Gtk::MenuItem(_("_Text and Font..."), true));
     mi->signal_activate().connect(sigc::mem_fun(*this, &ContextMenu::TextSettings));
@@ -919,6 +953,64 @@ void ContextMenu::SpellcheckSettings ()
 
     _desktop->_dlg_mgr->showDialog("SpellCheck");
 }
+
+void ContextMenu::ShiftIcons()
+{
+    static auto provider = Gtk::CssProvider::create();
+    static bool provider_added = false;
+
+    Gtk::MenuItem *menuitem = nullptr;
+    Gtk::Box *content = nullptr;
+    Gtk::Image *icon = nullptr;
+
+    static int current_shift = 0;
+    int calculated_shift = 0;
+
+    // install CssProvider for our custom styles
+    if (!provider_added) {
+        auto const screen = Gdk::Screen::get_default();
+        Gtk::StyleContext::add_provider_for_screen(screen, provider, GTK_STYLE_PROVIDER_PRIORITY_APPLICATION);
+        provider_added = true;
+    }
+
+    // get the first MenuItem with an image (i.e. "ImageMenuItem" as named below)
+    std::vector<Gtk::Widget *> children = get_children();
+    for (auto child: children) {
+        if (child->get_name() == "ImageMenuItem") {
+            menuitem = static_cast<Gtk::MenuItem *>(child);
+            content = static_cast<Gtk::Box *>(menuitem->get_child());
+            icon = static_cast<Gtk::Image *>(content->get_children()[0]);
+            break;
+        }
+    }
+
+    // calculate how far we have to shift the icon to fit it into the empty space between menuitem and its content
+    if (icon) {
+        auto allocation_menuitem = menuitem->get_allocation();
+        auto allocation_icon = icon->get_allocation();
+
+        if (menuitem->get_direction() == Gtk::TEXT_DIR_RTL) {
+            calculated_shift = allocation_menuitem.get_width() - allocation_icon.get_x() - allocation_icon.get_width();
+        } else {
+            calculated_shift = -allocation_icon.get_x();
+        }
+    }
+
+    // install CSS to shift icon, use a threshold to avoid overly frequent updates
+    // (gtk's own calculations for the reserved space are off by a few pixels if there is no check/radio item in a menu)
+    if (calculated_shift && std::abs(current_shift - calculated_shift) > 2) {
+        current_shift = calculated_shift;
+
+        std::string css_str;
+        if (menuitem->get_direction() == Gtk::TEXT_DIR_RTL) {
+            css_str = "#ImageMenuItem image {margin-right:" + std::to_string(-calculated_shift) + "px;}";
+        } else {
+            css_str = "#ImageMenuItem image {margin-left:" + std::to_string(calculated_shift) + "px;}";
+        }
+        provider->load_from_data(css_str);
+    }
+}
+
 /*
   Local Variables:
   mode:c++
diff --git a/src/ui/contextmenu.h b/src/ui/contextmenu.h
index bd58fde00..a940e7f27 100644
--- a/src/ui/contextmenu.h
+++ b/src/ui/contextmenu.h
@@ -35,7 +35,7 @@ class Verb;
  * Implements the Inkscape context menu.
  * 
  * For the context menu implementation, the ContextMenu class stores the object
- * that was selected in a private data member. This should be farely safe to do
+ * that was selected in a private data member. This should be fairly safe to do
  * and a pointer to the SPItem as well as SPObject class are kept.
  * All callbacks of the context menu entries are implemented as private
  * functions.
@@ -54,7 +54,14 @@ class ContextMenu : public Gtk::Menu
          */
         ContextMenu(SPDesktop *desktop, SPItem *item);
         ~ContextMenu() override;
-        
+
+        /**
+         * install CSS to shift menu icons into the space reserved for toggles (i.e. check and radio items)
+         *
+         * TODO: This should be private but we already re-use this code in ui/interface.cpp which is not c++ified yet.
+         *       In future ContextMenu and the (to be created) class for the menu bar should then be derived from one common base class.
+         */
+        void ShiftIcons();
     private:
         SPItem *_item; // pointer to the object selected at the time the ContextMenu is created
         SPObject *_object; // pointer to the object selected at the time the ContextMenu is created
@@ -71,11 +78,15 @@ class ContextMenu : public Gtk::Menu
         Gtk::SeparatorMenuItem* AddSeparator();
         
         /**
-         * c++ified version of sp_ui_menu_append_item.
+         * Appends a custom menu UI from a verb.
          * 
+         * c++ified version of sp_ui_menu_append_item.
          * @see sp_ui_menu_append_item_from_verb and synchronize/drop that function when c++ifying other code in interface.cpp
+         * 
+         * @param show_icon True if an icon should be displayed before the menu item's label
+         * 
          */
-        void AppendItemFromVerb(Inkscape::Verb *verb);
+        void AppendItemFromVerb(Inkscape::Verb *verb, bool show_icon = false);
         
         /**
          * main function which is responsible for creating the context sensitive menu items,
diff --git a/src/ui/interface.cpp b/src/ui/interface.cpp
index 24efb8644..1480f267f 100644
--- a/src/ui/interface.cpp
+++ b/src/ui/interface.cpp
@@ -19,8 +19,6 @@
  * Released under GNU GPL, read the file 'COPYING' for more information
  */
 
-#include <gtkmm/cssprovider.h>
-#include <gtkmm/icontheme.h>
 #include <gtkmm/radiomenuitem.h>
 #include <gtkmm/separatormenuitem.h>
 #include <glibmm/miscutils.h>
@@ -68,6 +66,7 @@
 #include "svg/svg-color.h"
 
 #include "ui/clipboard.h"
+#include "ui/contextmenu.h"
 #include "ui/dialog-events.h"
 #include "ui/dialog/dialog-manager.h"
 #include "ui/dialog/inkscape-preferences.h"
@@ -448,62 +447,15 @@ sp_ui_dialog_title_string(Inkscape::Verb *verb, gchar *c)
     }
 }
 
-/* install CSS to shift icons into the space reserved for toggles (i.e. check and radio items) */
+/* install CSS to shift icons into the space reserved for toggles (i.e. check and radio items)
+ *
+ * TODO: This code already exists as a C++ version in the class ContextMenu so we can simply wrap it here.
+ *       In future ContextMenu and the (to be created) class for the menu bar should then be derived from one common base class.
+ */
 void shift_icons(GtkWidget *menu, gpointer /* user_data */)
 {
-    static auto provider = Gtk::CssProvider::create();
-    static bool provider_added = false;
-
-    Gtk::MenuItem *menuitem = nullptr;
-    Gtk::Box *content = nullptr;
-    Gtk::Image *icon = nullptr;
-
-    static int current_shift = 0;
-    int calculated_shift = 0;
-
-    // install CssProvider for our custom styles
-    if (!provider_added) {
-        auto const screen = Gdk::Screen::get_default();
-        Gtk::StyleContext::add_provider_for_screen(screen, provider, GTK_STYLE_PROVIDER_PRIORITY_APPLICATION);
-        provider_added = true;
-    }
-
-    // get the first MenuItem with an image (i.e. "ImageMenuItem" as named below)
-    std::vector<Gtk::Widget *> children = Glib::wrap(GTK_MENU(menu))->get_children();
-    for (auto child: children) {
-        if (child->get_name() == "ImageMenuItem") {
-            menuitem = static_cast<Gtk::MenuItem *>(child);
-            content = static_cast<Gtk::Box *>(menuitem->get_child());
-            icon = static_cast<Gtk::Image *>(content->get_children()[0]);
-            break;
-        }
-    }
-
-    // calculate how far we have to shift the icon to fit it into the empty space between menuitem and its content
-    if (icon) {
-        auto allocation_menuitem = menuitem->get_allocation();
-        auto allocation_icon = icon->get_allocation();
-
-        if (menuitem->get_direction() == Gtk::TEXT_DIR_RTL) {
-            calculated_shift = allocation_menuitem.get_width() - allocation_icon.get_x() - allocation_icon.get_width();
-        } else {
-            calculated_shift = -allocation_icon.get_x();
-        }
-    }
-
-    // install CSS to shift icon, use a threshold to avoid overly frequent updates
-    // (gtk's own calculations for the reserved space are off by a few pixels if there is no check/radio item in a menu)
-    if (calculated_shift && std::abs(current_shift - calculated_shift) > 2) {
-        current_shift = calculated_shift;
-
-        std::string css_str;
-        if (menuitem->get_direction() == Gtk::TEXT_DIR_RTL) {
-            css_str = "#ImageMenuItem image {margin-right:" + std::to_string(-calculated_shift) + "px;}";
-        } else {
-            css_str = "#ImageMenuItem image {margin-left:" + std::to_string(calculated_shift) + "px;}";
-        }
-        provider->load_from_data(css_str);
-    }
+    ContextMenu *contextmenu = static_cast<ContextMenu *>(Glib::wrap(menu));
+    contextmenu->ShiftIcons();
 }
 
 /**
@@ -557,10 +509,6 @@ static GtkWidget *sp_ui_menu_append_item_from_verb(GtkMenu                  *men
 #else
         gtk_misc_set_alignment(GTK_MISC(label), 0.0, 0.5);
 #endif
-
-        GtkAccelGroup *accel_group = sp_shortcut_get_accel_group();
-        gtk_menu_set_accel_group(menu, accel_group);
-
         sp_shortcut_add_accelerator(item->gobj(), sp_shortcut_get_primary(verb));
         gtk_accel_label_set_accel_widget(GTK_ACCEL_LABEL(label), item->gobj());