From f94da4f78257156e5382e73163920178efb2cc31 Mon Sep 17 00:00:00 2001 From: PankajBhojwani Date: Fri, 18 Feb 2022 14:31:39 -0800 Subject: [PATCH] No longer load content dialogs when there is already one being shown (#12517) Somehow, the controls v2 update caused an issue where if you as much as _load_ a content dialog when there's already one open, we get holes in the terminal window (#12447) This commit introduces logic to `TerminalPage` to check whether there is a content dialog open before we try to load another one. * [x] Closes #12447 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [x] I work here Can no longer repro #12447 --- 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, 57 insertions(+), 34 deletions(-) diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index 2dc201c876c..e9b22b4b1dd 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -424,6 +424,14 @@ 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 e87f3d59bfe..39acf807f78 100644 --- a/src/cascadia/TerminalApp/AppLogic.h +++ b/src/cascadia/TerminalApp/AppLogic.h @@ -107,6 +107,7 @@ 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 32d413bbf40..da2bc57cc2f 100644 --- a/src/cascadia/TerminalApp/AppLogic.idl +++ b/src/cascadia/TerminalApp/AppLogic.idl @@ -19,6 +19,8 @@ namespace TerminalApp String WindowName { get; }; }; + // See IDialogPresenter and TerminalPage's DialogPresenter for more + // information. [default_interface] runtimeclass AppLogic : IDirectKeyListener, IDialogPresenter { AppLogic(); @@ -89,11 +91,6 @@ 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 1713bef1d6b..05389d2022f 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -561,10 +561,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() @@ -584,6 +581,33 @@ 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. @@ -592,11 +616,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: @@ -608,22 +628,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: @@ -636,11 +648,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: @@ -651,11 +659,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 1d8cf439060..29334269a16 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -204,12 +204,15 @@ namespace winrt::TerminalApp::implementation std::shared_ptr _windowIdToast{ nullptr }; std::shared_ptr _windowRenameFailedToast{ nullptr }; + 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 979483d0ad8..b726fcf23ef 100644 --- a/src/cascadia/TerminalApp/TerminalPage.idl +++ b/src/cascadia/TerminalApp/TerminalPage.idl @@ -14,6 +14,8 @@ 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 f7855356634..5ca8ed25858 100644 --- a/src/cascadia/TerminalApp/TerminalPage.xaml +++ b/src/cascadia/TerminalApp/TerminalPage.xaml @@ -75,6 +75,7 @@ @@ -96,21 +97,25 @@ @@ -145,6 +152,7 @@