From e202b00a74f6c35c1dbd29f383e00d57f377631b Mon Sep 17 00:00:00 2001
From: Ian McInerney <ian.s.mcinerney@ieee.org>
Date: Fri, 28 Feb 2025 01:56:14 +0000
Subject: [PATCH] Rework the toolbar settings storage and panel

This format is more extendable in the future, should separator and
spacer support be added to groups.

Also, this now has a working UI for modifying the toolbars.
---
 .../dialogs/panel_toolbar_customization.cpp   | 162 ++++++++++++---
 .../dialogs/panel_toolbar_customization.fbp   |   1 +
 .../panel_toolbar_customization_base.cpp      |   2 +
 .../panel_toolbar_customization_base.h        |   1 +
 common/eda_base_frame.cpp                     |   3 +-
 common/tool/action_toolbar.cpp                |  23 ++-
 common/tool/ui/toolbar_configuration.cpp      | 194 +++++++++---------
 include/dialogs/panel_toolbar_customization.h |  24 ++-
 include/eda_base_frame.h                      |   2 +-
 include/eda_draw_frame.h                      |   1 -
 include/settings/settings_manager.h           |   1 +
 include/tool/ui/toolbar_configuration.h       |  30 +--
 12 files changed, 299 insertions(+), 145 deletions(-)

diff --git a/common/dialogs/panel_toolbar_customization.cpp b/common/dialogs/panel_toolbar_customization.cpp
index b9ad99aa5a..f7d48a5905 100644
--- a/common/dialogs/panel_toolbar_customization.cpp
+++ b/common/dialogs/panel_toolbar_customization.cpp
@@ -29,6 +29,7 @@
 #include <widgets/split_button.h>
 #include <widgets/std_bitmap_button.h>
 
+#include <magic_enum.hpp>
 #include <wx/listctrl.h>
 #include <wx/menu.h>
 
@@ -41,6 +42,14 @@ enum
 };
 
 
