From e93325a7a04cdd23a30bdb9970c74199a0d6eeb5 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 6 Apr 2022 10:33:52 -0500 Subject: [PATCH 1/5] 27 - Once Again, With Feeling --- src/cascadia/TerminalApp/TerminalPage.xaml | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/cascadia/TerminalApp/TerminalPage.xaml b/src/cascadia/TerminalApp/TerminalPage.xaml index 2f6e8f66960..92af7878323 100644 --- a/src/cascadia/TerminalApp/TerminalPage.xaml +++ b/src/cascadia/TerminalApp/TerminalPage.xaml @@ -72,8 +72,26 @@ HorizontalAlignment="Stretch" VerticalAlignment="Stretch" /> + + @@ -96,24 +114,28 @@ @@ -134,12 +156,14 @@ @@ -151,6 +175,7 @@ From 7c017fc606069851d49463c36d67b4df1e8b0dc5 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 6 Apr 2022 11:21:35 -0500 Subject: [PATCH 2/5] Revert "Fix showing a dialog multiple times (#12625)" This reverts commit 2e78ebfdc7626509242475b710640b3ca2f6ca60. --- src/cascadia/TerminalApp/AppLogic.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index fe6636a4f93..d032b30b3fb 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -348,11 +348,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. From ab3fdf96681c7b1f8526ec406e6925d3dcd99768 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 6 Apr 2022 11:38:02 -0500 Subject: [PATCH 3/5] Revert "No longer load content dialogs when there is already one being shown (#12517)" This reverts commit 788d33ce94d28e2903bc49f841ce279211b7f557. --- src/cascadia/TerminalApp/AppLogic.cpp | 8 --- src/cascadia/TerminalApp/AppLogic.h | 1 - src/cascadia/TerminalApp/AppLogic.idl | 7 ++- src/cascadia/TerminalApp/TerminalPage.cpp | 62 ++++++++++------------ src/cascadia/TerminalApp/TerminalPage.h | 3 -- src/cascadia/TerminalApp/TerminalPage.idl | 2 - src/cascadia/TerminalApp/TerminalPage.xaml | 8 --- 7 files changed, 34 insertions(+), 57 deletions(-) diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index d032b30b3fb..c177e3e6163 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -396,14 +396,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 diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h index 582866704b4..1d43f113ba5 100644 --- a/src/cascadia/TerminalApp/AppLogic.h +++ b/src/cascadia/TerminalApp/AppLogic.h @@ -122,7 +122,6 @@ namespace winrt::TerminalApp::implementation bool GetShowTitleInTitlebar(); winrt::Windows::Foundation::IAsyncOperation ShowDialog(winrt::Windows::UI::Xaml::Controls::ContentDialog dialog); - bool CanShowDialog(); void DismissDialog(); Windows::Foundation::Collections::IMapView GlobalHotkeys(); diff --git a/src/cascadia/TerminalApp/AppLogic.idl b/src/cascadia/TerminalApp/AppLogic.idl index df0f7b3cd47..57e7a914e25 100644 --- a/src/cascadia/TerminalApp/AppLogic.idl +++ b/src/cascadia/TerminalApp/AppLogic.idl @@ -33,8 +33,6 @@ namespace TerminalApp SystemMenuItemHandler Handler { get; }; }; - // See IDialogPresenter and TerminalPage's DialogPresenter for more - // information. [default_interface] runtimeclass AppLogic : IDirectKeyListener, IDialogPresenter { AppLogic(); @@ -108,6 +106,11 @@ namespace TerminalApp Windows.Foundation.Collections.IMapView GlobalHotkeys(); + // See IDialogPresenter and TerminalPage's DialogPresenter for more + // information. + Windows.Foundation.IAsyncOperation ShowDialog(Windows.UI.Xaml.Controls.ContentDialog dialog); + void DismissDialog(); + event Windows.Foundation.TypedEventHandler SetTitleBarContent; event Windows.Foundation.TypedEventHandler TitleChanged; event Windows.Foundation.TypedEventHandler LastTabClosed; diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 851aa47a746..bb59d0f9378 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -685,7 +685,10 @@ namespace winrt::TerminalApp::implementation // Notes link, and privacy policy link. void TerminalPage::_ShowAboutDialog() { - _ShowDialogHelper(L"AboutDialog"); + if (auto presenter{ _dialogPresenter.get() }) + { + presenter.ShowDialog(FindName(L"AboutDialog").try_as()); + } } winrt::hstring TerminalPage::ApplicationDisplayName() @@ -705,33 +708,6 @@ 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 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()); - } - } - co_return ContentDialogResult::None; - } - // Method Description: // - Displays a dialog to warn the user that they are about to close all open windows. // Once the user clicks the OK button, shut down the application. @@ -740,7 +716,11 @@ namespace winrt::TerminalApp::implementation // when this is called, nothing happens. See _ShowDialog for details winrt::Windows::Foundation::IAsyncOperation TerminalPage::_ShowQuitDialog() { - return _ShowDialogHelper(L"QuitDialog"); + if (auto presenter{ _dialogPresenter.get() }) + { + co_return co_await presenter.ShowDialog(FindName(L"QuitDialog").try_as()); + } + co_return ContentDialogResult::None; } // Method Description: @@ -752,14 +732,22 @@ namespace winrt::TerminalApp::implementation // when this is called, nothing happens. See _ShowDialog for details winrt::Windows::Foundation::IAsyncOperation TerminalPage::_ShowCloseWarningDialog() { - return _ShowDialogHelper(L"CloseAllDialog"); + if (auto presenter{ _dialogPresenter.get() }) + { + co_return co_await presenter.ShowDialog(FindName(L"CloseAllDialog").try_as()); + } + co_return ContentDialogResult::None; } // Method Description: // - Displays a dialog for warnings found while closing the terminal tab marked as read-only winrt::Windows::Foundation::IAsyncOperation TerminalPage::_ShowCloseReadOnlyDialog() { - return _ShowDialogHelper(L"CloseReadOnlyDialog"); + if (auto presenter{ _dialogPresenter.get() }) + { + co_return co_await presenter.ShowDialog(FindName(L"CloseReadOnlyDialog").try_as()); + } + co_return ContentDialogResult::None; } // Method Description: @@ -772,7 +760,11 @@ namespace winrt::TerminalApp::implementation // when this is called, nothing happens. See _ShowDialog for details winrt::Windows::Foundation::IAsyncOperation TerminalPage::_ShowMultiLinePasteWarningDialog() { - return _ShowDialogHelper(L"MultiLinePasteDialog"); + if (auto presenter{ _dialogPresenter.get() }) + { + co_return co_await presenter.ShowDialog(FindName(L"MultiLinePasteDialog").try_as()); + } + co_return ContentDialogResult::None; } // Method Description: @@ -783,7 +775,11 @@ namespace winrt::TerminalApp::implementation // when this is called, nothing happens. See _ShowDialog for details winrt::Windows::Foundation::IAsyncOperation TerminalPage::_ShowLargePasteWarningDialog() { - return _ShowDialogHelper(L"LargePasteDialog"); + if (auto presenter{ _dialogPresenter.get() }) + { + co_return co_await presenter.ShowDialog(FindName(L"LargePasteDialog").try_as()); + } + co_return ContentDialogResult::None; } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 1a72fe40c12..c93c1001d33 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -219,15 +219,12 @@ namespace winrt::TerminalApp::implementation int _renamerLayoutCount{ 0 }; bool _renamerPressedEnter{ false }; - winrt::Windows::Foundation::IAsyncOperation _ShowDialogHelper(const std::wstring_view& name); - void _ShowAboutDialog(); winrt::Windows::Foundation::IAsyncOperation _ShowQuitDialog(); winrt::Windows::Foundation::IAsyncOperation _ShowCloseWarningDialog(); winrt::Windows::Foundation::IAsyncOperation _ShowCloseReadOnlyDialog(); winrt::Windows::Foundation::IAsyncOperation _ShowMultiLinePasteWarningDialog(); winrt::Windows::Foundation::IAsyncOperation _ShowLargePasteWarningDialog(); - void _DialogCloseClick(const IInspectable& sender, const Windows::UI::Xaml::Controls::ContentDialogButtonClickEventArgs& eventArgs); void _CreateNewTabFlyout(); void _OpenNewTabDropdown(); diff --git a/src/cascadia/TerminalApp/TerminalPage.idl b/src/cascadia/TerminalApp/TerminalPage.idl index b726fcf23ef..979483d0ad8 100644 --- a/src/cascadia/TerminalApp/TerminalPage.idl +++ b/src/cascadia/TerminalApp/TerminalPage.idl @@ -14,8 +14,6 @@ namespace TerminalApp interface IDialogPresenter { Windows.Foundation.IAsyncOperation 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 diff --git a/src/cascadia/TerminalApp/TerminalPage.xaml b/src/cascadia/TerminalApp/TerminalPage.xaml index 92af7878323..86281724e73 100644 --- a/src/cascadia/TerminalApp/TerminalPage.xaml +++ b/src/cascadia/TerminalApp/TerminalPage.xaml @@ -93,7 +93,6 @@ x:Uid="AboutDialog" Grid.Row="2" x:Load="False" - CloseButtonClick="_DialogCloseClick" DefaultButton="Close"> @@ -116,28 +115,24 @@ x:Uid="QuitDialog" Grid.Row="2" x:Load="False" - CloseButtonClick="_DialogCloseClick" DefaultButton="Primary" /> @@ -177,7 +170,6 @@ x:Uid="CouldNotOpenUriDialog" Grid.Row="2" x:Load="False" - CloseButtonClick="_DialogCloseClick" DefaultButton="Primary"> From c8e312855132f90b8079b1cf284c73b83d200204 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 6 Apr 2022 12:00:20 -0500 Subject: [PATCH 4/5] Update src/cascadia/TerminalApp/TerminalPage.xaml Co-authored-by: Dustin L. Howett --- src/cascadia/TerminalApp/TerminalPage.xaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/TerminalPage.xaml b/src/cascadia/TerminalApp/TerminalPage.xaml index 86281724e73..9270e383d56 100644 --- a/src/cascadia/TerminalApp/TerminalPage.xaml +++ b/src/cascadia/TerminalApp/TerminalPage.xaml @@ -82,7 +82,7 @@ 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 - TebContent, fixing the issue. + 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 From 88e00cbbd2a38668921fed8283a0c11d31f44bd9 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 6 Apr 2022 14:37:17 -0500 Subject: [PATCH 5/5] this helper was still useful, even if the rest of the commit was reverted --- src/cascadia/TerminalApp/AppLogic.idl | 2 + src/cascadia/TerminalApp/TerminalPage.cpp | 47 +++++++++-------------- src/cascadia/TerminalApp/TerminalPage.h | 2 + 3 files changed, 22 insertions(+), 29 deletions(-) diff --git a/src/cascadia/TerminalApp/AppLogic.idl b/src/cascadia/TerminalApp/AppLogic.idl index 57e7a914e25..469bf27affd 100644 --- a/src/cascadia/TerminalApp/AppLogic.idl +++ b/src/cascadia/TerminalApp/AppLogic.idl @@ -33,6 +33,8 @@ namespace TerminalApp SystemMenuItemHandler Handler { get; }; }; + // See IDialogPresenter and TerminalPage's DialogPresenter for more + // information. [default_interface] runtimeclass AppLogic : IDirectKeyListener, IDialogPresenter { AppLogic(); diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index bb59d0f9378..62c8885fffa 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -685,10 +685,7 @@ namespace winrt::TerminalApp::implementation // Notes link, and privacy policy link. void TerminalPage::_ShowAboutDialog() { - if (auto presenter{ _dialogPresenter.get() }) - { - presenter.ShowDialog(FindName(L"AboutDialog").try_as()); - } + _ShowDialogHelper(L"AboutDialog"); } winrt::hstring TerminalPage::ApplicationDisplayName() @@ -708,6 +705,18 @@ namespace winrt::TerminalApp::implementation ShellExecute(nullptr, nullptr, currentPath.c_str(), nullptr, nullptr, SW_SHOW); } + // 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 TerminalPage::_ShowDialogHelper(const std::wstring_view& name) + { + if (auto presenter{ _dialogPresenter.get() }) + { + co_return co_await presenter.ShowDialog(FindName(name).try_as()); + } + co_return ContentDialogResult::None; + } + // Method Description: // - Displays a dialog to warn the user that they are about to close all open windows. // Once the user clicks the OK button, shut down the application. @@ -716,11 +725,7 @@ namespace winrt::TerminalApp::implementation // when this is called, nothing happens. See _ShowDialog for details winrt::Windows::Foundation::IAsyncOperation TerminalPage::_ShowQuitDialog() { - if (auto presenter{ _dialogPresenter.get() }) - { - co_return co_await presenter.ShowDialog(FindName(L"QuitDialog").try_as()); - } - co_return ContentDialogResult::None; + return _ShowDialogHelper(L"QuitDialog"); } // Method Description: @@ -732,22 +737,14 @@ namespace winrt::TerminalApp::implementation // when this is called, nothing happens. See _ShowDialog for details winrt::Windows::Foundation::IAsyncOperation TerminalPage::_ShowCloseWarningDialog() { - if (auto presenter{ _dialogPresenter.get() }) - { - co_return co_await presenter.ShowDialog(FindName(L"CloseAllDialog").try_as()); - } - co_return ContentDialogResult::None; + return _ShowDialogHelper(L"CloseAllDialog"); } // Method Description: // - Displays a dialog for warnings found while closing the terminal tab marked as read-only winrt::Windows::Foundation::IAsyncOperation TerminalPage::_ShowCloseReadOnlyDialog() { - if (auto presenter{ _dialogPresenter.get() }) - { - co_return co_await presenter.ShowDialog(FindName(L"CloseReadOnlyDialog").try_as()); - } - co_return ContentDialogResult::None; + return _ShowDialogHelper(L"CloseReadOnlyDialog"); } // Method Description: @@ -760,11 +757,7 @@ namespace winrt::TerminalApp::implementation // when this is called, nothing happens. See _ShowDialog for details winrt::Windows::Foundation::IAsyncOperation TerminalPage::_ShowMultiLinePasteWarningDialog() { - if (auto presenter{ _dialogPresenter.get() }) - { - co_return co_await presenter.ShowDialog(FindName(L"MultiLinePasteDialog").try_as()); - } - co_return ContentDialogResult::None; + return _ShowDialogHelper(L"MultiLinePasteDialog"); } // Method Description: @@ -775,11 +768,7 @@ namespace winrt::TerminalApp::implementation // when this is called, nothing happens. See _ShowDialog for details winrt::Windows::Foundation::IAsyncOperation TerminalPage::_ShowLargePasteWarningDialog() { - if (auto presenter{ _dialogPresenter.get() }) - { - co_return co_await presenter.ShowDialog(FindName(L"LargePasteDialog").try_as()); - } - co_return ContentDialogResult::None; + return _ShowDialogHelper(L"LargePasteDialog"); } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index c93c1001d33..f1be2b6177d 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -219,6 +219,8 @@ namespace winrt::TerminalApp::implementation int _renamerLayoutCount{ 0 }; bool _renamerPressedEnter{ false }; + winrt::Windows::Foundation::IAsyncOperation _ShowDialogHelper(const std::wstring_view& name); + void _ShowAboutDialog(); winrt::Windows::Foundation::IAsyncOperation _ShowQuitDialog(); winrt::Windows::Foundation::IAsyncOperation _ShowCloseWarningDialog();