Viewport switcher keys are platform-specific.

Also fixes a regression where ctrl-tab doesn't get recognized.

Fixes https://gitlab.com/kicad/code/kicad/issues/11778

Fixes https://gitlab.com/kicad/code/kicad/issues/10127

3d-viewer/3d_viewer/3d_toolbar.cpp

@@ -28,6 +28,7 @@
 #include <wx/choice.h>
 
 #include <bitmaps.h>
+#include <dialogs/eda_view_switcher.h>
 #include <eda_3d_viewer_frame.h>
 #include <menus_helpers.h>
 #include <tool/action_toolbar.h>
@@ -52,7 +53,10 @@ void EDA_3D_VIEWER_FRAME::ReCreateMainToolbar()
     }
 
     // Show the hotkey to open the windows list selector:
-    m_viewportsLabel = new wxStaticText( m_mainToolBar, wxID_ANY, _( "Viewports (Shift+Tab):" ) );
+    wxString keyName = KeyNameFromKeyCode( VIEWPORT_SWITCH_KEY );
+
+    m_viewportsLabel = new wxStaticText( m_mainToolBar, wxID_ANY,
+                                         wxString::Format( _( "Viewports (%sTab):" ), keyName ) );
    	m_viewportsLabel->Wrap( -1 );
 
    	m_cbViewports = new wxChoice( m_mainToolBar, wxID_ANY );
@@ -64,9 +68,11 @@ void EDA_3D_VIEWER_FRAME::ReCreateMainToolbar()
     m_cbViewports->Append( _( "Save viewport..." ) );
     m_cbViewports->Append( _( "Delete viewport..." ) );
 
-    m_cbViewports->SetToolTip( _( "Save and restore view orientation and zoom.  Use Shift+Tab to "
+    m_cbViewports->SetToolTip( wxString::Format( _( "Save and restore view orientation and zoom.\n"
-                                  "activate selector.  Successive Tabs while holding Shift down "
+                                                    "Use %sTab to activate selector.\n"
-                                  "will cycle through viewports in popup." ) );
+                                                    "Successive Tabs while holding %s down will "
+                                                    "cycle through viewports in the popup." ),
+                                                 keyName, keyName ) );
 
     m_cbViewports->SetSelection( m_cbViewports->GetCount() - 3 );
 

3d-viewer/3d_viewer/eda_3d_viewer_frame.cpp

