From 7835b8ddc746ddc40211cfc2d980a5e1b9c107f3 Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Mon, 25 Sep 2023 12:10:18 +0100
Subject: [PATCH] Bring various appearance managers into line over preset
 editing.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/15726
---
 3d-viewer/dialogs/appearance_controls_3D.cpp  | 56 ++++++++++++-------
 .../dialogs/dialog_symbol_fields_table.cpp    | 18 +++---
 pcbnew/widgets/appearance_controls.cpp        | 23 ++++++--
 3 files changed, 61 insertions(+), 36 deletions(-)

diff --git a/3d-viewer/dialogs/appearance_controls_3D.cpp b/3d-viewer/dialogs/appearance_controls_3D.cpp
index fe79ca36cb..b0e97168d3 100644
--- a/3d-viewer/dialogs/appearance_controls_3D.cpp
+++ b/3d-viewer/dialogs/appearance_controls_3D.cpp
@@ -21,6 +21,7 @@
 #include <dialogs/appearance_controls_3D.h>
 
 #include <bitmaps.h>
+#include <confirm.h>
 #include <pgm_base.h>
 #include <dpi_scaling_common.h>
 #include <eda_list_dialog.h>
@@ -616,6 +617,12 @@ void APPEARANCE_CONTROLS_3D::onLayerPresetChanged( wxCommandEvent& aEvent )
     int                       index = m_cbLayerPresets->GetSelection();
     wxString                  name;
 
+    auto resetSelection =
+            [&]()
+            {
+                updateLayerPresetWidget( cfg->m_CurrentPreset );
+            };
+
     if( index == 0 )
     {
         name = FOLLOW_PCB;
@@ -633,7 +640,7 @@ void APPEARANCE_CONTROLS_3D::onLayerPresetChanged( wxCommandEvent& aEvent )
     else if( index == count - 3 )
     {
         // Separator: reject the selection
-        updateLayerPresetWidget( cfg->m_CurrentPreset );
+        resetSelection();
         return;
     }
     else if( index == count - 2 )
@@ -642,30 +649,37 @@ void APPEARANCE_CONTROLS_3D::onLayerPresetChanged( wxCommandEvent& aEvent )
 
         if( dlg.ShowModal() != wxID_OK )
         {
-            updateLayerPresetWidget( cfg->m_CurrentPreset );
+            resetSelection();
             return;
         }
+
+        std::bitset<LAYER_3D_END> visibleLayers = m_frame->GetAdapter().GetVisibleLayers();
+        std::map<int, COLOR4D>    colors = m_frame->GetAdapter().GetLayerColors();
+
+        name = dlg.GetValue();
+
+        if( LAYER_PRESET_3D* preset = cfg->FindPreset( name ) )
+        {
+            if( !IsOK( this, _( "Overwrite existing preset?" ) ) )
+            {
+                resetSelection();
+                return;
+            }
+
+            preset->layers = visibleLayers;
+            preset->colors = colors;
+            m_cbLayerPresets->SetSelection( m_cbLayerPresets->FindString( name ) );
+        }
         else
         {
-            std::bitset<LAYER_3D_END> visibleLayers = m_frame->GetAdapter().GetVisibleLayers();
-            std::map<int, COLOR4D>    colors = m_frame->GetAdapter().GetLayerColors();
-
-            name = dlg.GetValue();
-
-            if( LAYER_PRESET_3D* preset = cfg->FindPreset( name ) )
-            {
-                preset->layers = visibleLayers;
-                preset->colors = colors;
-                m_cbLayerPresets->SetSelection( m_cbLayerPresets->FindString( name ) );
-            }
-            else
-            {
-                cfg->m_LayerPresets.emplace_back( name, visibleLayers, colors );
-                m_cbLayerPresets->SetSelection( m_cbLayerPresets->Insert( name, index - 1 ) );
-            }
-
-            cfg->m_CurrentPreset = name;
+            cfg->m_LayerPresets.emplace_back( name, visibleLayers, colors );
+            m_cbLayerPresets->SetSelection( m_cbLayerPresets->Insert( name, index - 1 ) );
         }
+
+        cfg->m_CurrentPreset = name;
+        m_presetMRU.Insert( name, 0 );
+
+        return;
     }
     else if( index == count - 1 )
     {
@@ -703,7 +717,7 @@ void APPEARANCE_CONTROLS_3D::onLayerPresetChanged( wxCommandEvent& aEvent )
             m_presetMRU.Remove( name );
         }
 
-        updateLayerPresetWidget( cfg->m_CurrentPreset );
+        resetSelection();
         return;
     }
     else if( LAYER_PRESET_3D* preset = cfg->FindPreset( m_cbLayerPresets->GetStringSelection() ) )