+static std::map<TOOLBAR_LOC, wxString> s_toolbarNameMap = {
+    { TOOLBAR_LOC::LEFT,     _( "Left" ) },
+    { TOOLBAR_LOC::RIGHT,    _( "Right" ) },
+    { TOOLBAR_LOC::TOP_AUX,  _( "Top main" ) },
+    { TOOLBAR_LOC::TOP_MAIN, _( "Top auxillary" ) }
+};
+
+
 class TOOLBAR_TREE_ITEM_DATA : public wxTreeItemData
 {
 public:
@@ -76,8 +85,8 @@ public:
     void SetAction( TOOL_ACTION* aAction ) { m_action = aAction; }
     TOOL_ACTION* GetAction() const         { return m_action; }
 
-    void SetName( wxString& aName )  { m_name = aName; }
-    const wxString& GetName() const    { return m_name; }
+    void SetName( const wxString& aName ) { m_name = aName; }
+    const wxString& GetName() const       { return m_name; }
 
     void SetSize( int aSize )  { m_size = aSize; }
     int GetSize() const        { return m_size; }
@@ -106,7 +115,7 @@ PANEL_TOOLBAR_CUSTOMIZATION::PANEL_TOOLBAR_CUSTOMIZATION( wxWindow* aParent, APP
         PANEL_TOOLBAR_CUSTOMIZATION_BASE( aParent ),
         m_actionImageList( nullptr ),
         m_appSettings( aCfg ),
-        m_tbSettings( aTbSettings )
+        m_appTbSettings( aTbSettings )
 {
     // Copy the tools and controls into the internal maps
     for( auto& tool : aTools )
@@ -149,24 +158,68 @@ PANEL_TOOLBAR_CUSTOMIZATION::~PANEL_TOOLBAR_CUSTOMIZATION()
     delete m_actionImageList;
 }
 
+
 void PANEL_TOOLBAR_CUSTOMIZATION::ResetPanel()
 {
+    // Go over every toolbar and initialize things
+    for( auto& tb : magic_enum::enum_values<TOOLBAR_LOC>() )
+    {
+        // Create a shadow toolbar
+        auto tbConfig = m_appTbSettings->DefaultToolbarConfig( tb );
+
+        if( tbConfig.has_value() )
+            m_toolbars[tb] = tbConfig.value();
+    }
+
+    // Populate the toolbar view with the default toolbar
+    m_tbChoice->SetSelection( 0 );
+
+    auto firstTb = magic_enum::enum_cast<TOOLBAR_LOC>( 0 );
+
+    if( firstTb.has_value() )
+        m_currentToolbar = firstTb.value();
+
+    populateToolbarTree();
 
 }
 
 
 bool PANEL_TOOLBAR_CUSTOMIZATION::TransferDataToWindow()
 {
-    auto tb = m_tbSettings->GetToolbarConfig( TOOLBAR_LOC::RIGHT, false );
+    wxArrayString tbChoices;
+
+    // Go over every toolbar and initialize things
+    for( auto& tb : magic_enum::enum_values<TOOLBAR_LOC>() )
+    {
+        // Create a shadow toolbar
+        auto tbConfig = m_appTbSettings->GetToolbarConfig( tb );
+
+        if( tbConfig.has_value() )
+            m_toolbars.emplace( tb, tbConfig.value() );
+
+        // Setup the UI name
+        const auto& tbName = s_toolbarNameMap.find( tb );
+
+        wxASSERT_MSG( tbName != s_toolbarNameMap.end(),
+                      wxString::Format( "Unknown toolbar: %s", magic_enum::enum_name( tb ) ) );
+
+        tbChoices.Add( tbName->second );
+    }
+
+    m_tbChoice->Set( tbChoices );
 
     // Always populate the actions before the toolbars, that way the icons are available
-    populateActions( m_availableTools, m_availableControls );
+    populateActions();
 
-    // Populate the choicebox to select the toolbar to edit
+    // Populate the toolbar view
+    m_tbChoice->SetSelection( 0 );
 
+    auto firstTb = magic_enum::enum_cast<TOOLBAR_LOC>( 0 );
 
-    if( tb.has_value() )
-        populateToolbarTree( tb.value() );
+    if( firstTb.has_value() )
+        m_currentToolbar = firstTb.value();
+
+    populateToolbarTree();
 
     // Sync the enable/disable control
     enableCustomControls( m_appSettings->m_CustomToolbars );
@@ -180,16 +233,31 @@ bool PANEL_TOOLBAR_CUSTOMIZATION::TransferDataFromWindow()
 {
     m_appSettings->m_CustomToolbars = m_customToolbars->GetValue();
 
+    // Store the current toolbar
+    auto currentTb = parseToolbarTree();
+
+    if( currentTb.has_value() )
+        m_toolbars[m_currentToolbar] = currentTb.value();
+
+    // Write the shadow toolbars with changes back to the app toolbar settings
+    for( auto& tb : m_toolbars )
+        m_appTbSettings->SetStoredToolbarConfig( tb.first, tb.second );
+
     return true;
 }
 
 
-void PANEL_TOOLBAR_CUSTOMIZATION::parseToolbarTree( TOOLBAR_CONFIGURATION& aToolbar )
+std::optional<TOOLBAR_CONFIGURATION> PANEL_TOOLBAR_CUSTOMIZATION::parseToolbarTree()
 {
+    TOOLBAR_CONFIGURATION config;
+
     wxTreeItemId      mainId;
     wxTreeItemId      rootId = m_toolbarTree->GetRootItem();
     wxTreeItemIdValue mainCookie;
 
+    if( !rootId.IsOk() )
+        return std::nullopt;
+
     mainId = m_toolbarTree->GetFirstChild( rootId, mainCookie );
 
     while( mainId.IsOk() )
@@ -203,18 +271,19 @@ void PANEL_TOOLBAR_CUSTOMIZATION::parseToolbarTree( TOOLBAR_CONFIGURATION& aTool
         switch( tbData->GetType() )
         {
         case TOOLBAR_ITEM_TYPE::SPACER:
-            aToolbar.AppendSpacer( tbData->GetSize() );
+            config.AppendSpacer( tbData->GetSize() );
             break;
 
         case TOOLBAR_ITEM_TYPE::SEPARATOR:
-            aToolbar.AppendSeparator();
+            config.AppendSeparator();
             break;
 
         case TOOLBAR_ITEM_TYPE::CONTROL:
+            config.AppendControl( tbData->GetName().ToStdString() );
             break;
 
         case TOOLBAR_ITEM_TYPE::TOOL:
-            aToolbar.AppendAction( *( tbData->GetAction() ) );
+            config.AppendAction( *( tbData->GetAction() ) );
             break;
 
         case TOOLBAR_ITEM_TYPE::GROUP:
@@ -251,22 +320,38 @@ void PANEL_TOOLBAR_CUSTOMIZATION::parseToolbarTree( TOOLBAR_CONFIGURATION& aTool
                 }
             }
 
-            aToolbar.AppendGroup( grpConfig );
+            config.AppendGroup( grpConfig );
         }
 
         mainId = m_toolbarTree->GetNextChild( rootId, mainCookie );
     }
+
+    return config;
 }
 
 
-void PANEL_TOOLBAR_CUSTOMIZATION::populateToolbarTree( const TOOLBAR_CONFIGURATION& aToolbar )
+void PANEL_TOOLBAR_CUSTOMIZATION::populateToolbarTree()
 {
     m_toolbarTree->DeleteAllItems();
     m_toolbarTree->SetImageList( m_actionImageList );
 
+    const auto& it = m_toolbars.find( m_currentToolbar );
+
+    if( it == m_toolbars.end() )
+    {
+        // Disable the controls and bail out - no toolbar here
+        enableToolbarControls( false );
+        return;
+    }
+
+    // Ensure the controls are enabled
+    enableToolbarControls( true );
+
+    TOOLBAR_CONFIGURATION toolbar = it->second;
+
     wxTreeItemId root = m_toolbarTree->AddRoot( "Toolbar" );
 
-    for( auto& item : aToolbar.GetToolbarItems() )
+    for( auto& item : toolbar.GetToolbarItems() )
     {
         switch( item.m_Type )
         {
@@ -289,8 +374,14 @@ void PANEL_TOOLBAR_CUSTOMIZATION::populateToolbarTree( const TOOLBAR_CONFIGURATI
             }
 
         case TOOLBAR_ITEM_TYPE::CONTROL:
-            // TODO
+            {
+            // Add a control
+            TOOLBAR_TREE_ITEM_DATA* controlTreeItem = new TOOLBAR_TREE_ITEM_DATA( TOOLBAR_ITEM_TYPE::CONTROL );
+            controlTreeItem->SetName( item.m_ControlName );
+            m_toolbarTree->AppendItem( root, item.m_ControlName, -1, -1,
+                                       controlTreeItem );
             break;
+            }
 
         case TOOLBAR_ITEM_TYPE::TOOL:
             {
@@ -329,11 +420,11 @@ void PANEL_TOOLBAR_CUSTOMIZATION::populateToolbarTree( const TOOLBAR_CONFIGURATI
             // Add the elements below the group
             for( auto& groupItem : item.m_GroupItems )
             {
-                auto toolMap = m_availableTools.find( groupItem );
+                auto toolMap = m_availableTools.find( groupItem.m_ActionName );
 
                 if( toolMap == m_availableTools.end() )
                 {
-                    wxASSERT_MSG( false, wxString::Format( "Unable to find group tool %s", groupItem ) );
+                    wxASSERT_MSG( false, wxString::Format( "Unable to find group tool %s", groupItem.m_ActionName ) );
                     continue;
                 }
 
@@ -341,7 +432,7 @@ void PANEL_TOOLBAR_CUSTOMIZATION::populateToolbarTree( const TOOLBAR_CONFIGURATI
                 toolTreeItem->SetAction( toolMap->second );
 
                 int  imgIdx = -1;
-                auto imgMap = m_actionImageListMap.find( groupItem );
+                auto imgMap = m_actionImageListMap.find( groupItem.m_ActionName );
 
                 if( imgMap != m_actionImageListMap.end() )
                     imgIdx = imgMap->second;
@@ -367,8 +458,7 @@ void PANEL_TOOLBAR_CUSTOMIZATION::populateToolbarTree( const TOOLBAR_CONFIGURATI
 }
 
 
-void PANEL_TOOLBAR_CUSTOMIZATION::populateActions( const std::map<std::string, TOOL_ACTION*>& aTools,
-                                                   const std::map<std::string, ACTION_TOOLBAR_CONTROL*>& aControls )
+void PANEL_TOOLBAR_CUSTOMIZATION::populateActions()
 {
     // Clear all existing information for the actions
     delete m_actionImageList;
@@ -407,12 +497,12 @@ void PANEL_TOOLBAR_CUSTOMIZATION::populateActions( const std::map<std::string, T
     };
 
     m_actionImageList = new wxImageList( logicSize, logicSize, true,
-                                         static_cast<int>( aTools.size() ) );
+                                         static_cast<int>( m_availableTools.size() ) );
 
     // Populate the various image lists for the action icons, and the actual control
     int itemIdx = 0;
 
-    for( auto [k, tool] : aTools )
+    for( auto [k, tool] : m_availableTools )
     {
         if( tool->CheckToolbarState( TOOLBAR_STATE::HIDDEN ) )
             continue;
@@ -576,6 +666,12 @@ void PANEL_TOOLBAR_CUSTOMIZATION::onCustomizeTbCb( wxCommandEvent& event )
 void PANEL_TOOLBAR_CUSTOMIZATION::enableCustomControls( bool enable )
 {
     m_tbChoice->Enable( enable );
+    enableToolbarControls( enable );
+}
+
+
+void PANEL_TOOLBAR_CUSTOMIZATION::enableToolbarControls( bool enable )
+{
     m_toolbarTree->Enable( enable );
     m_btnAddTool->Enable( enable );
     m_btnToolDelete->Enable( enable );
@@ -623,6 +719,7 @@ void PANEL_TOOLBAR_CUSTOMIZATION::onToolMoveDown( wxCommandEvent& event )
 
 }
 
+
 void PANEL_TOOLBAR_CUSTOMIZATION::onBtnAddAction( wxCommandEvent& event )
 {
     // Get the selected item
@@ -719,3 +816,22 @@ void PANEL_TOOLBAR_CUSTOMIZATION::onTreeEndLabelEdit( wxTreeEvent& event )
 {
 
 }
+
+
+void PANEL_TOOLBAR_CUSTOMIZATION::onTbChoiceSelect( wxCommandEvent& event )
+{
+    // Store the current toolbar
+    auto currentTb = parseToolbarTree();
+
+    if( currentTb.has_value() )
+        m_toolbars[m_currentToolbar] = currentTb.value();
+
+    // Populate the new one
+    auto newTb = magic_enum::enum_cast<TOOLBAR_LOC>( event.GetInt() );
+
+    if( newTb.has_value() )
+    {
+        m_currentToolbar = newTb.value();
+        populateToolbarTree();
+    }
+}
diff --git a/common/dialogs/panel_toolbar_customization.fbp b/common/dialogs/panel_toolbar_customization.fbp
index 0de2be7bf3..468d2416b4 100644
--- a/common/dialogs/panel_toolbar_customization.fbp
+++ b/common/dialogs/panel_toolbar_customization.fbp
@@ -274,6 +274,7 @@
                     <property name="window_extra_style"></property>
                     <property name="window_name"></property>
                     <property name="window_style"></property>
+                    <event name="OnChoice">onTbChoiceSelect</event>
                   </object>
                 </object>
                 <object class="sizeritem" expanded="true">
diff --git a/common/dialogs/panel_toolbar_customization_base.cpp b/common/dialogs/panel_toolbar_customization_base.cpp
index 476865f8e1..ec43e37aca 100644
--- a/common/dialogs/panel_toolbar_customization_base.cpp
+++ b/common/dialogs/panel_toolbar_customization_base.cpp
@@ -109,6 +109,7 @@ PANEL_TOOLBAR_CUSTOMIZATION_BASE::PANEL_TOOLBAR_CUSTOMIZATION_BASE( wxWindow* pa
 	// Connect Events
 	this->Connect( wxEVT_UPDATE_UI, wxUpdateUIEventHandler( PANEL_TOOLBAR_CUSTOMIZATION_BASE::OnUpdateUI ) );
 	m_customToolbars->Connect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( PANEL_TOOLBAR_CUSTOMIZATION_BASE::onCustomizeTbCb ), NULL, this );
+	m_tbChoice->Connect( wxEVT_COMMAND_CHOICE_SELECTED, wxCommandEventHandler( PANEL_TOOLBAR_CUSTOMIZATION_BASE::onTbChoiceSelect ), NULL, this );
 	m_toolbarTree->Connect( wxEVT_COMMAND_TREE_BEGIN_LABEL_EDIT, wxTreeEventHandler( PANEL_TOOLBAR_CUSTOMIZATION_BASE::onTreeBeginLabelEdit ), NULL, this );
 	m_toolbarTree->Connect( wxEVT_COMMAND_TREE_END_LABEL_EDIT, wxTreeEventHandler( PANEL_TOOLBAR_CUSTOMIZATION_BASE::onTreeEndLabelEdit ), NULL, this );
 	m_btnToolDelete->Connect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( PANEL_TOOLBAR_CUSTOMIZATION_BASE::onToolDelete ), NULL, this );
@@ -122,6 +123,7 @@ PANEL_TOOLBAR_CUSTOMIZATION_BASE::~PANEL_TOOLBAR_CUSTOMIZATION_BASE()
 	// Disconnect Events
 	this->Disconnect( wxEVT_UPDATE_UI, wxUpdateUIEventHandler( PANEL_TOOLBAR_CUSTOMIZATION_BASE::OnUpdateUI ) );
 	m_customToolbars->Disconnect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( PANEL_TOOLBAR_CUSTOMIZATION_BASE::onCustomizeTbCb ), NULL, this );
+	m_tbChoice->Disconnect( wxEVT_COMMAND_CHOICE_SELECTED, wxCommandEventHandler( PANEL_TOOLBAR_CUSTOMIZATION_BASE::onTbChoiceSelect ), NULL, this );
 	m_toolbarTree->Disconnect( wxEVT_COMMAND_TREE_BEGIN_LABEL_EDIT, wxTreeEventHandler( PANEL_TOOLBAR_CUSTOMIZATION_BASE::onTreeBeginLabelEdit ), NULL, this );
 	m_toolbarTree->Disconnect( wxEVT_COMMAND_TREE_END_LABEL_EDIT, wxTreeEventHandler( PANEL_TOOLBAR_CUSTOMIZATION_BASE::onTreeEndLabelEdit ), NULL, this );
 	m_btnToolDelete->Disconnect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( PANEL_TOOLBAR_CUSTOMIZATION_BASE::onToolDelete ), NULL, this );
diff --git a/common/dialogs/panel_toolbar_customization_base.h b/common/dialogs/panel_toolbar_customization_base.h
index f10781b2c1..f7461caefe 100644
--- a/common/dialogs/panel_toolbar_customization_base.h
+++ b/common/dialogs/panel_toolbar_customization_base.h
@@ -56,6 +56,7 @@ class PANEL_TOOLBAR_CUSTOMIZATION_BASE : public RESETTABLE_PANEL
 		// Virtual event handlers, override them in your derived class
 		virtual void OnUpdateUI( wxUpdateUIEvent& event ) { event.Skip(); }
 		virtual void onCustomizeTbCb( wxCommandEvent& event ) { event.Skip(); }
+		virtual void onTbChoiceSelect( wxCommandEvent& event ) { event.Skip(); }
 		virtual void onTreeBeginLabelEdit( wxTreeEvent& event ) { event.Skip(); }
 		virtual void onTreeEndLabelEdit( wxTreeEvent& event ) { event.Skip(); }
 		virtual void onToolDelete( wxCommandEvent& event ) { event.Skip(); }
diff --git a/common/eda_base_frame.cpp b/common/eda_base_frame.cpp
index c0cb71da68..703ab67b82 100644
--- a/common/eda_base_frame.cpp
+++ b/common/eda_base_frame.cpp
@@ -723,8 +723,7 @@ void EDA_BASE_FRAME::CommonSettingsChanged( int aFlags )
         GetMenuBar()->Refresh();
     }
 
-    // Update the toolbars by loading the settings from disk
-    m_toolbarSettings->LoadFromFile( Pgm().GetSettingsManager().GetToolbarSettingsPath() );
+    // Update the toolbars
     RecreateToolbars();
 }
 
diff --git a/common/tool/action_toolbar.cpp b/common/tool/action_toolbar.cpp
index 095818fe67..e4ea2ce616 100644
--- a/common/tool/action_toolbar.cpp
+++ b/common/tool/action_toolbar.cpp
@@ -287,15 +287,26 @@ void ACTION_TOOLBAR::ApplyConfiguration( const TOOLBAR_CONFIGURATION& aConfig )
 
             for( auto& groupItem : item.m_GroupItems )
             {
-                TOOL_ACTION* action = m_toolManager->GetActionManager()->FindAction( groupItem );
-
-                if( !action )
+                switch( groupItem.m_Type )
                 {
-                    wxASSERT_MSG( false, wxString::Format( "Unable to find group tool %s", groupItem ) );
+                case TOOLBAR_ITEM_TYPE::SEPARATOR:
+                case TOOLBAR_ITEM_TYPE::SPACER:
+                case TOOLBAR_ITEM_TYPE::GROUP:
+                case TOOLBAR_ITEM_TYPE::CONTROL:
+                    wxASSERT_MSG( false, "Unsupported group item type" );
                     continue;
-                }
 
-                tools.push_back( action );
+                case TOOLBAR_ITEM_TYPE::TOOL:
+                    TOOL_ACTION* grpAction = m_toolManager->GetActionManager()->FindAction( groupItem.m_ActionName );
+
+                    if( !grpAction )
+                    {
+                        wxASSERT_MSG( false, wxString::Format( "Unable to find group tool %s", groupItem.m_ActionName ) );
+                        continue;
+                    }
+
+                    tools.push_back( grpAction );
+                }
             }
 
             AddGroup( std::make_unique<ACTION_GROUP>( item.m_GroupName.ToStdString(), tools ) );
diff --git a/common/tool/ui/toolbar_configuration.cpp b/common/tool/ui/toolbar_configuration.cpp
index 296ed63185..b4eeba036c 100644
--- a/common/tool/ui/toolbar_configuration.cpp
+++ b/common/tool/ui/toolbar_configuration.cpp
@@ -31,50 +31,101 @@
 ///! Update the schema version whenever a migration is required
 const int toolbarSchemaVersion = 1;
 
+void to_json( nlohmann::json& aJson, const TOOLBAR_ITEM& aItem )
+{
+    aJson = { { "type", magic_enum::enum_name( aItem.m_Type ) } };
+
+    switch( aItem.m_Type )
+    {
+    case TOOLBAR_ITEM_TYPE::SEPARATOR:
+        // Nothing to add for a separator
+        break;
+
+    case TOOLBAR_ITEM_TYPE::SPACER:
+        aJson["size"] = aItem.m_Size;
+        break;
+
+    case TOOLBAR_ITEM_TYPE::CONTROL:
+        aJson["name"] = aItem.m_ControlName;
+        break;
+
+    case TOOLBAR_ITEM_TYPE::TOOL:
+        aJson["name"] = aItem.m_ActionName;
+        break;
+
+    case TOOLBAR_ITEM_TYPE::GROUP:
+        aJson["group_name"] = aItem.m_GroupName;
+
+        nlohmann::json grpItems = nlohmann::json::array();
+
+        for( const auto& it : aItem.m_GroupItems )
+            grpItems.push_back( it );
+
+        aJson["group_items"] = grpItems;
+
+        break;
+    }
+}
+
+
+void from_json( const nlohmann::json& aJson, TOOLBAR_ITEM& aItem )
+{
+    if( aJson.empty() )
+        return;
+
+    if( aJson.contains( "type" ) )
+    {
+        auto type = magic_enum::enum_cast<TOOLBAR_ITEM_TYPE>( aJson["type"].get<std::string>(),
+                                                            magic_enum::case_insensitive );
+
+        if( type.has_value() )
+            aItem.m_Type = type.value();
+    }
+
+    switch( aItem.m_Type )
+    {
+    case TOOLBAR_ITEM_TYPE::SEPARATOR:
+        // Nothing to read for a separator
+        break;
+
+    case TOOLBAR_ITEM_TYPE::SPACER:
+        if( aJson.contains( "size" ) )
+            aItem.m_Size = aJson["size"].get<int>();
+
+        break;
+
+    case TOOLBAR_ITEM_TYPE::CONTROL:
+        if( aJson.contains( "name" ) )
+            aItem.m_ControlName = aJson["name"].get<std::string>();
+
+        break;
+
+    case TOOLBAR_ITEM_TYPE::TOOL:
+        if( aJson.contains( "name" ) )
+            aItem.m_ActionName = aJson["name"].get<std::string>();
+
+        break;
+
+    case TOOLBAR_ITEM_TYPE::GROUP:
+        if( aJson.contains( "group_name" ) )
+            aItem.m_GroupName = aJson["group_name"].get<wxString>();
+
+        if( aJson.contains( "group_items" ) )
+        {
+            for( const nlohmann::json& it : aJson.at( "group_items" ) )
+                aItem.m_GroupItems.push_back( it.get<TOOLBAR_ITEM>() );
+        }
+        break;
+    }
+}
+
 
 void to_json( nlohmann::json& aJson, const TOOLBAR_CONFIGURATION& aConfig )
 {
     aJson = nlohmann::json::array();
 
     for( const TOOLBAR_ITEM& item : aConfig.m_toolbarItems )
-    {
-        nlohmann::json jsItem = {
-            { "type", magic_enum::enum_name( item.m_Type ) }
-        };
-
-        switch( item.m_Type )
-        {
-        case TOOLBAR_ITEM_TYPE::SEPARATOR:
-            // Nothing to add for a separator
-            break;
-
-        case TOOLBAR_ITEM_TYPE::SPACER:
-            jsItem["size"] = item.m_Size;
-            break;
-
-        case TOOLBAR_ITEM_TYPE::CONTROL:
-            jsItem["name"] = item.m_ControlName;
-            break;
-
-        case TOOLBAR_ITEM_TYPE::TOOL:
-            jsItem["name"] = item.m_ActionName;
-            break;
-
-        case TOOLBAR_ITEM_TYPE::GROUP:
-            jsItem["group_name"] = item.m_GroupName;
-
-            nlohmann::json grpItems = nlohmann::json::array();
-
-            for( const auto& it : item.m_GroupItems )
-                grpItems.push_back( it );
-
-            jsItem["group_items"] = grpItems;
-
-            break;
-        }
-
-        aJson.push_back( jsItem );
-    }
+        aJson.push_back( item );
 }
 
 
@@ -88,60 +139,7 @@ void from_json( const nlohmann::json& aJson, TOOLBAR_CONFIGURATION& aConfig )
     if( aJson.is_array() )
     {
         for( const nlohmann::json& item : aJson )
-        {
-            TOOLBAR_ITEM tbItem;
-
-            if( item.contains( "type" ) )
-            {
-                auto type = magic_enum::enum_cast<TOOLBAR_ITEM_TYPE>( item["type"].get<std::string>(),
-                                                                    magic_enum::case_insensitive );
-
-                if( type.has_value() )
-                    tbItem.m_Type = type.value();
-            }
-
-            switch( tbItem.m_Type )
-            {
-            case TOOLBAR_ITEM_TYPE::SEPARATOR:
-                // Nothing to read for a separator
-                break;
-
-            case TOOLBAR_ITEM_TYPE::SPACER:
-                if( item.contains( "size" ) )
-                    tbItem.m_Size = item["size"].get<int>();
-
-                break;
-
-            case TOOLBAR_ITEM_TYPE::CONTROL:
-                if( item.contains( "name" ) )
-                    tbItem.m_ControlName = item["name"].get<std::string>();
-
-                break;
-
-            case TOOLBAR_ITEM_TYPE::TOOL:
-                if( item.contains( "name" ) )
-                    tbItem.m_ActionName = item["name"].get<std::string>();
-
-                break;
-
-            case TOOLBAR_ITEM_TYPE::GROUP:
-                if( item.contains( "group_name" ) )
-                    tbItem.m_GroupName = item["group_name"].get<wxString>();
-
-                if( item.contains( "group_items" ) )
-                {
-                    for( const nlohmann::json& it : item["group_items"].at( "group_items" ) )
-                    {
-                        if( it.is_string() )
-                            tbItem.m_GroupItems.push_back( it.get<std::string>() );
-                    }
-                }
-                break;
-            }
-
-            // We just directly add the item to the config
-            aConfig.m_toolbarItems.push_back( tbItem );
-        }
+            aConfig.m_toolbarItems.push_back( item.get<TOOLBAR_ITEM>() );
     }
 }
 
@@ -204,3 +202,15 @@ std::optional<TOOLBAR_CONFIGURATION> TOOLBAR_SETTINGS::GetToolbarConfig( TOOLBAR
 
     return DefaultToolbarConfig( aToolbar );
 }
+
+
+std::optional<TOOLBAR_CONFIGURATION> TOOLBAR_SETTINGS::GetStoredToolbarConfig( TOOLBAR_LOC aToolbar )
+{
+    auto tb = m_toolbars.find( aToolbar );
+
+    if( tb != m_toolbars.end() )
+        return tb->second;
+
+    // Return a nullopt if no toolbar is configured
+    return std::nullopt;
+}
\ No newline at end of file
diff --git a/include/dialogs/panel_toolbar_customization.h b/include/dialogs/panel_toolbar_customization.h
index c45f77f90b..bbbd04f001 100644
--- a/include/dialogs/panel_toolbar_customization.h
+++ b/include/dialogs/panel_toolbar_customization.h
@@ -27,6 +27,7 @@
 #include <dialogs/panel_toolbar_customization_base.h>
 
 #include <tool/action_toolbar.h>
+#include <tool/ui/toolbar_configuration.h>
 
 #include <wx/bmpbndl.h>
 
@@ -34,7 +35,6 @@ class wxImageList;
 
 class APP_SETTINGS_BASE;
 class TOOL_ACTION;
-class TOOLBAR_SETTINGS;
 
 class PANEL_TOOLBAR_CUSTOMIZATION : public PANEL_TOOLBAR_CUSTOMIZATION_BASE
 {
@@ -51,14 +51,14 @@ public:
     bool TransferDataToWindow() override;
 
 protected:
-    void parseToolbarTree( TOOLBAR_CONFIGURATION& aToolbar );
+    std::optional<TOOLBAR_CONFIGURATION> parseToolbarTree();
 
-    void populateToolbarTree( const TOOLBAR_CONFIGURATION& aToolbar );
+    void populateToolbarTree();
 
-    void populateActions( const std::map<std::string, TOOL_ACTION*>& aTools,
-                          const std::map<std::string, ACTION_TOOLBAR_CONTROL*>& aControls );
+    void populateActions();
 
     void enableCustomControls( bool enable );
+    void enableToolbarControls( bool enable );
 
     void onGroupPress( wxCommandEvent& aEvent );
     void onSpacerPress( wxCommandEvent& aEvent );
@@ -72,17 +72,25 @@ protected:
     void onBtnAddAction( wxCommandEvent& event ) override;
     void onTreeBeginLabelEdit( wxTreeEvent& event ) override;
     void onTreeEndLabelEdit( wxTreeEvent& event ) override;
+    void onTbChoiceSelect( wxCommandEvent& event ) override;
 
 protected:
     wxImageList*               m_actionImageList;
     wxVector<wxBitmapBundle>   m_actionImageBundleVector;
     std::map<std::string, int> m_actionImageListMap;
 
+    // Actual settings for the frame
     APP_SETTINGS_BASE* m_appSettings;
-    TOOLBAR_SETTINGS*  m_tbSettings;
+    TOOLBAR_SETTINGS*  m_appTbSettings;
 
-    std::map<std::string, TOOL_ACTION*>               m_availableTools;
-    std::map<std::string, ACTION_TOOLBAR_CONTROL*>    m_availableControls;
+    // The toolbar currently being viewed
+    TOOLBAR_LOC  m_currentToolbar;
+
+    // Shadow copy of the toolbar configurations used to store the changes in the dialog
+    std::map<TOOLBAR_LOC, TOOLBAR_CONFIGURATION>   m_toolbars;
+
+    std::map<std::string, TOOL_ACTION*>            m_availableTools;
+    std::map<std::string, ACTION_TOOLBAR_CONTROL*> m_availableControls;
 };
 
  #endif /* PANEL_TOOLBAR_CUSTOMIZATION_H_ */
diff --git a/include/eda_base_frame.h b/include/eda_base_frame.h
index c136d90daa..5038f179c0 100644
--- a/include/eda_base_frame.h
+++ b/include/eda_base_frame.h
@@ -44,7 +44,6 @@
 #include <kiway_holder.h>
 #include <tool/action_toolbar.h>
 #include <tool/tools_holder.h>
-#include <tool/ui/toolbar_configuration.h>
 #include <widgets/ui_common.h>
 #include <widgets/wx_infobar.h>
 #include <undo_redo_container.h>
@@ -89,6 +88,7 @@ struct WINDOW_SETTINGS;
 struct WINDOW_STATE;
 class ACTION_MENU;
 class TOOL_INTERACTIVE;
+class TOOLBAR_SETTINGS;
 
 #define DEFAULT_MAX_UNDO_ITEMS 0
 #define ABS_MAX_UNDO_ITEMS (INT_MAX / 2)
diff --git a/include/eda_draw_frame.h b/include/eda_draw_frame.h
index b0eabc43f4..808652a586 100644
--- a/include/eda_draw_frame.h
+++ b/include/eda_draw_frame.h
@@ -36,7 +36,6 @@
 #include <class_draw_panel_gal.h>
 #include <kiid.h>
 #include <hotkeys_basic.h>
-#include <tool/ui/toolbar_configuration.h>
 #include <widgets/lib_tree.h>
 
 class EDA_ITEM;
diff --git a/include/settings/settings_manager.h b/include/settings/settings_manager.h
index 912fffa66e..47550d8f0f 100644
--- a/include/settings/settings_manager.h
+++ b/include/settings/settings_manager.h
@@ -183,6 +183,7 @@ public:
         }
         else
         {
+            std::cout << "Registering new file " << aFilename << std::endl;
             ret = RegisterSettings( new T );
         }
 
diff --git a/include/tool/ui/toolbar_configuration.h b/include/tool/ui/toolbar_configuration.h
index a457ef48d4..b7047aaf9f 100644
--- a/include/tool/ui/toolbar_configuration.h
+++ b/include/tool/ui/toolbar_configuration.h
@@ -81,8 +81,8 @@ public:
     int m_Size;
 
     // Group properties
-    wxString                 m_GroupName;
-    std::vector<std::string> m_GroupItems;
+    wxString                  m_GroupName;
+    std::vector<TOOLBAR_ITEM> m_GroupItems;
 };
 
 class KICOMMON_API TOOLBAR_GROUP_CONFIG
@@ -100,17 +100,18 @@ public:
 
     TOOLBAR_GROUP_CONFIG& AddAction( std::string aActionName )
     {
-        m_groupItems.push_back( aActionName );
+
+        m_groupItems.emplace_back( TOOLBAR_ITEM_TYPE::TOOL, aActionName );
         return *this;
     }
 
     TOOLBAR_GROUP_CONFIG& AddAction( const TOOL_ACTION& aAction )
     {
-        m_groupItems.push_back( aAction.GetName() );
+        m_groupItems.emplace_back( TOOLBAR_ITEM_TYPE::TOOL, aAction.GetName() );
         return *this;
     }
 
-    std::vector<std::string> GetGroupItems() const
+    std::vector<TOOLBAR_ITEM> GetGroupItems() const
     {
         return m_groupItems;
     }
@@ -118,8 +119,8 @@ public:
 public:
     // These are public to write the JSON, but are lower-cased to encourage people not to directly
     // access them and treat them as private.
-    wxString                 m_groupName;
-    std::vector<std::string> m_groupItems;
+    wxString                  m_groupName;
+    std::vector<TOOLBAR_ITEM> m_groupItems;
 };
 
 class KICOMMON_API TOOLBAR_CONFIGURATION
@@ -194,7 +195,7 @@ public:
 
 enum class TOOLBAR_LOC
 {
-    LEFT,           ///< Toolbar on the left side of the canvas
+    LEFT = 0,       ///< Toolbar on the left side of the canvas
     RIGHT,          ///< Toolbar on the right side of the canvas
     TOP_MAIN,       ///< Toolbar on the top of the canvas
     TOP_AUX         ///< Toolbar on the top of the canvas
@@ -220,18 +221,23 @@ public:
      *
      * Returns the user-configured tools, and if not customized, the default tools.
      */
-    std::optional<TOOLBAR_CONFIGURATION> GetToolbarConfig( TOOLBAR_LOC aToolbar, bool aForceDefault );
+    std::optional<TOOLBAR_CONFIGURATION> GetToolbarConfig( TOOLBAR_LOC aToolbar, bool aAllowCustom = true );
 
     /**
-     * Set a configuration for the toolbar.
+     * Get the stored configuration for the given toolbar.
      */
-    void SetToolbarConfig( TOOLBAR_LOC aToolbar, TOOLBAR_CONFIGURATION& aConfig )
+    std::optional<TOOLBAR_CONFIGURATION> GetStoredToolbarConfig( TOOLBAR_LOC aToolbar );
+
+    /**
+     * Set the stored configuration for the given toolbar.
+     */
+    void SetStoredToolbarConfig( TOOLBAR_LOC aToolbar, TOOLBAR_CONFIGURATION& aConfig )
     {
         m_toolbars[aToolbar] = aConfig;
     }
 
 protected:
-    // The toolbars - only public to aid in JSON serialization/deserialization
+    // The toolbars
     std::map<TOOLBAR_LOC, TOOLBAR_CONFIGURATION> m_toolbars;
 };