Addressing more feedback from AMZN-alexpeete

Signed-off-by: T.J. Kotha <[email protected]>

Code/Tools/ProjectManager/Source/CreateAGemScreen.cpp

@@ -39,14 +39,6 @@ namespace O3DE::ProjectManager
 
         m_header = new ScreenHeader();
         m_header->setSubTitle(tr("Create a new gem"));
-        connect(
-            m_header->backButton(),
-            &QPushButton::clicked,
-            this,
-            [&]()
-            {
-                emit GoToPreviousScreenRequest();
-            });
         screenLayout->addWidget(m_header);
         
         QHBoxLayout* hLayout = new QHBoxLayout();
@@ -77,9 +69,6 @@ namespace O3DE::ProjectManager
         m_backButton->setProperty("secondary", true);
         m_nextButton = m_backNextButtons->addButton(tr("Next"), QDialogButtonBox::ApplyRole);
 
-        connect(m_backButton, &QPushButton::clicked, this, &CreateGem::HandleBackButton);
-        connect(m_nextButton, &QPushButton::clicked, this, &CreateGem::HandleNextButton);
-
         setObjectName("createAGemBody");
         setLayout(screenLayout);
 
@@ -87,6 +76,21 @@ namespace O3DE::ProjectManager
         m_indexBackLimit = GemTemplateSelectionScreen;
     }
 
+    void CreateGem::HookConnections()
+    {
+        connect(
+            m_header->backButton(),
+            &QPushButton::clicked,
+            this,
+            [&]()
+            {
+                emit GoToPreviousScreenRequest();
+            });
+        
+        connect(m_backButton, &QPushButton::clicked, this, &CreateGem::HandleBackButton);
+        connect(m_nextButton, &QPushButton::clicked, this, &CreateGem::HandleNextButton);
+    }
+
     QFrame* CreateGem::CreateTabButtonsFrame()
     {
         QFrame* tabButtonsFrame = new QFrame();
@@ -354,6 +358,11 @@ namespace O3DE::ProjectManager
         return gemSystemNameIsValid;
     }
 