diff --git a/eeschema/dialogs/dialog_symbol_fields_table.cpp b/eeschema/dialogs/dialog_symbol_fields_table.cpp
index aab779c8c4..cc2960750c 100644
--- a/eeschema/dialogs/dialog_symbol_fields_table.cpp
+++ b/eeschema/dialogs/dialog_symbol_fields_table.cpp
@@ -1573,16 +1573,15 @@ void DIALOG_SYMBOL_FIELDS_TABLE::onBomPresetChanged( wxCommandEvent& aEvent )
         }
         else if( preset->readOnly )
         {
-            wxMessageBox( _( "Cannot modify default presets." ), _( "Error" ), wxOK | wxICON_ERROR,
-                          this );
+            wxMessageBox( _( "Default presets cannot be modified.\nPlease use a different name." ),
+                          _( "Error" ), wxOK | wxICON_ERROR, this );
+            resetSelection();
             return;
         }
         else
         {
             // Ask the user if they want to overwrite the existing preset
-            if( wxMessageBox( _( "Overwrite existing preset?" ), _( "Save BOM Preset" ),
-                              wxYES_NO | wxICON_QUESTION, this )
-                != wxYES )
+            if( !IsOK( this, _( "Overwrite existing preset?" ) ) )
             {
                 resetSelection();
                 return;
@@ -1975,16 +1974,15 @@ void DIALOG_SYMBOL_FIELDS_TABLE::onBomFmtPresetChanged( wxCommandEvent& aEvent )
         }
         else if( preset->readOnly )
         {
-            wxMessageBox( _( "Cannot modify default presets." ), _( "Error" ), wxOK | wxICON_ERROR,
-                          this );
+            wxMessageBox( _( "Default presets cannot be modified.\nPlease use a different name." ),
+                          _( "Error" ), wxOK | wxICON_ERROR, this );
+            resetSelection();
             return;
         }
         else
         {
             // Ask the user if they want to overwrite the existing preset
-            if( wxMessageBox( _( "Overwrite existing preset?" ), _( "Save BOM Preset" ),
-                              wxYES_NO | wxICON_QUESTION, this )
-                != wxYES )
+            if( !IsOK( this, _( "Overwrite existing preset?" ) ) )
             {
                 resetSelection();
                 return;
diff --git a/pcbnew/widgets/appearance_controls.cpp b/pcbnew/widgets/appearance_controls.cpp
index 8298847128..6d52c080eb 100644
--- a/pcbnew/widgets/appearance_controls.cpp
+++ b/pcbnew/widgets/appearance_controls.cpp
@@ -25,11 +25,11 @@
 #include <board_design_settings.h>
 #include <pad.h>
 #include <pcb_track.h>
-#include <dpi_scaling_common.h>
 #include <eda_list_dialog.h>
 #include <string_utils.h>
 #include <footprint_edit_frame.h>
 #include <menus_helpers.h>
+#include <confirm.h>
 #include <pcb_display_options.h>
 #include <pcb_edit_frame.h>
 #include <pcb_painter.h>
@@ -51,7 +51,6 @@
 #include <widgets/wx_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>
 #include <wx/radiobut.h>
@@ -2639,20 +2638,33 @@ void APPEARANCE_CONTROLS::onLayerPresetChanged( wxCommandEvent& aEvent )
 
         if( !exists )
         {
-            m_layerPresets[name] = LAYER_PRESET( name, getVisibleLayers(),
-                                                 getVisibleObjects(), UNSELECTED_LAYER );
+            m_layerPresets[name] = LAYER_PRESET( name, getVisibleLayers(), getVisibleObjects(),
+                                                 UNSELECTED_LAYER );
         }
 
         LAYER_PRESET* preset = &m_layerPresets[name];
-        m_currentPreset      = preset;
 
         if( !exists )
         {
             index = m_cbLayerPresets->Insert( name, index - 1, static_cast<void*>( preset ) );
             preset->flipBoard = m_cbFlipBoard->GetValue();
         }
+        else if( preset->readOnly )
+        {
+            wxMessageBox( _( "Default presets cannot be modified.\nPlease use a different name." ),
+                          _( "Error" ), wxOK | wxICON_ERROR, this );
+            resetSelection();
+            return;
+        }
         else
         {
+            // Ask the user if they want to overwrite the existing preset
+            if( !IsOK( this, _( "Overwrite existing preset?" ) ) )
+            {
+                resetSelection();
+                return;
+            }
+
             preset->layers       = getVisibleLayers();
             preset->renderLayers = getVisibleObjects();
             preset->flipBoard    = m_cbFlipBoard->GetValue();
@@ -2661,6 +2673,7 @@ void APPEARANCE_CONTROLS::onLayerPresetChanged( wxCommandEvent& aEvent )
             m_presetMRU.Remove( name );
         }
 
+        m_currentPreset      = preset;
         m_cbLayerPresets->SetSelection( index );
         m_presetMRU.Insert( name, 0 );