From d827bb8a1fe1aae375ecc4a3cb76623c57f4a073 Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Wed, 31 May 2023 10:21:49 +0100
Subject: [PATCH] Leave back/forward/up nav buttons enabled so the action
 doesn't change.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/14783
---
 eeschema/sch_edit_frame.cpp          | 20 ++++++-----------
 eeschema/tools/sch_navigate_tool.cpp | 33 ++++++++++++++++++++++++----
 2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp
index 4b54e87e6a..78e7ebf249 100644
--- a/eeschema/sch_edit_frame.cpp
+++ b/eeschema/sch_edit_frame.cpp
@@ -611,18 +611,6 @@ void SCH_EDIT_FRAME::setupUIConditions()
                 return m_toolManager->GetTool<SCH_NAVIGATE_TOOL>()->CanGoUp();
             };
 
-    auto navHistoryHasForward =
-            [this]( const SELECTION& aSel )
-            {
-                return m_toolManager->GetTool<SCH_NAVIGATE_TOOL>()->CanGoForward();
-            };
-
-    auto navHistoryHasBackward =
-            [this]( const SELECTION& aSel )
-            {
-                return m_toolManager->GetTool<SCH_NAVIGATE_TOOL>()->CanGoBack();
-            };
-
     auto navSchematicHasPreviousSheet =
             [this]( const SELECTION& aSel )
             {
@@ -636,9 +624,15 @@ void SCH_EDIT_FRAME::setupUIConditions()
             };
 
     mgr->SetConditions( EE_ACTIONS::leaveSheet,            ENABLE( belowRootSheetCondition ) );
+
+    /* Some of these are bound by default to arrow keys which will get a different action if we
+     * disable the buttons.  So always leave them enabled so the action is consistent.
+     * https://gitlab.com/kicad/code/kicad/-/issues/14783
     mgr->SetConditions( EE_ACTIONS::navigateUp,            ENABLE( belowRootSheetCondition ) );
     mgr->SetConditions( EE_ACTIONS::navigateForward,       ENABLE( navHistoryHasForward ) );
-    mgr->SetConditions( EE_ACTIONS::navigateBack,          ENABLE( navHistoryHasBackward ) );
+    mgr->SetConditions( EE_ACTIONS::navigateBack,          ENABLE( navHistoryHsBackward ) );
+     */
+
     mgr->SetConditions( EE_ACTIONS::navigatePrevious,      ENABLE( navSchematicHasPreviousSheet ) );
     mgr->SetConditions( EE_ACTIONS::navigateNext,          ENABLE( navSchematicHasNextSheet ) );
     mgr->SetConditions( EE_ACTIONS::remapSymbols,          ENABLE( remapSymbolsCondition ) );
diff --git a/eeschema/tools/sch_navigate_tool.cpp b/eeschema/tools/sch_navigate_tool.cpp
index 1ca2bb3a92..f9dba7df30 100644
--- a/eeschema/tools/sch_navigate_tool.cpp
+++ b/eeschema/tools/sch_navigate_tool.cpp
@@ -47,6 +47,7 @@ void SCH_NAVIGATE_TOOL::CleanHistory()
     // Search through our history, and removing any entries
     // that the no longer point to a sheet on the schematic
     auto entry = m_navHistory.begin();
+
     while( entry != m_navHistory.end() )
     {
         if( std::find( sheets.begin(), sheets.end(), *entry ) != sheets.end() )
@@ -111,6 +112,10 @@ int SCH_NAVIGATE_TOOL::Forward( const TOOL_EVENT& aEvent )
         m_frame->SetCurrentSheet( *m_navIndex );
         m_frame->DisplayCurrentSheet();
     }
+    else
+    {
+        wxBell();
+    }
 
     return 0;
 }
@@ -128,6 +133,10 @@ int SCH_NAVIGATE_TOOL::Back( const TOOL_EVENT& aEvent )
         m_frame->SetCurrentSheet( *m_navIndex );
         m_frame->DisplayCurrentSheet();
     }
+    else
+    {
+        wxBell();
+    }
 
     return 0;
 }
@@ -136,8 +145,14 @@ int SCH_NAVIGATE_TOOL::Back( const TOOL_EVENT& aEvent )
 int SCH_NAVIGATE_TOOL::Previous( const TOOL_EVENT& aEvent )
 {
     if( CanGoPrevious() )
-        changeSheet( m_frame->Schematic().GetSheets().at(
-                m_frame->GetCurrentSheet().GetVirtualPageNumber() - 2 ) );
+    {
+        int targetSheet = m_frame->GetCurrentSheet().GetVirtualPageNumber() - 1;
+        changeSheet( m_frame->Schematic().GetSheets().at( targetSheet - 1 ) );
+    }
+    else
+    {
+        wxBell();
+    }
 
     return 0;
 }
@@ -146,8 +161,14 @@ int SCH_NAVIGATE_TOOL::Previous( const TOOL_EVENT& aEvent )
 int SCH_NAVIGATE_TOOL::Next( const TOOL_EVENT& aEvent )
 {
     if( CanGoNext() )
-        changeSheet( m_frame->Schematic().GetSheets().at(
-                m_frame->GetCurrentSheet().GetVirtualPageNumber() ) );
+    {
+        int targetSheet = m_frame->GetCurrentSheet().GetVirtualPageNumber() + 1;
+        changeSheet( m_frame->Schematic().GetSheets().at( targetSheet - 1 ) );
+    }
+    else
+    {
+        wxBell();
+    }
 
     return 0;
 }
@@ -221,6 +242,10 @@ int SCH_NAVIGATE_TOOL::LeaveSheet( const TOOL_EVENT& aEvent )
 
         changeSheet( popped );
     }
+    else
+    {
+        wxBell();
+    }
 
     return 0;
 }