kicad-developers team mailing list archive

[Ian McInerney <Ian.S.McInerney@xxxxxxxx>]

Right now the addition of the Close item for the non-main windows is
scattered in the menu creation functions. This means it is easy to leave
out the accelerator key entry when adding a new one (yes, I forgot that in
cvpcb...). This patch will push the creation of the menu item into a common
method in CONDITIONAL_MENU that is then just called by each window. It also
introduces a tooltip for the item that includes the app name. This unifies
all the close items, and ensures they have the proper CTRL-W accelerator
key assigned.

Note that this does not unify the one for the simulation window. That
window is a wxFormbuilder window so its menu is not a CONDITIONAL_MENU and
can't use this.

-Ian

From 0fd6b672ce8b99f681497e1dba2746e2e84e5840 Mon Sep 17 00:00:00 2001
From: Ian McInerney <Ian.S.McInerney@xxxxxxxx>
Date: Wed, 14 Aug 2019 01:34:05 +0200
Subject: [PATCH] Unify menu item creation for closing a window

* Push a function into CONDITIONAL_MENU that adds the item
* Modify the tooltip for close and exit items to have the
  program name

Fixes: lp:1835454
* https://bugs.launchpad.net/kicad/+bug/1835454
---
 3d-viewer/3d_viewer/3d_menubar.cpp   |  3 +--
 common/tool/conditional_menu.cpp     | 14 +++++++++++---
 cvpcb/menubar.cpp                    |  2 +-
 eeschema/libedit/menubar_libedit.cpp |  2 +-
 eeschema/menubar.cpp                 |  2 +-
 eeschema/toolbars_viewlib.cpp        |  3 +--
 gerbview/menubar.cpp                 |  2 +-
 include/tool/conditional_menu.h      | 18 +++++++++++++++---
 kicad/menubar.cpp                    |  2 +-
 pagelayout_editor/menubar.cpp        |  2 +-
 pcbnew/menubar_footprint_editor.cpp  |  3 +--
 pcbnew/menubar_pcb_editor.cpp        |  2 +-
 pcbnew/toolbars_footprint_viewer.cpp |  3 +--
 13 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/3d-viewer/3d_viewer/3d_menubar.cpp b/3d-viewer/3d_viewer/3d_menubar.cpp
index 0b887430e..273403b59 100644
--- a/3d-viewer/3d_viewer/3d_menubar.cpp
+++ b/3d-viewer/3d_viewer/3d_menubar.cpp
@@ -54,8 +54,7 @@ void EDA_3D_VIEWER::CreateMenuBar()
                        export_xpm,                     SELECTION_CONDITIONS::ShowAlways );
 
     fileMenu->AddSeparator();
-    fileMenu->AddItem( wxID_CLOSE, _( "Close\tCTRL+W" ), "",
-                       exit_xpm,                       SELECTION_CONDITIONS::ShowAlways );
+    fileMenu->AddClose( _( "3D Viewer" ) );
 
     fileMenu->Resolve();
 
diff --git a/common/tool/conditional_menu.cpp b/common/tool/conditional_menu.cpp
index f55756d2e..fdda4686a 100644
--- a/common/tool/conditional_menu.cpp
+++ b/common/tool/conditional_menu.cpp
@@ -99,17 +99,25 @@ void CONDITIONAL_MENU::AddSeparator( int aOrder )
 }
 
 
-void CONDITIONAL_MENU::AddQuitOrClose( KIFACE_I* aKiface )
+void CONDITIONAL_MENU::AddClose( wxString aAppname )
+{
+    AddItem( wxID_CLOSE, _( "Close\tCTRL+W" ), wxString::Format( "Close %s", aAppname ), exit_xpm,
+            SELECTION_CONDITIONS::ShowAlways );
+}
+
+
+void CONDITIONAL_MENU::AddQuitOrClose( KIFACE_I* aKiface, wxString aAppname )
 {
     if( !aKiface || aKiface->IsSingle() ) // not when under a project mgr
     {
         // Don't use ACTIONS::quit; wxWidgets moves this on OSX and expects to find it via
         // wxID_EXIT
-        AddItem( wxID_EXIT, _( "Quit" ), "", exit_xpm, SELECTION_CONDITIONS::ShowAlways );
+        AddItem( wxID_EXIT, _( "Quit" ), wxString::Format( "Quit %s", aAppname ), exit_xpm,
+                SELECTION_CONDITIONS::ShowAlways );
     }
     else
     {
-        AddItem( wxID_CLOSE, _( "Close\tCTRL+W" ), "", exit_xpm, SELECTION_CONDITIONS::ShowAlways );
+        AddClose( aAppname );
     }
 }
 
