Improved look for viewport ui border close button (#5757)

* improved look for viewport ui border close button

Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com>

* remove unused variable

Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com>

* removed commented 'fixme' calls

Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com>

* updates following PR feedback

Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com>

Code/Editor/ActionManager.cpp

@@ -152,15 +152,6 @@ ActionManager::ActionWrapper& ActionManager::ActionWrapper::SetMenu(DynamicMenu*
     return *this;
 }
 
-ActionManager::ActionWrapper& ActionManager::ActionWrapper::SetApplyHoverEffect()
-{
-    // Our standard toolbar icons, when hovered on, get a white color effect.
-    // But for this to work we need .pngs that look good with this effect, so this only works with the standard toolbars
-    // and looks very ugly for other toolbars, including toolbars loaded from XML (which just show a white rectangle)
-    m_action->setProperty("IconHasHoverEffect", true);
-    return *this;
-}
-
 ActionManager::ActionWrapper& ActionManager::ActionWrapper::SetReserved()
 {
     m_action->setProperty("Reserved", true);

Code/Editor/ActionManager.h

@@ -151,7 +151,6 @@ public:
         }
 
         ActionWrapper& SetMenu(DynamicMenu* menu);
-        ActionWrapper& SetApplyHoverEffect();
 
         operator QAction*() const {
             return m_action;

Code/Editor/MainWindow.cpp

@@ -708,14 +708,10 @@ void MainWindow::InitActions()
         .SetShortcut(QKeySequence::Undo)
         .SetReserved()
         .SetStatusTip(tr("Undo last operation"))
-        //.SetMenu(new QMenu("FIXME"))
-        .SetApplyHoverEffect()
         .RegisterUpdateCallback(cryEdit, &CCryEditApp::OnUpdateUndo);
     am->AddAction(ID_REDO, tr("&Redo"))
         .SetShortcut(AzQtComponents::RedoKeySequence)
         .SetReserved()
-        //.SetMenu(new QMenu("FIXME"))
-        .SetApplyHoverEffect()
         .SetStatusTip(tr("Redo last undo operation"))
         .RegisterUpdateCallback(cryEdit, &CCryEditApp::OnUpdateRedo);
 
@@ -731,7 +727,6 @@ void MainWindow::InitActions()
     // Modify actions
     am->AddAction(AzToolsFramework::EditModeMove, tr("Move"))
         .SetIcon(Style::icon("Move"))
-        .SetApplyHoverEffect()
         .SetShortcut(tr("1"))
         .SetToolTip(tr("Move (1)"))
         .SetCheckable(true)
@@ -757,7 +752,6 @@ void MainWindow::InitActions()
             });                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                               
     am->AddAction(AzToolsFramework::EditModeRotate, tr("Rotate"))
         .SetIcon(Style::icon("Translate"))
-        .SetApplyHoverEffect()
         .SetShortcut(tr("2"))
         .SetToolTip(tr("Rotate (2)"))
         .SetCheckable(true)
@@ -783,7 +777,6 @@ void MainWindow::InitActions()
             });
     am->AddAction(AzToolsFramework::EditModeScale, tr("Scale"))
         .SetIcon(Style::icon("Scale"))
-        .SetApplyHoverEffect()
         .SetShortcut(tr("3"))
         .SetToolTip(tr("Scale (3)"))
         .SetCheckable(true)
@@ -808,7 +801,6 @@ void MainWindow::InitActions()
 
     am->AddAction(AzToolsFramework::SnapToGrid, tr("Snap to grid"))
         .SetIcon(Style::icon("Grid"))
-        .SetApplyHoverEffect()
         .SetShortcut(tr("G"))
         .SetToolTip(tr("Snap to grid (G)"))
         .SetStatusTip(tr("Toggles snap to grid"))
@@ -821,7 +813,6 @@ void MainWindow::InitActions()
 
     am->AddAction(AzToolsFramework::SnapAngle, tr("Snap angle"))
         .SetIcon(Style::icon("Angle"))
