Update close button visibility based on BOTH settings and readonly mo…

…de (#15914)

`TerminalTab::_RecalculateAndApplyReadOnly` didn't know about whether a
tab should be closable or not, based on the theme settings. Similarly
(though, unreported), the theme update in
`TerminalPage::_updateAllTabCloseButtons` didn't really know about
readonly mode.

This fixes both these issues by moving responsibility for the tab close
button visibility into `TabBase` itself.

Closes #15902

src/cascadia/TerminalApp/TabBase.cpp

@@ -25,10 +25,6 @@ namespace winrt
 
 namespace winrt::TerminalApp::implementation
 {
-    WUX::FocusState TabBase::FocusState() const noexcept
-    {
-        return _focusState;
-    }
 
     // Method Description:
     // - Prepares this tab for being removed from the UI hierarchy
@@ -590,4 +586,63 @@ namespace winrt::TerminalApp::implementation
             VisualStateManager::GoToState(item, L"Normal", true);
         }
     }
+
+    TabCloseButtonVisibility TabBase::CloseButtonVisibility()
+    {
+        return _closeButtonVisibility;
+    }
+
+    // Method Description:
+    // - set our internal state to track if we were requested to have a visible
+    //   tab close button or not.
+    // - This is called every time the active tab changes. That way, the changes
+    //   in focused tab can be reflected for the "ActiveOnly" state.
+    void TabBase::CloseButtonVisibility(TabCloseButtonVisibility visibility)
+    {
+        _closeButtonVisibility = visibility;
+        _updateIsClosable();
+    }
+
+    // Method Description:
+    // - Update our close button's visibility, to reflect both the ReadOnly
+    //   state of the tab content, and also if if we were told to have a visible
+    //   close button at all.
+    //   - the tab being read-only takes precedence. That will always suppress
+    //     the close button.
+    //   - Otherwise we'll use the state set in CloseButtonVisibility to control
+    //     the tab's visibility.
+    void TabBase::_updateIsClosable()
+    {
+        bool isClosable = true;
+
+        if (ReadOnly())
+        {
+            isClosable = false;
+        }
+        else
+        {
+            switch (_closeButtonVisibility)
+            {
+            case TabCloseButtonVisibility::Never:
+                isClosable = false;
+                break;
+            case TabCloseButtonVisibility::Hover:
+                isClosable = true;
+                break;
+            case TabCloseButtonVisibility::ActiveOnly:
+                isClosable = _focused();
+                break;
+            default:
+                isClosable = true;
+                break;
+            }
+        }
+        TabViewItem().IsClosable(isClosable);
+    }
+
+    bool TabBase::_focused() const noexcept
+    {
+        return _focusState != FocusState::Unfocused;
+    }
+
 }

src/cascadia/TerminalApp/TabBase.h

@@ -16,7 +16,6 @@ namespace winrt::TerminalApp::implementation
     {
     public:
         virtual void Focus(winrt::Windows::UI::Xaml::FocusState focusState) = 0;
-        winrt::Windows::UI::Xaml::FocusState FocusState() const noexcept;
 
         virtual void Shutdown();
         void SetDispatch(const winrt::TerminalApp::ShortcutActionDispatch& dispatch);
@@ -30,6 +29,9 @@ namespace winrt::TerminalApp::implementation
                         const winrt::Microsoft::Terminal::Settings::Model::ThemeColor& unfocused,
                         const til::color& tabRowColor);
 
+        Microsoft::Terminal::Settings::Model::TabCloseButtonVisibility CloseButtonVisibility();
+        void CloseButtonVisibility(Microsoft::Terminal::Settings::Model::TabCloseButtonVisibility visible);
+
         WINRT_CALLBACK(RequestFocusActiveControl, winrt::delegate<void()>);
 
         WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
@@ -60,6 +62,8 @@ namespace winrt::TerminalApp::implementation
         winrt::Microsoft::Terminal::Settings::Model::ThemeColor _unfocusedThemeColor{ nullptr };
         til::color _tabRowColor;
 
+        Microsoft::Terminal::Settings::Model::TabCloseButtonVisibility _closeButtonVisibility{ Microsoft::Terminal::Settings::Model::TabCloseButtonVisibility::Always };
+
         virtual void _CreateContextMenu();
         virtual winrt::hstring _CreateToolTipTitle();
 
@@ -78,6 +82,9 @@ namespace winrt::TerminalApp::implementation
         void _RefreshVisualState();
         virtual winrt::Windows::UI::Xaml::Media::Brush _BackgroundBrush() = 0;
 
+        bool _focused() const noexcept;
+        void _updateIsClosable();
+
         friend class ::TerminalAppLocalTests::TabTests;
     };
 }

src/cascadia/TerminalApp/TabBase.idl

@@ -9,10 +9,10 @@ namespace TerminalApp
         String Title { get; };
         String Icon { get; };
         Boolean ReadOnly { get; };
