Store menu titles in CONTEXT_MENU

Previously a title had to be provided when a submenu was added,
but this led to storing the same title in many places.

common/tool/context_menu.cpp

@@ -32,7 +32,7 @@
 using namespace std::placeholders;
 
 CONTEXT_MENU::CONTEXT_MENU() :
-    m_titleSet( false ), m_selected( -1 ), m_tool( nullptr ), m_icon( nullptr )
+    m_titleDisplayed( false ), m_selected( -1 ), m_tool( nullptr ), m_icon( nullptr )
 {
     setupEvents();
 }
@@ -61,19 +61,42 @@ void CONTEXT_MENU::setupEvents()
 
 void CONTEXT_MENU::SetTitle( const wxString& aTitle )
 {
-    // TODO handle an empty string (remove title and separator)
+    // Unfortunately wxMenu::SetTitle() does nothing, but saves the title.. (at least wxGTK)
+    wxMenu::SetTitle( aTitle );
 
-    // Unfortunately wxMenu::SetTitle() does nothing.. (at least wxGTK)
+    // Update the menu title
+    if( m_titleDisplayed )
+        DisplayTitle( true );
+}
 
-    if( m_titleSet )
+
+void CONTEXT_MENU::DisplayTitle( bool aDisplay )
+{
+    const wxString& title = wxMenu::GetTitle();
+
+    if( ( !aDisplay || title.IsEmpty() ) && m_titleDisplayed )
     {
-        FindItemByPosition( 0 )->SetItemLabel( aTitle );
+        // Destroy the menu entry keeping the title..
+        Destroy( FindItemByPosition( 0 ) );
+        // ..and separator
+        Destroy( FindItemByPosition( 0 ) );
+        m_titleDisplayed = false;
     }
-    else
+
+    else if( aDisplay && !title.IsEmpty() )
     {
-        InsertSeparator( 0 );
+        if( m_titleDisplayed )
-        Insert( 0, new wxMenuItem( this, wxID_NONE, aTitle, wxEmptyString, wxITEM_NORMAL ) );
+        {
-        m_titleSet = true;
+            // Simply update the title
+            FindItemByPosition( 0 )->SetItemLabel( title );
+        }
+        else
+        {
+            // Add a separator and a menu entry to display the title
+            InsertSeparator( 0 );
+            Insert( 0, new wxMenuItem( this, wxID_NONE, title, wxEmptyString, wxITEM_NORMAL ) );
+            m_titleDisplayed = true;
+        }
     }
 }
 
@@ -113,7 +136,7 @@ wxMenuItem* CONTEXT_MENU::Add( const TOOL_ACTION& aAction )
 }
 
 