@@ -292,31 +292,36 @@ bool EDA_3D_VIEWER_FRAME::TryBefore( wxEvent& aEvent )
 {
     static bool s_viewportSwitcherShown = false;
 
-    // On Windows, the Alt key is not usable, especially with TAB key
+    // wxWidgets generates no key events for the tab key when the ctrl key is held down.  One
-    // Shift key is OK on all platforms
+    // way around this is to look at all events and inspect the keyboard state of the tab key.
-    wxKeyCode viewSwitchKey = WXK_SHIFT;
+    // However, this runs into issues on some linux VMs where querying the keyboard state is
-
+    // very slow.  Fortunately we only use ctrl-tab on Mac, so we implement this lovely hack:
-    if( aEvent.GetEventType() != wxEVT_CHAR && aEvent.GetEventType() != wxEVT_CHAR_HOOK )
+#ifdef __WXMAC__
-        return wxFrame::TryBefore( aEvent );
+    if( wxGetKeyState( WXK_TAB ) )
-
+#else
-    if( !s_viewportSwitcherShown && wxGetKeyState( viewSwitchKey ) && wxGetKeyState( WXK_TAB ) )
+    if( ( aEvent.GetEventType() == wxEVT_CHAR || aEvent.GetEventType() == wxEVT_CHAR_HOOK )
+            && static_cast<wxKeyEvent&>( aEvent ).GetKeyCode() == WXK_TAB )
+#endif
     {
-        if( this->IsActive() )
+        if( !s_viewportSwitcherShown && wxGetKeyState( VIEWPORT_SWITCH_KEY ) )
         {
-            if( m_viewportMRU.size() > 0 )
+            if( this->IsActive() )
             {
-                EDA_VIEW_SWITCHER switcher( this, m_viewportMRU, viewSwitchKey );
+                if( m_viewportMRU.size() > 0 )
+                {
+                    EDA_VIEW_SWITCHER switcher( this, m_viewportMRU, VIEWPORT_SWITCH_KEY );
 
-                s_viewportSwitcherShown = true;
+                    s_viewportSwitcherShown = true;
-                switcher.ShowModal();
+                    switcher.ShowModal();
-                s_viewportSwitcherShown = false;
+                    s_viewportSwitcherShown = false;
 
-                int idx = switcher.GetSelection();
+                    int idx = switcher.GetSelection();
 
-                if( idx >= 0 && idx < (int) m_viewportMRU.size() )
+                    if( idx >= 0 && idx < (int) m_viewportMRU.size() )
-                    applyViewport( m_viewportMRU[idx] );
+                        applyViewport( m_viewportMRU[idx] );
 
-                return true;
+                    return true;
+                }
             }
         }
     }

common/dialogs/eda_view_switcher.cpp

@@ -106,7 +106,7 @@ bool EDA_VIEW_SWITCHER::TryBefore( wxEvent& aEvent )
 
         int idx = m_listBox->GetSelection();
 
-        if( wxGetKeyState( WXK_SHIFT ) )
+        if( wxGetKeyState( WXK_SHIFT ) && m_ctrlKey != WXK_SHIFT )
         {
             if( --idx < 0 )
                 m_listBox->SetSelection( (int) m_listBox->GetCount() - 1 );

common/hotkeys_basic.cpp

@@ -175,6 +175,15 @@ wxString KeyNameFromKeyCode( int aKeycode, bool* aIsFound )
     int      ii;
     bool     found = false;
 
+    if( aKeycode == WXK_CONTROL )
+        return MODIFIER_CTRL;
+    else if( aKeycode == WXK_RAW_CONTROL )
+        return MODIFIER_CTRL_BASE;
+    else if( aKeycode == WXK_SHIFT )
+        return MODIFIER_SHIFT;
+    else if( aKeycode == WXK_ALT )
+        return MODIFIER_ALT;
+
     // Assume keycode of 0 is "unassigned"
     if( (aKeycode & MD_CTRL) != 0 )
         modifier << MODIFIER_CTRL;

include/dialogs/eda_view_switcher.h

@@ -1,7 +1,7 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2020 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2020-2022 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -26,6 +26,16 @@
 
 #include <eda_view_switcher_base.h>
 
+
+#ifdef __WXMAC__
+    #define PRESET_SWITCH_KEY WXK_RAW_CONTROL
+    #define VIEWPORT_SWITCH_KEY WXK_ALT
+#else
+    #define PRESET_SWITCH_KEY WXK_CONTROL
+    #define VIEWPORT_SWITCH_KEY WXK_SHIFT
+#endif
+
+
 class EDA_VIEW_SWITCHER : public EDA_VIEW_SWITCHER_BASE
 {
 public:

pcbnew/pcb_base_edit_frame.cpp

@@ -100,60 +100,61 @@ bool PCB_BASE_EDIT_FRAME::TryBefore( wxEvent& aEvent )
     static bool s_presetSwitcherShown = false;
     static bool s_viewportSwitcherShown = false;
 
+    // wxWidgets generates no key events for the tab key when the ctrl key is held down.  One
+    // way around this is to look at all events and inspect the keyboard state of the tab key.
+    // However, this runs into issues on some linux VMs where querying the keyboard state is
+    // very slow.  Fortunately we only use ctrl-tab on Mac, so we implement this lovely hack:
 #ifdef __WXMAC__
-    wxKeyCode presetSwitchKey = WXK_RAW_CONTROL;
+    if( wxGetKeyState( WXK_TAB ) )
-    wxKeyCode viewSwitchKey = WXK_ALT;
 #else
-    wxKeyCode presetSwitchKey = WXK_RAW_CONTROL;
+    if( ( aEvent.GetEventType() == wxEVT_CHAR || aEvent.GetEventType() == wxEVT_CHAR_HOOK )
-    wxKeyCode viewSwitchKey = WXK_WINDOWS_LEFT;
+            && static_cast<wxKeyEvent&>( aEvent ).GetKeyCode() == WXK_TAB )
 #endif
-
-    if( aEvent.GetEventType() != wxEVT_CHAR && aEvent.GetEventType() != wxEVT_CHAR_HOOK )
-        return PCB_BASE_FRAME::TryBefore( aEvent );
-
-    if( !s_presetSwitcherShown && wxGetKeyState( presetSwitchKey ) && wxGetKeyState( WXK_TAB ) )
     {
-        if( m_appearancePanel && this->IsActive() )
+        if( !s_presetSwitcherShown && wxGetKeyState( PRESET_SWITCH_KEY ) )
         {
-            const wxArrayString& mru = m_appearancePanel->GetLayerPresetsMRU();
+            if( m_appearancePanel && this->IsActive() )
-
-            if( mru.size() > 0 )
             {
-                EDA_VIEW_SWITCHER switcher( this, mru, presetSwitchKey );
+                const wxArrayString& mru = m_appearancePanel->GetLayerPresetsMRU();
 
-                s_presetSwitcherShown = true;
+                if( mru.size() > 0 )
-                switcher.ShowModal();
+                {
-                s_presetSwitcherShown = false;
+                    EDA_VIEW_SWITCHER switcher( this, mru, PRESET_SWITCH_KEY );
 
-                int idx = switcher.GetSelection();
+                    s_presetSwitcherShown = true;
+                    switcher.ShowModal();
+                    s_presetSwitcherShown = false;
 
-                if( idx >= 0 && idx < (int) mru.size() )
+                    int idx = switcher.GetSelection();
-                    m_appearancePanel->ApplyLayerPreset( mru[idx] );
 
-                return true;
+                    if( idx >= 0 && idx < (int) mru.size() )
+                        m_appearancePanel->ApplyLayerPreset( mru[idx] );
+
+                    return true;
+                }
             }
         }
-    }
+        else if( !s_viewportSwitcherShown && wxGetKeyState( VIEWPORT_SWITCH_KEY ) )
-    else if( !s_viewportSwitcherShown && wxGetKeyState( viewSwitchKey ) && wxGetKeyState( WXK_TAB ) )
-    {
-        if( m_appearancePanel && this->IsActive() )
         {
-            const wxArrayString& mru = m_appearancePanel->GetViewportsMRU();
+            if( m_appearancePanel && this->IsActive() )
-
-            if( mru.size() > 0 )
             {
-                EDA_VIEW_SWITCHER switcher( this, mru, viewSwitchKey );
+                const wxArrayString& mru = m_appearancePanel->GetViewportsMRU();
 
-                s_viewportSwitcherShown = true;
+                if( mru.size() > 0 )
-                switcher.ShowModal();
+                {
-                s_viewportSwitcherShown = false;
+                    EDA_VIEW_SWITCHER switcher( this, mru, VIEWPORT_SWITCH_KEY );
 
-                int idx = switcher.GetSelection();
+                    s_viewportSwitcherShown = true;
+                    switcher.ShowModal();
+                    s_viewportSwitcherShown = false;
 
-                if( idx >= 0 && idx < (int) mru.size() )
+                    int idx = switcher.GetSelection();
-                    m_appearancePanel->ApplyViewport( mru[idx] );
 
-                return true;
+                    if( idx >= 0 && idx < (int) mru.size() )
+                        m_appearancePanel->ApplyViewport( mru[idx] );
+
+                    return true;
+                }
             }
         }
     }

pcbnew/widgets/appearance_controls.cpp

@@ -48,6 +48,7 @@
 #include <widgets/indicator_icon.h>
 #include <widgets/infobar.h>
 #include <widgets/wx_grid.h>
+#include <dialogs/eda_view_switcher.h>
 #include <wx/bmpbuttn.h>
 #include <wx/checkbox.h>
 #include <wx/hyperlink.h>
@@ -443,6 +444,20 @@ APPEARANCE_CONTROLS::APPEARANCE_CONTROLS( PCB_BASE_FRAME* aParent, wxWindow* aFo
     m_presetsLabel->SetFont( infoFont );
     m_viewportsLabel->SetFont( infoFont );
 
+    m_cbLayerPresets->SetToolTip( wxString::Format( _( "Save and restore layer visibility combinations.\n"
+                                                       "Use %sTab to activate selector.\n"
+                                                       "Successive Tabs while holding %s down will "
+                                                       "cycle through presets in the popup." ),
+                                                    KeyNameFromKeyCode( PRESET_SWITCH_KEY ),
+                                                    KeyNameFromKeyCode( PRESET_SWITCH_KEY ) ) );
+
+    m_cbViewports->SetToolTip( wxString::Format( _( "Save and restore view location and zoom.\n"
+                                                    "Use %sTab to activate selector.\n"
+                                                    "Successive Tabs while holding %s down will "
+                                                    "cycle through viewports in the popup." ),
+                                                 KeyNameFromKeyCode( VIEWPORT_SWITCH_KEY ),
+                                                 KeyNameFromKeyCode( VIEWPORT_SWITCH_KEY ) ) );
+
     createControls();
 
     m_btnNetInspector->SetBitmap( KiBitmap( BITMAPS::list_nets_16 ) );
@@ -2429,7 +2444,8 @@ void APPEARANCE_CONTROLS::rebuildNets()
 
 void APPEARANCE_CONTROLS::rebuildLayerPresetsWidget()
 {
-    m_presetsLabel->SetLabel( _( "Presets (Ctrl+Tab):" ) );
+    m_viewportsLabel->SetLabel( wxString::Format( _( "Presets (%sTab):" ),
+                                                  KeyNameFromKeyCode( PRESET_SWITCH_KEY ) ) );
 
     m_cbLayerPresets->Clear();
 
@@ -2681,7 +2697,8 @@ void APPEARANCE_CONTROLS::doApplyLayerPreset( const LAYER_PRESET& aPreset )
 
 void APPEARANCE_CONTROLS::rebuildViewportsWidget()
 {
-    m_viewportsLabel->SetLabel( _( "Viewports (Alt+Tab):" ) );
+    m_viewportsLabel->SetLabel( wxString::Format( _( "Viewports (%sTab):" ),
+                                                  KeyNameFromKeyCode( VIEWPORT_SWITCH_KEY ) ) );
 
     m_cbViewports->Clear();
 

pcbnew/widgets/appearance_controls_base.cpp

@@ -166,8 +166,6 @@ APPEARANCE_CONTROLS_BASE::APPEARANCE_CONTROLS_BASE( wxWindow* parent, wxWindowID
 	int m_cbLayerPresetsNChoices = sizeof( m_cbLayerPresetsChoices ) / sizeof( wxString );
 	m_cbLayerPresets = new wxChoice( this, wxID_ANY, wxDefaultPosition, wxDefaultSize, m_cbLayerPresetsNChoices, m_cbLayerPresetsChoices, 0 );
 	m_cbLayerPresets->SetSelection( 1 );
-	m_cbLayerPresets->SetToolTip( _("Save and restore layer visibility combinations.  Use Shift+Tab to activate selector.  Successive Tabs while holding Shift down will cycle through presets in popup.") );
-
 	bPresets->Add( m_cbLayerPresets, 0, wxALL|wxEXPAND, 2 );
 
 
@@ -187,8 +185,6 @@ APPEARANCE_CONTROLS_BASE::APPEARANCE_CONTROLS_BASE( wxWindow* parent, wxWindowID
 	int m_cbViewportsNChoices = sizeof( m_cbViewportsChoices ) / sizeof( wxString );
 	m_cbViewports = new wxChoice( this, wxID_ANY, wxDefaultPosition, wxDefaultSize, m_cbViewportsNChoices, m_cbViewportsChoices, 0 );
 	m_cbViewports->SetSelection( 1 );
-	m_cbViewports->SetToolTip( _("Save and restore view orientation and zoom.  Use Shift+Tab to activate selector.  Successive Tabs while holding Shift down will cycle through viewports in popup.") );
-
 	bViewports->Add( m_cbViewports, 0, wxALL|wxEXPAND, 2 );
 
 

pcbnew/widgets/appearance_controls_base.fbp

@@ -1243,7 +1243,7 @@
                                         <property name="style"></property>
                                         <property name="subclass">; ; forward_declare</property>
                                         <property name="toolbar_pane">0</property>
-                                        <property name="tooltip">Save and restore layer visibility combinations.  Use Shift+Tab to activate selector.  Successive Tabs while holding Shift down will cycle through presets in popup.</property>
+                                        <property name="tooltip"></property>
                                         <property name="validator_data_type"></property>
                                         <property name="validator_style">wxFILTER_NONE</property>
                                         <property name="validator_type">wxDefaultValidator</property>
@@ -1390,7 +1390,7 @@
                                         <property name="style"></property>
                                         <property name="subclass">; ; forward_declare</property>
                                         <property name="toolbar_pane">0</property>
-                                        <property name="tooltip">Save and restore view orientation and zoom.  Use Shift+Tab to activate selector.  Successive Tabs while holding Shift down will cycle through viewports in popup.</property>
+                                        <property name="tooltip"></property>
                                         <property name="validator_data_type"></property>
                                         <property name="validator_style">wxFILTER_NONE</property>
                                         <property name="validator_type">wxDefaultValidator</property>