this is so bodgy

microsoft · Aug 11, 2021 · 0077482 · 0077482
1 parent ba5bd73
commit 0077482
Show file tree

Hide file tree

Showing 8 changed files with 21 additions and 35 deletions.
diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp
@@ -1037,11 +1037,6 @@ namespace winrt::TerminalApp::implementation
         return _root.as<winrt::Windows::UI::Xaml::Controls::Control>();
     }
 
-    void AppLogic::DismissSettingsPopups() noexcept
-    {
-        return _root->DismissSettingsPopups();
-    }
-
     // Method Description:
     // - Gets the title of the currently focused terminal control. If there
     //   isn't a control selected for any reason, returns "Windows Terminal"

diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h
@@ -87,7 +87,6 @@ namespace winrt::TerminalApp::implementation
         hstring Title();
         void TitlebarClicked();
         bool OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down);
-        void DismissSettingsPopups() noexcept;
 
         void WindowCloseButtonClicked();
 

diff --git a/src/cascadia/TerminalApp/AppLogic.idl b/src/cascadia/TerminalApp/AppLogic.idl
@@ -67,7 +67,6 @@ namespace TerminalApp
         Single CalcSnappedDimension(Boolean widthOrHeight, Single dimension);
         void TitlebarClicked();
         void WindowCloseButtonClicked();
-        void DismissSettingsPopups();
 
         TaskbarState TaskbarState{ get; };
 

diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -2913,19 +2913,4 @@ namespace winrt::TerminalApp::implementation
     {
         return WindowName() == QuakeWindowName;
     }
-
-    void TerminalPage::DismissSettingsPopups()
-    {
-        if (_settingsTab)
-        {
-            const auto xamlRoot{ _settingsTab.Content().XamlRoot() };
-            // This is basically DismissAllPopups
-            const auto popups{ Media::VisualTreeHelper::GetOpenPopupsForXamlRoot(xamlRoot) };
-            for (const auto& p : popups)
-            {
-                p.IsOpen(false);
-            }
-        }
-    }
-
 }
diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h
@@ -107,7 +107,6 @@ namespace winrt::TerminalApp::implementation
         winrt::hstring WindowIdForDisplay() const noexcept;
         winrt::hstring WindowNameForDisplay() const noexcept;
         bool IsQuakeWindow() const noexcept;
-        void DismissSettingsPopups();
 
         WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
 

diff --git a/src/cascadia/TerminalApp/TerminalPage.idl b/src/cascadia/TerminalApp/TerminalPage.idl
@@ -35,7 +35,6 @@ namespace TerminalApp
         String WindowIdForDisplay { get; };
         void RenameFailed();
         Boolean IsQuakeWindow();
-        void DismissSettingsPopups();
 
         // We cannot use the default XAML APIs because we want to make sure
         // that there's only one application-global dialog visible at a time,

diff --git a/src/cascadia/TerminalSettingsEditor/Utils.h b/src/cascadia/TerminalSettingsEditor/Utils.h
@@ -141,7 +141,7 @@ namespace winrt::Microsoft::Terminal::Settings
 
 // BODGY!
 //
-// The following are a workaround for GH#9320.
+// The following function and struct are a workaround for GH#9320.
 //
 // DismissAllPopups can be used to dismiss all popups for a particular UI
 // element. However, we've got a bunch of pages with scroll viewers that may or
@@ -169,7 +169,7 @@ struct HasScrollViewer
         // won't be able to figure it out.
         //
         // Fortunately for us, we don't need to! The sender is a UIElement, so
-        // we can just get _their_ XamlRoot(),
+        // we can just get _their_ XamlRoot().
         if (const auto& uielem{ sender.try_as<winrt::Windows::UI::Xaml::UIElement>() })
         {
             DismissAllPopups(uielem.XamlRoot());

diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp
@@ -1025,18 +1025,28 @@ void AppHost::_UpdateTrayIcon()
     }
 }
 
+// Method Description:
+// - BODGY workaround for GH#9320. When the window moves, dismiss all the popups
+//   in the UI tree. Xaml Islands unfortunately doesn't do this for us, see
+//   microsoft/microsoft-ui-xaml#4554
+// Arguments:
+// - <none>
+// Return Value:
+// - <none>
 void AppHost::_WindowMoved()
 {
     if (_logic)
     {
-        _logic.DismissSettingsPopups();
-        // const auto root{ _logic.GetRoot() };
-
-        // // This is basically DismissAllPopups
-        // const auto popups{ Media::VisualTreeHelper::GetOpenPopupsForXamlRoot(root.XamlRoot()) };
-        // for (const auto& p : popups)
-        // {
-        //     p.IsOpen(false);
-        // }
+        const auto root{ _logic.GetRoot() };
+
+        // This is basically DismissAllPopups which is also in
+        // TerminalSettingsEditor/Utils.h
+        // There isn't a good place that's shared between these two files, but
+        // it's only 5 LOC so whatever.
+        const auto popups{ Media::VisualTreeHelper::GetOpenPopupsForXamlRoot(root.XamlRoot()) };
+        for (const auto& p : popups)
+        {
+            p.IsOpen(false);
+        }
     }
 }