-std::list<wxMenuItem*> CONTEXT_MENU::Add( CONTEXT_MENU* aMenu, const wxString& aLabel, bool aExpand )
+std::list<wxMenuItem*> CONTEXT_MENU::Add( CONTEXT_MENU* aMenu, bool aExpand )
 {
     std::list<wxMenuItem*> items;
     CONTEXT_MENU* menuCopy = aMenu->Clone();
@@ -129,16 +152,18 @@ std::list<wxMenuItem*> CONTEXT_MENU::Add( CONTEXT_MENU* aMenu, const wxString& a
     }
     else
     {
+        wxASSERT_MSG( !menuCopy->GetTitle().IsEmpty(), "Set a title for CONTEXT_MENU using SetTitle()" );
+
         if( aMenu->m_icon )
         {
-            wxMenuItem* newItem = new wxMenuItem( this, -1, aLabel, wxEmptyString, wxITEM_NORMAL );
+            wxMenuItem* newItem = new wxMenuItem( this, -1, menuCopy->GetTitle() );
             newItem->SetBitmap( KiBitmap( aMenu->m_icon ) );
             newItem->SetSubMenu( menuCopy );
             items.push_back( Append( newItem ) );
         }
         else
         {
-            items.push_back( AppendSubMenu( menuCopy, aLabel ) );
+            items.push_back( AppendSubMenu( menuCopy, menuCopy->GetTitle() ) );
         }
     }
 
@@ -148,7 +173,7 @@ std::list<wxMenuItem*> CONTEXT_MENU::Add( CONTEXT_MENU* aMenu, const wxString& a
 
 void CONTEXT_MENU::Clear()
 {
-    m_titleSet = false;
+    m_titleDisplayed = false;
 
     for( int i = GetMenuItemCount() - 1; i >= 0; --i )
         Destroy( FindItemByPosition( i ) );
@@ -331,7 +356,7 @@ void CONTEXT_MENU::runOnSubmenus( std::function<void(CONTEXT_MENU*)> aFunction )
 void CONTEXT_MENU::copyFrom( const CONTEXT_MENU& aMenu )
 {
     m_icon = aMenu.m_icon;
-    m_titleSet = aMenu.m_titleSet;
+    m_titleDisplayed = aMenu.m_titleDisplayed;
     m_selected = -1; // aMenu.m_selected;
     m_tool = aMenu.m_tool;
     m_toolActions = aMenu.m_toolActions;

include/tool/context_menu.h

@@ -60,6 +60,12 @@ public:
      */
     void SetTitle( const wxString& aTitle ) override;
 
+    /**
+     * Function DisplayTitle()
+     * Decides whether a title for a pop up menu should be displayed.
+     */
+    void DisplayTitle( bool aDisplay = true );
+
     /**
      * Function SetIcon()
      * Assigns an icon for the entry.
@@ -93,11 +99,10 @@ public:
      * Adds a context menu as a submenu. The difference between this function and wxMenu::AppendSubMenu()
      * is the capability to handle icons.
      * @param aMenu is the submenu to be added.
-     * @param aLabel is the caption displayed for the menu entry.
      * @param aExpand allows to add all entries from the menu as individual entries rather than
      *                add everything as a submenu.
      */
-    std::list<wxMenuItem*> Add( CONTEXT_MENU* aMenu, const wxString& aLabel, bool aExpand = false );
+    std::list<wxMenuItem*> Add( CONTEXT_MENU* aMenu, bool aExpand = false );
 
     /**
      * Function Clear()
@@ -195,7 +200,10 @@ private:
     void runOnSubmenus( std::function<void(CONTEXT_MENU*)> aFunction );
 
     ///> Flag indicating that the menu title was set up.
-    bool m_titleSet;
+    bool m_titleDisplayed;
+
+    ///> Menu title
+    wxString m_title;
 
     ///> Stores the id number of selected item.
     int m_selected;

pcbnew/router/length_tuner_tool.cpp

@@ -79,6 +79,7 @@ public:
     TUNER_TOOL_MENU()
     {
         SetTitle( _( "Length Tuner" ) );
+        DisplayTitle( true );
 
         //Add( ACT_StartTuning );
         //Add( ACT_EndTuning );

pcbnew/router/router_tool.cpp

@@ -123,6 +123,7 @@ public:
     TRACK_WIDTH_MENU( const BOARD* aBoard )
         : TRACK_VIA_SIZE_MENU( true, true )
     {
+        SetTitle( _( "Select Track/Via Width" ) );
         SetBoard( aBoard );
     }
 
@@ -234,7 +235,7 @@ public:
         AppendSeparator();
 
         m_widthMenu.SetBoard( aBoard );
-        Add( &m_widthMenu, _( "Select Track/Via Width" ) );
+        Add( &m_widthMenu );
 
         Add( ACT_CustomTrackWidth );
 
@@ -245,8 +246,8 @@ public:
         Add( PNS::TOOL_BASE::ACT_RouterOptions );
 
         AppendSeparator();
-        Add( &m_zoomMenu, _( "Zoom Select" ), false );
+        Add( &m_zoomMenu );
-        Add( &m_gridMenu, _( "Grid Select" ), false );
+        Add( &m_gridMenu );
     }
 
 private:

pcbnew/tools/conditional_menu.cpp

@@ -33,10 +33,10 @@ void CONDITIONAL_MENU::AddItem( const TOOL_ACTION& aAction, const SELECTION_COND
 }
 
 
-void CONDITIONAL_MENU::AddMenu( CONTEXT_MENU* aMenu, const wxString& aLabel, bool aExpand,
+void CONDITIONAL_MENU::AddMenu( CONTEXT_MENU* aMenu, bool aExpand,
                                 const SELECTION_CONDITION& aCondition, int aOrder )
 {
-    addEntry( ENTRY( aMenu, aLabel, aExpand, aCondition, aOrder ) );
+    addEntry( ENTRY( aMenu, aExpand, aCondition, aOrder ) );
 }
 
 
@@ -72,7 +72,7 @@ CONTEXT_MENU* CONDITIONAL_MENU::Generate( SELECTION& aSelection )
                 break;
 
             case ENTRY::MENU:
-                m_menu->Add( it->Menu(), it->Label(), it->Expand() );
+                m_menu->Add( it->Menu(), it->Expand() );
                 break;
 
             case ENTRY::WXITEM:

pcbnew/tools/conditional_menu.h

@@ -64,14 +64,13 @@ public:
      * Adds a submenu to the menu. CONDITIONAL_MENU takes ownership of the added menu, so it will
      * be freed when the CONDITIONAL_MENU object is destroyed.
      * @param aMenu is the submenu to be added.
-     * @param aLabel is the label of added submenu.
      * @param aExpand determines if the added submenu items should be added as individual items
      * or as a submenu.
      * @param aCondition is a condition that has to be fulfilled to enable the submenu entry.
      * @param aOrder determines location of the added menu, higher numbers are put on the bottom.
      * You may use ANY_ORDER here if you think it does not matter.
      */
-    void AddMenu( CONTEXT_MENU* aMenu, const wxString& aLabel, bool aExpand = false,
+    void AddMenu( CONTEXT_MENU* aMenu, bool aExpand = false,
                   const SELECTION_CONDITION& aCondition = SELECTION_CONDITIONS::ShowAlways,
                   int aOrder = ANY_ORDER );
 
@@ -108,10 +107,10 @@ private:
             m_data.action = aAction;
         }
 
-        ENTRY( CONTEXT_MENU* aMenu, const wxString aLabel, bool aExpand = false,
+        ENTRY( CONTEXT_MENU* aMenu, bool aExpand = false,
                             const SELECTION_CONDITION& aCondition = SELECTION_CONDITIONS::ShowAlways,
                             int aOrder = ANY_ORDER ) :
-            m_type( MENU ), m_condition( aCondition ), m_order( aOrder ), m_label( aLabel ), m_expand( aExpand )
+            m_type( MENU ), m_condition( aCondition ), m_order( aOrder ), m_expand( aExpand )
         {
             m_data.menu = aMenu;
         }
@@ -162,12 +161,6 @@ private:
             return m_data.wxItem;
         }
 
-        inline const wxString& Label() const
-        {
-            assert( m_type == MENU );
-            return m_label;
-        }
-
         inline bool Expand() const
         {
             assert( m_type == MENU );
@@ -204,8 +197,7 @@ private:
         ///> Order number, the higher the number the lower position it takes it is in the menu.
         int m_order;
 
-        /// CONTEXT_MENU specific fields.
+        ///> CONTEXT_MENU expand flag
-        const wxString m_label;
         bool m_expand;
     };
 

pcbnew/tools/grid_menu.cpp

@@ -36,6 +36,7 @@ GRID_MENU::GRID_MENU( EDA_DRAW_FRAME* aParent ) : m_parent( aParent )
 {
     BASE_SCREEN* screen = aParent->GetScreen();
 
+    SetTitle( _( "Grid" ) );
     SetIcon( grid_select_xpm );
 
     wxArrayString gridsList;

pcbnew/tools/pcb_editor_control.cpp

@@ -60,6 +60,8 @@ public:
     ZONE_CONTEXT_MENU()
     {
         SetIcon( add_zone_xpm );
+        SetTitle( _( "Zones" ) );
+
         Add( COMMON_ACTIONS::zoneFill );
         Add( COMMON_ACTIONS::zoneFillAll );
         Add( COMMON_ACTIONS::zoneUnfill );
@@ -101,6 +103,8 @@ public:
     LOCK_CONTEXT_MENU()
     {
         SetIcon( locked_xpm );
+        SetTitle( _( "Locking" ) );
+
         Add( COMMON_ACTIONS::lock );
         Add( COMMON_ACTIONS::unlock );
         Add( COMMON_ACTIONS::toggleLock );
@@ -164,11 +168,11 @@ bool PCB_EDITOR_CONTROL::Init()
         toolMenu.AddSubMenu( zoneMenu );
         toolMenu.AddSubMenu( lockMenu );
 
-        menu.AddMenu( zoneMenu.get(), _( "Zones" ), false,
+        menu.AddMenu( zoneMenu.get(), false,
-                      SELECTION_CONDITIONS::OnlyType( PCB_ZONE_AREA_T ) );
+                SELECTION_CONDITIONS::OnlyType( PCB_ZONE_AREA_T ) );
 
-        menu.AddMenu( lockMenu.get(), _( "Locking" ), false,
+        menu.AddMenu( lockMenu.get(), false,
-                      SELECTION_CONDITIONS::OnlyTypes( GENERAL_COLLECTOR::Tracks ) );
+                SELECTION_CONDITIONS::OnlyTypes( GENERAL_COLLECTOR::Tracks ) );
     }
 
     DRAWING_TOOL* drawingTool = m_toolMgr->GetTool<DRAWING_TOOL>();
@@ -190,8 +194,7 @@ bool PCB_EDITOR_CONTROL::Init()
             };
         };
 
-        menu.AddMenu( zoneMenu.get(), _( "Zones" ), false,
+        menu.AddMenu( zoneMenu.get(), false, toolActiveFunctor( DRAWING_TOOL::MODE::ZONE ) );
-                      toolActiveFunctor( DRAWING_TOOL::MODE::ZONE ) );
     }
 
     return true;

pcbnew/tools/placement_tool.cpp

@@ -60,6 +60,7 @@ bool PLACEMENT_TOOL::Init()
     // Create a context menu and make it available through selection tool
     m_placementMenu = new CONTEXT_MENU;
     m_placementMenu->SetIcon( align_items_xpm );
+    m_placementMenu->SetTitle( _( "Align/distribute" ) );
 
     // Add all align/distribute commands
     m_placementMenu->Add( COMMON_ACTIONS::alignTop );
@@ -70,8 +71,7 @@ bool PLACEMENT_TOOL::Init()
     m_placementMenu->Add( COMMON_ACTIONS::distributeHorizontally );
     m_placementMenu->Add( COMMON_ACTIONS::distributeVertically );
 
-    m_selectionTool->GetToolMenu().GetMenu().AddMenu(
+    m_selectionTool->GetToolMenu().GetMenu().AddMenu( m_placementMenu, false,
-            m_placementMenu, _( "Align/distribute" ), false,
             SELECTION_CONDITIONS::MoreThan( 1 ) );
 
     return true;

pcbnew/tools/selection_tool.cpp

@@ -58,6 +58,7 @@ class SELECT_MENU: public CONTEXT_MENU
 public:
     SELECT_MENU()
     {
+        SetTitle( _( "Select..." ) );
         Add( COMMON_ACTIONS::selectConnection );
         Add( COMMON_ACTIONS::selectCopper );
         Add( COMMON_ACTIONS::selectNet );
@@ -100,7 +101,7 @@ bool SELECTION_TOOL::Init()
 
     auto& menu = m_menu.GetMenu();
 
-    menu.AddMenu( selectMenu.get(), _( "Select..." ), false, showSelectMenuFunctor );
+    menu.AddMenu( selectMenu.get(), false, showSelectMenuFunctor );
     // only show separator if there is a Select menu to show above it
     menu.AddSeparator( showSelectMenuFunctor, 1000 );
 
@@ -722,6 +723,7 @@ BOARD_ITEM* SELECTION_TOOL::disambiguationMenu( GENERAL_COLLECTOR* aCollector )
     }
 
     menu.SetTitle( _( "Clarify selection" ) );
+    menu.DisplayTitle( true );
     SetContextMenu( &menu, CMENU_NOW );
 
     while( OPT_TOOL_EVENT evt = Wait() )

pcbnew/tools/tool_menu.cpp

@@ -96,10 +96,8 @@ void TOOL_MENU::AddStandardSubMenus( EDA_DRAW_FRAME& aFrame )
     m_menu.AddItem( COMMON_ACTIONS::zoomOut, S_C::ShowAlways, 1000 );
     m_menu.AddItem( COMMON_ACTIONS::zoomFitScreen, S_C::ShowAlways, 1000 );
 
-    m_menu.AddSeparator(SELECTION_CONDITIONS::ShowAlways, 1000);
+    m_menu.AddSeparator(SELECTION_CONDITIONS::ShowAlways, 1000 );
-    m_menu.AddMenu( createOwnSubMenu<ZOOM_MENU>( &aFrame ).get(),
-            _( "Zoom Select" ), false, S_C::ShowAlways, 1000 );
 
-    m_menu.AddMenu( createOwnSubMenu<GRID_MENU>( &aFrame ).get(),
+    m_menu.AddMenu( createOwnSubMenu<ZOOM_MENU>( &aFrame ).get(), false, S_C::ShowAlways, 1000 );
-            _( "Grid Select" ), false, S_C::ShowAlways, 1000 );
+    m_menu.AddMenu( createOwnSubMenu<GRID_MENU>( &aFrame ).get(), false, S_C::ShowAlways, 1000 );
 }

pcbnew/tools/zoom_menu.cpp

@@ -34,6 +34,8 @@ using namespace std::placeholders;
 ZOOM_MENU::ZOOM_MENU( EDA_DRAW_FRAME* aParent ) : m_parent( aParent )
 {
     BASE_SCREEN* screen = aParent->GetScreen();
+
+    SetTitle( _( "Zoom" ) );
     SetIcon( zoom_selection_xpm );
 
     //int zoom = screen->GetZoom();