diff --git a/cvpcb/menubar.cpp b/cvpcb/menubar.cpp
index 5ecae291d..c0d9e9088 100644
--- a/cvpcb/menubar.cpp
+++ b/cvpcb/menubar.cpp
@@ -47,7 +47,7 @@ void CVPCB_MAINFRAME::ReCreateMenuBar()
 
     fileMenu->AddItem( CVPCB_ACTIONS::saveAssociations, SELECTION_CONDITIONS::ShowAlways );
     fileMenu->AddSeparator();
-    fileMenu->AddItem( wxID_CLOSE, _( "Close" ), "", exit_xpm, SELECTION_CONDITIONS::ShowAlways );
+    fileMenu->AddClose( _( "Assign Footprints" ) );
 
     fileMenu->Resolve();
 
diff --git a/eeschema/libedit/menubar_libedit.cpp b/eeschema/libedit/menubar_libedit.cpp
index 4d1dfa875..a1c2b1bfc 100644
--- a/eeschema/libedit/menubar_libedit.cpp
+++ b/eeschema/libedit/menubar_libedit.cpp
@@ -82,7 +82,7 @@ void LIB_EDIT_FRAME::ReCreateMenuBar()
     fileMenu->AddMenu( submenuExport,              EE_CONDITIONS::ShowAlways );
 
     fileMenu->AddSeparator();
-    fileMenu->AddItem( wxID_CLOSE, _( "Close\tCTRL+W" ), "", exit_xpm, EE_CONDITIONS::ShowAlways );
+    fileMenu->AddClose( _( "Library Editor" ) );
 
     fileMenu->Resolve();
 
diff --git a/eeschema/menubar.cpp b/eeschema/menubar.cpp
index 1f0d70e70..74c529843 100644
--- a/eeschema/menubar.cpp
+++ b/eeschema/menubar.cpp
@@ -115,7 +115,7 @@ void SCH_EDIT_FRAME::ReCreateMenuBar()
     fileMenu->AddItem( ACTIONS::plot,              EE_CONDITIONS::ShowAlways );
 
     fileMenu->AddSeparator();
-    fileMenu->AddQuitOrClose( &Kiface() );
+    fileMenu->AddQuitOrClose( &Kiface(), _( "Eeschema" ) );
 
     fileMenu->Resolve();
 
diff --git a/eeschema/toolbars_viewlib.cpp b/eeschema/toolbars_viewlib.cpp
index e9af694fb..8ba05fce5 100644
--- a/eeschema/toolbars_viewlib.cpp
+++ b/eeschema/toolbars_viewlib.cpp
@@ -102,8 +102,7 @@ void LIB_VIEW_FRAME::ReCreateMenuBar()
     //
     CONDITIONAL_MENU* fileMenu = new CONDITIONAL_MENU( false, libControl );
 
-    fileMenu->AddItem( wxID_CLOSE, _( "Close" ), _( "Close footprint viewer" ), exit_xpm,
-            EE_CONDITIONS::ShowAlways );
+    fileMenu->AddClose( _( "Footprint Viewer" ) );
 
     fileMenu->Resolve();
 
diff --git a/gerbview/menubar.cpp b/gerbview/menubar.cpp
index e072001d4..1f26cc220 100644
--- a/gerbview/menubar.cpp
+++ b/gerbview/menubar.cpp
@@ -139,7 +139,7 @@ void GERBVIEW_FRAME::ReCreateMenuBar()
     fileMenu->AddItem( ACTIONS::print,            SELECTION_CONDITIONS::ShowAlways );
 
     fileMenu->AddSeparator();
-    fileMenu->AddQuitOrClose( &Kiface() );
+    fileMenu->AddQuitOrClose( &Kiface(), _( "GerbView" ) );
 
     fileMenu->Resolve();
 
diff --git a/include/tool/conditional_menu.h b/include/tool/conditional_menu.h
index dc788c037..5c3faf4d7 100644
--- a/include/tool/conditional_menu.h
+++ b/include/tool/conditional_menu.h
@@ -100,13 +100,25 @@ public:
      */
     void AddSeparator( int aOrder = ANY_ORDER );
 