+    bool CreateGem::ValidateGemLocation(const QDir& chosenGemLocation) const
+    {
+        return !chosenGemLocation.exists() || chosenGemLocation.isEmpty();
+    }
+
     bool CreateGem::ValidateGemPath()
     {
         // This first isEmpty check is to check that the input field is not empty. If it is QDir will use the current
@@ -435,15 +444,15 @@ namespace O3DE::ProjectManager
         
         if (canProceedToDetailsPage)
         {
-            m_focalGemInfo.m_displayName = m_gemDisplayName->lineEdit()->text();
-            m_focalGemInfo.m_name = m_gemName->lineEdit()->text();
-            m_focalGemInfo.m_summary = m_gemSummary->lineEdit()->text();
-            m_focalGemInfo.m_requirement = m_requirements->lineEdit()->text();
-            m_focalGemInfo.m_licenseText = m_license->lineEdit()->text();
-            m_focalGemInfo.m_licenseLink = m_licenseURL->lineEdit()->text();
-            m_focalGemInfo.m_documentationLink = m_documentationURL->lineEdit()->text();
-            m_focalGemInfo.m_path = m_gemLocation->lineEdit()->text();
-            m_focalGemInfo.m_features = m_userDefinedGemTags->getTags();
+            m_gemInfo.m_displayName = m_gemDisplayName->lineEdit()->text();
+            m_gemInfo.m_name = m_gemName->lineEdit()->text();
+            m_gemInfo.m_summary = m_gemSummary->lineEdit()->text();
+            m_gemInfo.m_requirement = m_requirements->lineEdit()->text();
+            m_gemInfo.m_licenseText = m_license->lineEdit()->text();
+            m_gemInfo.m_licenseLink = m_licenseURL->lineEdit()->text();
+            m_gemInfo.m_documentationLink = m_documentationURL->lineEdit()->text();
+            m_gemInfo.m_path = m_gemLocation->lineEdit()->text();
+            m_gemInfo.m_features = m_userDefinedGemTags->getTags();
 
             m_stackWidget->setCurrentIndex(GemCreatorDetailsScreen);
             
@@ -466,11 +475,11 @@ namespace O3DE::ProjectManager
         }
         else
         {
-            int templateID = m_radioButtonGroup->checkedId();
+            const int templateID = m_radioButtonGroup->checkedId();
             templateLocation = m_gemTemplates[templateID].m_path;
         }
 
-        auto result = PythonBindingsInterface::Get()->CreateGem(templateLocation, m_focalGemInfo);
+        auto result = PythonBindingsInterface::Get()->CreateGem(templateLocation, m_gemInfo);
         if (result.IsSuccess())
         {
             ClearFields();
@@ -493,9 +502,9 @@ namespace O3DE::ProjectManager
         bool repoURLIsValid = ValidateRepositoryURL();
         if (originIsValid && repoURLIsValid)
         {
-            m_focalGemInfo.m_origin = m_origin->lineEdit()->text();
-            m_focalGemInfo.m_originURL = m_originURL->lineEdit()->text();
-            m_focalGemInfo.m_repoUri = m_repositoryURL->lineEdit()->text();
+            m_gemInfo.m_origin = m_origin->lineEdit()->text();
+            m_gemInfo.m_originURL = m_originURL->lineEdit()->text();
+            m_gemInfo.m_repoUri = m_repositoryURL->lineEdit()->text();
             
 
             GemAction();

Code/Tools/ProjectManager/Source/CreateAGemScreen.h

@@ -17,8 +17,6 @@
 #include <GemCatalog/GemInfo.h>
 #include <PythonBindings.h>
 #include <ScreenHeaderWidget.h>
-#include <ScreenDefs.h>
-#include <QDir>
 #endif
 
 QT_FORWARD_DECLARE_CLASS(QButtonGroup)
@@ -38,13 +36,13 @@ namespace O3DE::ProjectManager
         explicit CreateGem(QWidget* parent = nullptr);
         ~CreateGem() = default;
 
-        void ClearFields();
-
         ProjectManagerScreen GetScreenEnum() override
         {
             return ProjectManagerScreen::CreateGem;
         }
 
+        void HookConnections() override;
+
     signals:
         void GemCreated(const GemInfo& gemInfo);
 
@@ -59,24 +57,11 @@ namespace O3DE::ProjectManager
         void HandleGemCreatorDetailsTab();
 
     protected:
-        bool ValidateGemTemplateLocation();
-        bool ValidateGemDisplayName();
-        bool ValidateGemName();
-        bool ValidateGemPath();
-        bool ValidateFormNotEmpty(FormLineEditWidget* form);
-        bool ValidateRepositoryURL();
-        
-        void ProceedToGemDetailsPage();
-        void ProceedToGemCreatorDetailsPage();
-        void ProceedToGemAction();
+        void ClearFields();
 
         virtual void GemAction();
 
-        virtual bool ValidateGemLocation(QDir chosenGemLocation)
-        {
-            return !chosenGemLocation.exists() || chosenGemLocation.isEmpty();
-        }
-
+        virtual bool ValidateGemLocation(const QDir& chosenGemLocation) const;
 
         //Gem Setup
         QVector<TemplateInfo> m_gemTemplates;
@@ -114,7 +99,7 @@ namespace O3DE::ProjectManager
         QRadioButton* m_gemDetailsTab = nullptr;
         QRadioButton* m_gemCreatorDetailsTab = nullptr;
 
-        GemInfo m_focalGemInfo;
+        GemInfo m_gemInfo;
 
         static constexpr int GemTemplateSelectionScreen = 0;
         static constexpr int GemDetailsScreen = 1;
@@ -132,6 +117,16 @@ namespace O3DE::ProjectManager
         QFrame* CreateTabButtonsFrame();
         QFrame* CreateTabPaneFrame();
         void SetupCreateWorkflow();
-        
+
+        void ProceedToGemDetailsPage();
+        void ProceedToGemCreatorDetailsPage();
+        void ProceedToGemAction();
+
+        bool ValidateGemTemplateLocation();
+        bool ValidateGemDisplayName();
+        bool ValidateGemName();
+        bool ValidateGemPath();
+        bool ValidateFormNotEmpty(FormLineEditWidget* form);
+        bool ValidateRepositoryURL();
     };
 } // namespace O3DE::ProjectManager

Code/Tools/ProjectManager/Source/EditAGemScreen.cpp

@@ -7,6 +7,7 @@
  */
 
 #include <EditAGemScreen.h>
+#include <QDir>
 
 namespace O3DE::ProjectManager
 {
@@ -14,29 +15,8 @@ namespace O3DE::ProjectManager
     EditGem::EditGem(QWidget* parent)
         : CreateGem(parent)
     {
-        //undo the connections of the parent class
-        m_header->backButton()->disconnect();
-        m_backButton->disconnect();
-        m_nextButton->disconnect();
-
         m_header->setSubTitle(tr("Edit gem"));
-        
-        connect(
-            m_header->backButton(),
-            &QPushButton::clicked,
-            this,
-            [&]()
-            {
-                //if previously editing a gem, cancel the operation and revert to old creation workflow
-                //this way we prevent accidental stale data for an already existing gem.
-                ClearFields();
-                emit GoToPreviousScreenRequest();
-            });
-        
-        connect(m_backButton, &QPushButton::clicked, this, &EditGem::HandleBackButton);
-        connect(m_nextButton, &QPushButton::clicked, this, &EditGem::HandleNextButton);
-
-        
+           
         //we will only have two pages: details page and creator details page
         m_gemTemplateSelectionTab->setChecked(false);
         m_gemTemplateSelectionTab->setVisible(false);
@@ -59,6 +39,31 @@ namespace O3DE::ProjectManager
         m_indexBackLimit = GemDetailsScreen;
     }
 
+    void EditGem::HookConnections()
+    {
+        connect(
+            m_header->backButton(),
+            &QPushButton::clicked,
+            this,
+            [&]()
+            {
+                // if previously editing a gem, cancel the operation and revert to old creation workflow
+                // this way we prevent accidental stale data for an already existing gem.
+                ClearFields();
+                emit GoToPreviousScreenRequest();
+            });
+
+        connect(m_backButton, &QPushButton::clicked, this, &EditGem::HandleBackButton);
+        connect(m_nextButton, &QPushButton::clicked, this, &EditGem::HandleNextButton);
+
+     
+    }
+
+    bool EditGem::ValidateGemLocation(const QDir& chosenGemLocation) const
+    {
+        return chosenGemLocation.exists();
+    }
+
     void EditGem::ResetWorkflow(const GemInfo& oldGemInfo)
     {
         //details page
@@ -95,7 +100,7 @@ namespace O3DE::ProjectManager
         m_originURL->lineEdit()->setText(oldGemInfo.m_originURL);
         m_repositoryURL->lineEdit()->setText(oldGemInfo.m_repoUri);
 
-        m_oldGemInfo = oldGemInfo;
+        m_oldGemName = oldGemInfo.m_name;
     }
 
 
@@ -104,12 +109,12 @@ namespace O3DE::ProjectManager
     {
         //during editing, we remove the gem name tag to prevent the user accidentally altering it
         //so add it back here before submission
-        if(!m_focalGemInfo.m_features.contains(m_focalGemInfo.m_name))
+        if(!m_gemInfo.m_features.contains(m_gemInfo.m_name))
         {
-            m_focalGemInfo.m_features << m_focalGemInfo.m_name;
+            m_gemInfo.m_features << m_gemInfo.m_name;
         }
 
-        auto result = PythonBindingsInterface::Get()->EditGem(m_oldGemInfo.m_name, m_focalGemInfo);
+        auto result = PythonBindingsInterface::Get()->EditGem(m_oldGemName, m_gemInfo);
         if(result.IsSuccess())
         {
             ClearFields();

Code/Tools/ProjectManager/Source/EditAGemScreen.h

@@ -10,6 +10,8 @@
 
 #if !defined(Q_MOC_RUN)
 #include <CreateAGemScreen.h>
+
+QT_FORWARD_DECLARE_CLASS(QDir)
 #endif
 
 namespace O3DE::ProjectManager
@@ -28,20 +30,17 @@ namespace O3DE::ProjectManager
             return ProjectManagerScreen::EditGem;
         }
 
+        void HookConnections() override;
+
     signals:
         void GemEdited(const GemInfo& newGemInfo);
         
     private: 
         void GemAction() override;
 
-        bool ValidateGemLocation(QDir chosenGemLocation) override
-        {
-            return chosenGemLocation.exists();
-        }
+        bool ValidateGemLocation(const QDir& chosenGemLocation) const override;
 
-        //Edit Gem workflow
-        bool m_isEditGem = false;
-        GemInfo m_oldGemInfo;
+        QString m_oldGemName;
 
     };
 

Code/Tools/ProjectManager/Source/GemCatalog/GemCatalogScreen.cpp

@@ -654,16 +654,14 @@ namespace O3DE::ProjectManager
     {
         if (m_screensControl)
         {
-            ScreenWidget* editGemScreen = m_screensControl->FindScreen(ProjectManagerScreen::EditGem);
+            auto editGemScreen = qobject_cast<EditGem*>(m_screensControl->FindScreen(ProjectManagerScreen::EditGem));
             if (editGemScreen)
             {
-                auto editGem = qobject_cast<EditGem*>(editGemScreen);
                 m_curEditedIndex = currentModelIndex;
-                editGem->ResetWorkflow(m_gemModel->GetGemInfo(currentModelIndex));
+                editGemScreen->ResetWorkflow(m_gemModel->GetGemInfo(currentModelIndex));
                 emit ChangeScreenRequest(ProjectManagerScreen::EditGem);
             }
         }
-        
     }
 
     void GemCatalogScreen::UpdateAndShowGemCart(QWidget* cartWidget)

Code/Tools/ProjectManager/Source/ScreenFactory.cpp

@@ -74,6 +74,9 @@ namespace O3DE::ProjectManager
             newScreen = new ScreenWidget(parent);
         }
 
+        //handle any code that needs to run after construction but before startup 
+        newScreen->HookConnections();
+
         return newScreen;
     }
 } // namespace O3DE::ProjectManager

Code/Tools/ProjectManager/Source/ScreenWidget.h

@@ -54,7 +54,9 @@ namespace O3DE::ProjectManager
         virtual void GoToScreen([[maybe_unused]] ProjectManagerScreen screen)
         {
         }
-
+        virtual void HookConnections()
+        {
+        }
         //! Notify this screen it is the current screen 
         virtual void NotifyCurrentScreen()
         {