Manually put our ContentDialogs in the tab content, rather than above (…

…#12840)

After switching to ControlsV2, it seems that
delay-loading a dialog causes the ContentDialog to be assigned a
Height equal to it's content size. If we DON'T assign the
ContentDialog a Row, I believe it's assigned Row 0 by default. So,
when the dialog gets opened, the dialog seemingly causes a giant
hole to appear in the body of the app.

Assigning all the dialogs to Row 2 (where the rest of the content
is) makes the "hole" appear in the same space as the rest of the
TabContent, fixing the issue.

Note that the actual content in a content dialog gets parented to
the PopupRoot, so it actually always appeared in the correct place, it's
just this weird hole that appeared in Row 0.

* [x] Closes #12775
  * See also:
    * #12202 was fixed by #12208
    * #12447 was fixed by #12517
* [x] Tested manually
* [x] Reverts #12625
* [x] Reverts #12517

(cherry picked from commit c4e5ebf)
Service-Card-Id: 80266902
Service-Version: 1.12

src/cascadia/TerminalApp/AppLogic.cpp

@@ -376,11 +376,7 @@ namespace winrt::TerminalApp::implementation
         }
 
         _dialog = dialog;
-        // GH#12622: After the dialog is displayed, always clear it out. If we
-        // don't, we won't be able to display another!
-        const auto cleanup = wil::scope_exit([this]() {
-            _dialog = nullptr;
-        });
+
         // IMPORTANT: This is necessary as documented in the ContentDialog MSDN docs.
         // Since we're hosting the dialog in a Xaml island, we need to connect it to the
         // xaml tree somehow.
@@ -428,14 +424,6 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
-    // Method Description:
-    // - Returns true if there is no dialog currently being shown (meaning that we can show a dialog)
-    // - Returns false if there is a dialog currently being shown (meaning that we cannot show another dialog)
-    bool AppLogic::CanShowDialog()
-    {
-        return (_dialog == nullptr);
-    }
-
     // Method Description:
     // - Displays a dialog for errors found while loading or validating the
     //   settings. Uses the resources under the provided  title and content keys

src/cascadia/TerminalApp/AppLogic.h

@@ -107,7 +107,6 @@ namespace winrt::TerminalApp::implementation
         bool GetShowTitleInTitlebar();
 
         winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::UI::Xaml::Controls::ContentDialogResult> ShowDialog(winrt::Windows::UI::Xaml::Controls::ContentDialog dialog);
-        bool CanShowDialog();
         void DismissDialog();
 
         Windows::Foundation::Collections::IMapView<Microsoft::Terminal::Control::KeyChord, Microsoft::Terminal::Settings::Model::Command> GlobalHotkeys();

src/cascadia/TerminalApp/AppLogic.idl

@@ -91,6 +91,11 @@ namespace TerminalApp
 
         Windows.Foundation.Collections.IMapView<Microsoft.Terminal.Control.KeyChord, Microsoft.Terminal.Settings.Model.Command> GlobalHotkeys();
 
+        // See IDialogPresenter and TerminalPage's DialogPresenter for more
+        // information.
+        Windows.Foundation.IAsyncOperation<Windows.UI.Xaml.Controls.ContentDialogResult> ShowDialog(Windows.UI.Xaml.Controls.ContentDialog dialog);
+        void DismissDialog();
+
         event Windows.Foundation.TypedEventHandler<Object, Windows.UI.Xaml.UIElement> SetTitleBarContent;
         event Windows.Foundation.TypedEventHandler<Object, String> TitleChanged;
         event Windows.Foundation.TypedEventHandler<Object, LastTabClosedEventArgs> LastTabClosed;

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -581,29 +581,14 @@ namespace winrt::TerminalApp::implementation
         ShellExecute(nullptr, nullptr, currentPath.c_str(), nullptr, nullptr, SW_SHOW);
     }
 
-    // Method description:
-    // - Called when the user closes a content dialog
-    // - Tells the presenter to update its knowledge of whether there is a content dialog open
-    void TerminalPage::_DialogCloseClick(const IInspectable&,
-                                         const ContentDialogButtonClickEventArgs&)
-    {
-        if (auto presenter{ _dialogPresenter.get() })
-        {
-            presenter.DismissDialog();
-        }
-    }
-
     // Method Description:
     // - Helper to show a content dialog
     // - We only open a content dialog if there isn't one open already
     winrt::Windows::Foundation::IAsyncOperation<ContentDialogResult> TerminalPage::_ShowDialogHelper(const std::wstring_view& name)
     {
         if (auto presenter{ _dialogPresenter.get() })
         {
-            if (presenter.CanShowDialog())
-            {
-                co_return co_await presenter.ShowDialog(FindName(name).try_as<WUX::Controls::ContentDialog>());
-            }
+            co_return co_await presenter.ShowDialog(FindName(name).try_as<WUX::Controls::ContentDialog>());
         }
         co_return ContentDialogResult::None;
     }