+    /**
+     * Function AddClose()
+     *
+     * Add a standard close item to the menu with the accelerator key CTRL-W.
+     * Emits the wxID_CLOSE event.
+     *
+     * @param aAppname is the application name to append to the tooltip
+     */
+    void AddClose( wxString aAppname = "" );
+
     /**
      * Functions AddQuitOrClose()
      *
-     * Adds either a standard Quit or Close item to the menu (depending on whether or not the
-     * app was launched stand-alone).
+     * Adds either a standard Quit or Close item to the menu. If aKiface is NULL or in
+     * single-instance then Quite (wxID_QUIT) is used, otherwise Close (wxID_CLOSE) is used.
+     *
+     * @param aAppname is the application name to append to the tooltip
      */
-    void AddQuitOrClose( KIFACE_I* aKiface );
+    void AddQuitOrClose( KIFACE_I* aKiface, wxString aAppname = "" );
 
     /**
      * Function Evaluate()
diff --git a/kicad/menubar.cpp b/kicad/menubar.cpp
index 750f3aea8..c83dd0126 100644
--- a/kicad/menubar.cpp
+++ b/kicad/menubar.cpp
@@ -84,7 +84,7 @@ void KICAD_MANAGER_FRAME::ReCreateMenuBar()
                        unzip_xpm,                              SELECTION_CONDITIONS::ShowAlways );
 
     fileMenu->AddSeparator();
-    fileMenu->AddQuitOrClose( nullptr );
+    fileMenu->AddQuitOrClose( nullptr, _( "KiCad" ) );
 
     fileMenu->Resolve();
 
diff --git a/pagelayout_editor/menubar.cpp b/pagelayout_editor/menubar.cpp
index 6740be3be..fea8185cf 100644
--- a/pagelayout_editor/menubar.cpp
+++ b/pagelayout_editor/menubar.cpp
@@ -78,7 +78,7 @@ void PL_EDITOR_FRAME::ReCreateMenuBar()
     fileMenu->AddItem( ACTIONS::print,         SELECTION_CONDITIONS::ShowAlways );
 
     fileMenu->AddSeparator();
-    fileMenu->AddQuitOrClose( &Kiface() );
+    fileMenu->AddQuitOrClose( &Kiface(), _( "Page Layout Editor" ) );
 
     fileMenu->Resolve();
 
diff --git a/pcbnew/menubar_footprint_editor.cpp b/pcbnew/menubar_footprint_editor.cpp
index b33509a47..755ea396f 100644
--- a/pcbnew/menubar_footprint_editor.cpp
+++ b/pcbnew/menubar_footprint_editor.cpp
@@ -105,8 +105,7 @@ void FOOTPRINT_EDIT_FRAME::ReCreateMenuBar()
     fileMenu->AddItem( ACTIONS::print,              haveFootprintCondition );
 
     fileMenu->AddSeparator();
-    fileMenu->AddItem( wxID_CLOSE, _( "Close\tCTRL+W" ), "",
-                       exit_xpm,                    SELECTION_CONDITIONS::ShowAlways );
+    fileMenu->AddClose( _( "Footprint Editor" ) );
 
     fileMenu->Resolve();
 
diff --git a/pcbnew/menubar_pcb_editor.cpp b/pcbnew/menubar_pcb_editor.cpp
index e98960387..d88d87af2 100644
--- a/pcbnew/menubar_pcb_editor.cpp
+++ b/pcbnew/menubar_pcb_editor.cpp
@@ -193,7 +193,7 @@ void PCB_EDIT_FRAME::ReCreateMenuBar()
     fileMenu->AddMenu( submenuArchive,               SELECTION_CONDITIONS::ShowAlways );
 
     fileMenu->AddSeparator();
-    fileMenu->AddQuitOrClose( &Kiface() );
+    fileMenu->AddQuitOrClose( &Kiface(), _( "Pcbnew" ) );
 
     fileMenu->Resolve();
 
diff --git a/pcbnew/toolbars_footprint_viewer.cpp b/pcbnew/toolbars_footprint_viewer.cpp
index c1f9c37cf..6ea79841b 100644
--- a/pcbnew/toolbars_footprint_viewer.cpp
+++ b/pcbnew/toolbars_footprint_viewer.cpp
@@ -123,8 +123,7 @@ void FOOTPRINT_VIEWER_FRAME::ReCreateMenuBar()
     //
     CONDITIONAL_MENU* fileMenu = new CONDITIONAL_MENU( false, selTool );
 
-    fileMenu->AddItem( wxID_CLOSE, _( "Close" ), _( "Close footprint viewer" ), exit_xpm,
-            SELECTION_CONDITIONS::ShowAlways );
+    fileMenu->AddClose( _( "Footprint Viewer" ) );
 
     fileMenu->Resolve();
 
-- 
2.21.0