+        Microsoft.Terminal.Settings.Model.TabCloseButtonVisibility CloseButtonVisibility { get; set; };
 
         Microsoft.UI.Xaml.Controls.TabViewItem TabViewItem { get; };
         Windows.UI.Xaml.FrameworkElement Content { get; };
-        Windows.UI.Xaml.FocusState FocusState { get; };
 
         UInt32 TabViewIndex;
         UInt32 TabViewNumTabs;

src/cascadia/TerminalApp/TabManagement.cpp

@@ -936,7 +936,7 @@ namespace winrt::TerminalApp::implementation
             {
                 tab.Focus(FocusState::Programmatic);
                 _UpdateMRUTab(tab);
-                _updateAllTabCloseButtons(tab);
+                _updateAllTabCloseButtons();
             }
 
             tab.TabViewItem().StartBringIntoView();
@@ -1114,7 +1114,7 @@ namespace winrt::TerminalApp::implementation
             {
                 tab.Focus(FocusState::Programmatic);
                 _UpdateMRUTab(tab);
-                _updateAllTabCloseButtons(tab);
+                _updateAllTabCloseButtons();
             }
         }
     }

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -3283,44 +3283,23 @@ namespace winrt::TerminalApp::implementation
         // Begin Theme handling
         _updateThemeColors();
 
-        _updateAllTabCloseButtons(_GetFocusedTab());
+        _updateAllTabCloseButtons();
     }
 
-    void TerminalPage::_updateAllTabCloseButtons(const winrt::TerminalApp::TabBase& focusedTab)
+    void TerminalPage::_updateAllTabCloseButtons()
     {
         // Update the state of the CloseButtonOverlayMode property of
         // our TabView, to match the tab.showCloseButton property in the theme.
         //
         // Also update every tab's individual IsClosable to match the same property.
         const auto theme = _settings.GlobalSettings().CurrentTheme();
-        const auto visibility = theme && theme.Tab() ? theme.Tab().ShowCloseButton() : Settings::Model::TabCloseButtonVisibility::Always;
+        const auto visibility = (theme && theme.Tab()) ?
+                                    theme.Tab().ShowCloseButton() :
+                                    Settings::Model::TabCloseButtonVisibility::Always;
 
         for (const auto& tab : _tabs)
         {
-            switch (visibility)
-            {
-            case Settings::Model::TabCloseButtonVisibility::Never:
-                tab.TabViewItem().IsClosable(false);
-                break;
-            case Settings::Model::TabCloseButtonVisibility::Hover:
-                tab.TabViewItem().IsClosable(true);
-                break;
-            case Settings::Model::TabCloseButtonVisibility::ActiveOnly:
-            {
-                if (focusedTab && focusedTab == tab)
-                {
-                    tab.TabViewItem().IsClosable(true);
-                }
-                else
-                {
-                    tab.TabViewItem().IsClosable(false);
-                }
-                break;
-            }
-            default:
-                tab.TabViewItem().IsClosable(true);
-                break;
-            }
+            tab.CloseButtonVisibility(visibility);
         }
 
         switch (visibility)

src/cascadia/TerminalApp/TerminalPage.h

@@ -511,7 +511,7 @@ namespace winrt::TerminalApp::implementation
         static void _DismissMessage(const winrt::Microsoft::Terminal::Settings::Model::InfoBarMessage& message);
 
         void _updateThemeColors();
-        void _updateAllTabCloseButtons(const winrt::TerminalApp::TabBase& focusedTab);
+        void _updateAllTabCloseButtons();
         void _updatePaneResources(const winrt::Windows::UI::Xaml::ElementTheme& requestedTheme);
 
         winrt::fire_and_forget _ControlCompletionsChangedHandler(const winrt::Windows::Foundation::IInspectable sender, const winrt::Microsoft::Terminal::Control::CompletionsChangedEventArgs args);

src/cascadia/TerminalApp/TerminalTab.cpp

@@ -223,7 +223,7 @@ namespace winrt::TerminalApp::implementation
 
         _focusState = focusState;
 
-        if (_focusState != FocusState::Unfocused)
+        if (_focused())
         {
             auto lastFocusedControl = GetActiveTerminalControl();
             if (lastFocusedControl)
@@ -961,7 +961,7 @@ namespace winrt::TerminalApp::implementation
             co_await wil::resume_foreground(dispatcher);
             if (const auto tab{ weakThis.get() })
             {
-                if (tab->_focusState != FocusState::Unfocused)
+                if (tab->_focused())
                 {
                     if (const auto termControl{ sender.try_as<winrt::Microsoft::Terminal::Control::TermControl>() })
                     {
@@ -1693,7 +1693,7 @@ namespace winrt::TerminalApp::implementation
         }
 
         ReadOnly(_rootPane->ContainsReadOnly());
-        TabViewItem().IsClosable(!ReadOnly());
+        _updateIsClosable();
     }
 
     std::shared_ptr<Pane> TerminalTab::GetActivePane() const