-        .SetApplyHoverEffect()
         .SetStatusTip(tr("Snap angle"))
         .SetCheckable(true)
         .RegisterUpdateCallback([](QAction* action) {
@@ -961,7 +952,6 @@ void MainWindow::InitActions()
         .SetShortcut(tr("Ctrl+P"))
         .SetToolTip(tr("Simulate (Ctrl+P)"))
         .SetStatusTip(tr("Enable processing of Physics and AI."))
-        .SetApplyHoverEffect()
         .SetCheckable(true)
         .RegisterUpdateCallback(cryEdit, &CCryEditApp::OnSwitchPhysicsUpdate);
     am->AddAction(ID_GAME_SYNCPLAYER, tr("Move Player and Camera Separately")).SetCheckable(true)
@@ -1051,8 +1041,7 @@ void MainWindow::InitActions()
 
     // Editors Toolbar actions
     am->AddAction(ID_OPEN_ASSET_BROWSER, tr("Asset browser"))
-        .SetToolTip(tr("Open Asset Browser"))
+        .SetToolTip(tr("Open Asset Browser"));
-        .SetApplyHoverEffect();
 
     AZ::EBusReduceResult<bool, AZStd::logical_or<bool>> emfxEnabled(false);
     using AnimationRequestBus = AzToolsFramework::EditorAnimationSystemRequestsBus;
@@ -1062,8 +1051,7 @@ void MainWindow::InitActions()
     {
         QAction* action = am->AddAction(ID_OPEN_EMOTIONFX_EDITOR, tr("Animation Editor"))
             .SetToolTip(tr("Open Animation Editor (PREVIEW)"))
-            .SetIcon(QIcon(":/EMotionFX/EMFX_icon_32x32.png"))
+            .SetIcon(QIcon(":/EMotionFX/EMFX_icon_32x32.png"));
-            .SetApplyHoverEffect();
         QObject::connect(action, &QAction::triggered, this, []() {
             QtViewPaneManager::instance()->OpenPane(LyViewPane::AnimationEditor);
         });
@@ -1071,12 +1059,10 @@ void MainWindow::InitActions()
 
     am->AddAction(ID_OPEN_AUDIO_CONTROLS_BROWSER, tr("Audio Controls Editor"))
         .SetToolTip(tr("Open Audio Controls Editor"))
-        .SetIcon(Style::icon("Audio"))
+        .SetIcon(Style::icon("Audio"));
-        .SetApplyHoverEffect();
 
     am->AddAction(ID_OPEN_UICANVASEDITOR, tr(LyViewPane::UiEditor))
-        .SetToolTip(tr("Open UI Editor"))
+        .SetToolTip(tr("Open UI Editor"));
-        .SetApplyHoverEffect();
 
     // Edit Mode Toolbar Actions
     am->AddAction(IDC_SELECTION_MASK, tr("Selected Object Types"));
@@ -1089,12 +1075,10 @@ void MainWindow::InitActions()
     // Object Toolbar Actions
     am->AddAction(ID_GOTO_SELECTED, tr("Go to selected object"))
         .SetIcon(Style::icon("select_object"))
-        .SetApplyHoverEffect()
         .Connect(&QAction::triggered, this, &MainWindow::OnGotoSelected);
 
     // Misc Toolbar Actions
-    am->AddAction(ID_OPEN_SUBSTANCE_EDITOR, tr("Open Substance Editor"))
+    am->AddAction(ID_OPEN_SUBSTANCE_EDITOR, tr("Open Substance Editor"));
-        .SetApplyHoverEffect();
 }
 
 void MainWindow::InitToolActionHandlers()

Code/Framework/AzToolsFramework/AzToolsFramework/ViewportUi/ViewportUiCluster.cpp

@@ -62,9 +62,6 @@ namespace AzToolsFramework::ViewportUi::Internal
             return;
         }
 
-        // set hover to true by default
-        action->setProperty("IconHasHoverEffect", true);
-
         // add the action
         addAction(action);
 

Code/Framework/AzToolsFramework/AzToolsFramework/ViewportUi/ViewportUiDisplay.cpp

@@ -20,10 +20,9 @@
 namespace AzToolsFramework::ViewportUi::Internal
 {
     const static int HighlightBorderSize = 5;
-    const static char* HighlightBorderColor = "#4A90E2";
+    const static char* const HighlightBorderColor = "#4A90E2";
-    const static int HighlightBorderBackButtonMargin = 5;
     const static int HighlightBorderBackButtonIconSize = 20;
-    const static char* HighlightBorderBackButtonIconFile = "X_axis.svg";
+    const static char* const HighlightBorderBackButtonIconFile = "X_axis.svg";
 
     static void UnparentWidgets(ViewportUiElementIdInfoLookup& viewportUiElementIdInfoLookup)
     {
@@ -258,7 +257,7 @@ namespace AzToolsFramework::ViewportUi::Internal
         auto viewportUiMapElement = m_viewportUiElements.find(elementId);
         if (viewportUiMapElement != m_viewportUiElements.end())
         {
-            viewportUiMapElement->second.m_widget->setVisible(false);
+            viewportUiMapElement->second.m_widget->hide();
             viewportUiMapElement->second.m_widget->setParent(nullptr);
             m_viewportUiElements.erase(viewportUiMapElement);
         }
@@ -273,7 +272,7 @@ namespace AzToolsFramework::ViewportUi::Internal
     {
         if (ViewportUiElementInfo element = GetViewportUiElementInfo(elementId); element.m_widget)
         {
-            element.m_widget->setVisible(true);
+            element.m_widget->show();
         }
     }
 
@@ -281,7 +280,7 @@ namespace AzToolsFramework::ViewportUi::Internal
     {
         if (ViewportUiElementInfo element = GetViewportUiElementInfo(elementId); element.m_widget)
         {
-            element.m_widget->setVisible(false);
+            element.m_widget->hide();
         }
     }
 
@@ -298,14 +297,14 @@ namespace AzToolsFramework::ViewportUi::Internal
     void ViewportUiDisplay::CreateViewportBorder(
         const AZStd::string& borderTitle, AZStd::optional<ViewportUiBackButtonCallback> backButtonCallback)
     {
-        const AZStd::string styleSheet = AZStd::string::format(
+        m_uiOverlay.setStyleSheet(QString("border: %1px solid %2; border-top: %3px solid %4;")
-            "border: %dpx solid %s; border-top: %dpx solid %s;", HighlightBorderSize, HighlightBorderColor, ViewportUiTopBorderSize,
+                                      .arg(
-            HighlightBorderColor);
+                                          QString::number(HighlightBorderSize), HighlightBorderColor,
-        m_uiOverlay.setStyleSheet(styleSheet.c_str());
+                                          QString::number(ViewportUiTopBorderSize), HighlightBorderColor));
         m_uiOverlayLayout.setContentsMargins(
             HighlightBorderSize + ViewportUiOverlayMargin, ViewportUiTopBorderSize + ViewportUiOverlayMargin,
             HighlightBorderSize + ViewportUiOverlayMargin, HighlightBorderSize + ViewportUiOverlayMargin);
-        m_viewportBorderText.setVisible(true);
+        m_viewportBorderText.show();
         m_viewportBorderText.setText(borderTitle.c_str());
 
         // only display the back button if a callback was provided
@@ -315,13 +314,13 @@ namespace AzToolsFramework::ViewportUi::Internal
 
     void ViewportUiDisplay::RemoveViewportBorder()
     {
-        m_viewportBorderText.setVisible(false);
+        m_viewportBorderText.hide();
         m_uiOverlay.setStyleSheet("border: none;");
         m_uiOverlayLayout.setContentsMargins(
             ViewportUiOverlayMargin, ViewportUiOverlayMargin + ViewportUiOverlayTopMarginPadding, ViewportUiOverlayMargin,
             ViewportUiOverlayMargin);
         m_viewportBorderBackButtonCallback.reset();
-        m_viewportBorderBackButton.setVisible(false);
+        m_viewportBorderBackButton.hide();
     }
 
     void ViewportUiDisplay::PositionViewportUiElementFromWorldSpace(ViewportUiElementId elementId, const AZ::Vector3& pos)
@@ -358,41 +357,36 @@ namespace AzToolsFramework::ViewportUi::Internal
 
     void ViewportUiDisplay::InitializeUiOverlay()
     {
-        AZStd::string styleSheet;
-
         m_uiMainWindow.setObjectName(QString("ViewportUiWindow"));
         ConfigureWindowForViewportUi(&m_uiMainWindow);
-        m_uiMainWindow.setVisible(false);
+        m_uiMainWindow.hide();
 
         m_uiOverlay.setObjectName(QString("ViewportUiOverlay"));
         m_uiMainWindow.setCentralWidget(&m_uiOverlay);
-        m_uiOverlay.setVisible(false);
+        m_uiOverlay.hide();
 
         // remove any spacing and margins from the UI Overlay Layout
         m_fullScreenLayout.setSpacing(0);
         m_fullScreenLayout.setContentsMargins(0, 0, 0, 0);
         m_fullScreenLayout.addLayout(&m_uiOverlayLayout, 0, 0, 1, 1);
 
-        // format the label which will appear on top of the highlight border
+        // style the label which will appear on top of the highlight border
-        styleSheet = AZStd::string::format("background-color: %s; border: none;", HighlightBorderColor);
+        m_viewportBorderText.setStyleSheet(QString("background-color: %1; border: none").arg(HighlightBorderColor));
-        m_viewportBorderText.setStyleSheet(styleSheet.c_str());
         m_viewportBorderText.setFixedHeight(ViewportUiTopBorderSize);
-        m_viewportBorderText.setVisible(false);
+        m_viewportBorderText.hide();
         m_fullScreenLayout.addWidget(&m_viewportBorderText, 0, 0, Qt::AlignTop | Qt::AlignHCenter);
 
-        // format the back button which will appear in the top right of the highlight border
+        m_viewportBorderBackButton.setAutoRaise(true); // hover highlight
-        styleSheet = AZStd::string::format(
+        m_viewportBorderBackButton.hide();
-            "border: 0px; padding-left: %dpx; padding-right: %dpx", HighlightBorderBackButtonMargin, HighlightBorderBackButtonMargin);
+
-        m_viewportBorderBackButton.setStyleSheet(styleSheet.c_str());
+        QIcon backButtonIcon(QString(":/stylesheet/img/UI20/toolbar/%1").arg(HighlightBorderBackButtonIconFile));
-        m_viewportBorderBackButton.setVisible(false);
-        QIcon backButtonIcon(QString(AZStd::string::format(":/stylesheet/img/UI20/toolbar/%s", HighlightBorderBackButtonIconFile).c_str()));
         m_viewportBorderBackButton.setIcon(backButtonIcon);
         m_viewportBorderBackButton.setIconSize(QSize(HighlightBorderBackButtonIconSize, HighlightBorderBackButtonIconSize));
 
         // setup the handler for the back button to call the user provided callback (if any)
         QObject::connect(
-            &m_viewportBorderBackButton, &QPushButton::clicked,
+            &m_viewportBorderBackButton, &QToolButton::clicked,
-            [this]()
+            [this]
             {
                 if (m_viewportBorderBackButtonCallback.has_value())
                 {
@@ -452,16 +446,9 @@ namespace AzToolsFramework::ViewportUi::Internal
         region += m_uiOverlay.childrenRegion();
 
         // set viewport ui visibility depending on if elements are present
-        if (region.isEmpty() || !UiDisplayEnabled())
+        const bool visible = !region.isEmpty() && UiDisplayEnabled();
-        {
+        m_uiMainWindow.setVisible(visible);
-            m_uiMainWindow.setVisible(false);
+        m_uiOverlay.setVisible(visible);
-            m_uiOverlay.setVisible(false);
-        }
-        else
-        {
-            m_uiMainWindow.setVisible(true);
-            m_uiOverlay.setVisible(true);
-        }
 
         m_uiMainWindow.setMask(region);
     }

Code/Framework/AzToolsFramework/AzToolsFramework/ViewportUi/ViewportUiDisplay.h

@@ -17,7 +17,7 @@
 #include <QLabel>
 #include <QMainWindow>
 #include <QPointer>
-#include <QPushButton>
+#include <QToolButton>
 
 AZ_PUSH_DISABLE_WARNING(4251, "-Wunknown-warning-option")
 #include <QGridLayout>
@@ -115,9 +115,9 @@ namespace AzToolsFramework::ViewportUi::Internal
         QGridLayout m_fullScreenLayout; //!< The layout which extends across the full screen.
         ViewportUiDisplayLayout m_uiOverlayLayout; //!< The layout used for optionally anchoring Viewport UI Elements.
         QLabel m_viewportBorderText; //!< The text used for the viewport highlight border.
-        QPushButton m_viewportBorderBackButton; //!< The button to return from the viewport highlight border (only displayed if callback provided).
+        QToolButton m_viewportBorderBackButton; //!< The button to return from the viewport highlight border (only displayed if callback provided).
-        AZStd::optional<ViewportUiBackButtonCallback>
+        //! The optional callback for when the viewport highlight border back button is pressed.
-            m_viewportBorderBackButtonCallback; //!< The optional callback for when the viewport highlight border back button is pressed.
+        AZStd::optional<ViewportUiBackButtonCallback> m_viewportBorderBackButtonCallback;
 
         QWidget* m_renderOverlay;
         QPointer<QWidget> m_fullScreenWidget; //!< Reference to the widget attached to m_fullScreenLayout if any.

Code/Framework/AzToolsFramework/AzToolsFramework/ViewportUi/ViewportUiSwitcher.cpp

@@ -22,8 +22,6 @@ namespace AzToolsFramework::ViewportUi::Internal
 
         // Add am empty active button (is set in the call to SetActiveMode)
         m_activeButton = new QToolButton();
-        // No hover effect for the main button as it's not clickable
-        m_activeButton->setProperty("IconHasHoverEffect", false);
         m_activeButton->setCheckable(false);
         m_activeButton->setToolButtonStyle(Qt::ToolButtonTextBesideIcon);
         addWidget(m_activeButton);
@@ -56,9 +54,6 @@ namespace AzToolsFramework::ViewportUi::Internal
             return;
         }
 
-        // set hover to true by default
-        action->setProperty("IconHasHoverEffect", true);
-
         // add the action
         addAction(action);