src/cascadia/TerminalApp/TerminalPage.h

@@ -216,7 +216,6 @@ namespace winrt::TerminalApp::implementation
         winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::UI::Xaml::Controls::ContentDialogResult> _ShowCloseReadOnlyDialog();
         winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::UI::Xaml::Controls::ContentDialogResult> _ShowMultiLinePasteWarningDialog();
         winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::UI::Xaml::Controls::ContentDialogResult> _ShowLargePasteWarningDialog();
-        void _DialogCloseClick(const IInspectable& sender, const Windows::UI::Xaml::Controls::ContentDialogButtonClickEventArgs& eventArgs);
 
         void _CreateNewTabFlyout();
         void _OpenNewTabDropdown();

src/cascadia/TerminalApp/TerminalPage.idl

@@ -14,8 +14,6 @@ namespace TerminalApp
     interface IDialogPresenter
     {
         Windows.Foundation.IAsyncOperation<Windows.UI.Xaml.Controls.ContentDialogResult> ShowDialog(Windows.UI.Xaml.Controls.ContentDialog dialog);
-        Boolean CanShowDialog();
-        void DismissDialog();
     };
 
     [default_interface] runtimeclass TerminalPage : Windows.UI.Xaml.Controls.Page, Windows.UI.Xaml.Data.INotifyPropertyChanged

src/cascadia/TerminalApp/TerminalPage.xaml

@@ -72,10 +72,27 @@
               HorizontalAlignment="Stretch"
               VerticalAlignment="Stretch" />
 
+        <!--
+            GH#12775 et. al: After switching to ControlsV2, it seems that
+            delay-loading a dialog causes the ContentDialog to be assigned a
+            Height equal to it's content size. If we DON'T assign the
+            ContentDialog a Row, I believe it's assigned Row 0 by default. So,
+            when the dialog gets opened, the dialog seemingly causes a giant
+            hole to appear in the body of the app.
+            
+            Assigning all the dialogs to Row 2 (where the rest of the content
+            is) makes the "hole" appear in the same space as the rest of the
+            TabContent, fixing the issue.
+            
+            Note that the actual content in a content dialog gets parented to
+            the PopupRoot, so it actually always appeared in the correct place, it's
+            just this weird hole that appeared in Row 0.
+        -->
+
         <ContentDialog x:Name="AboutDialog"
                        x:Uid="AboutDialog"
+                       Grid.Row="2"
                        x:Load="False"
-                       CloseButtonClick="_DialogCloseClick"
                        DefaultButton="Close">
             <StackPanel Orientation="Vertical">
                 <TextBlock IsTextSelectionEnabled="True">
@@ -96,26 +113,26 @@
 
         <ContentDialog x:Name="QuitDialog"
                        x:Uid="QuitDialog"
+                       Grid.Row="2"
                        x:Load="False"
-                       CloseButtonClick="_DialogCloseClick"
                        DefaultButton="Primary" />
 
         <ContentDialog x:Name="CloseAllDialog"
                        x:Uid="CloseAllDialog"
+                       Grid.Row="2"
                        x:Load="False"
-                       CloseButtonClick="_DialogCloseClick"
                        DefaultButton="Primary" />
 
         <ContentDialog x:Name="CloseReadOnlyDialog"
                        x:Uid="CloseReadOnlyDialog"
+                       Grid.Row="2"
                        x:Load="False"
-                       CloseButtonClick="_DialogCloseClick"
                        DefaultButton="Close" />
 
         <ContentDialog x:Name="MultiLinePasteDialog"
                        x:Uid="MultiLinePasteDialog"
+                       Grid.Row="2"
                        x:Load="False"
-                       CloseButtonClick="_DialogCloseClick"
                        DefaultButton="Primary">
             <StackPanel>
                 <TextBlock x:Uid="MultiLineWarningText"
@@ -134,14 +151,14 @@
 
         <ContentDialog x:Name="LargePasteDialog"
                        x:Uid="LargePasteDialog"
+                       Grid.Row="2"
                        x:Load="False"
-                       CloseButtonClick="_DialogCloseClick"
                        DefaultButton="Primary" />
 
         <ContentDialog x:Name="ControlNoticeDialog"
                        x:Uid="ControlNoticeDialog"
+                       Grid.Row="2"
                        x:Load="False"
-                       CloseButtonClick="_DialogCloseClick"
                        DefaultButton="Primary">
             <TextBlock IsTextSelectionEnabled="True"
                        TextWrapping="WrapWholeWords">
@@ -151,8 +168,8 @@
 
         <ContentDialog x:Name="CouldNotOpenUriDialog"
                        x:Uid="CouldNotOpenUriDialog"
+                       Grid.Row="2"
                        x:Load="False"
-                       CloseButtonClick="_DialogCloseClick"
                        DefaultButton="Primary">
             <TextBlock IsTextSelectionEnabled="True"
                        TextWrapping="WrapWholeWords">