From 163e0d59607c64e555eaeb54c2833e5df8f8ecb5 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Thu, 17 Feb 2022 10:46:01 -0800 Subject: [PATCH 1/4] don't load dialog if one is already open --- src/cascadia/TerminalApp/AppLogic.cpp | 8 +++++ src/cascadia/TerminalApp/AppLogic.h | 1 + src/cascadia/TerminalApp/AppLogic.idl | 2 +- src/cascadia/TerminalApp/TerminalPage.cpp | 39 ++++++++++++++++++---- src/cascadia/TerminalApp/TerminalPage.h | 1 + src/cascadia/TerminalApp/TerminalPage.idl | 2 ++ src/cascadia/TerminalApp/TerminalPage.xaml | 8 +++++ 7 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index fc63fc0cc37..71698f7c4c2 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -396,6 +396,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 1d43f113ba5..582866704b4 100644 --- a/src/cascadia/TerminalApp/AppLogic.h +++ b/src/cascadia/TerminalApp/AppLogic.h @@ -122,6 +122,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 57e7a914e25..ee8b4bee687 100644 --- a/src/cascadia/TerminalApp/AppLogic.idl +++ b/src/cascadia/TerminalApp/AppLogic.idl @@ -109,7 +109,7 @@ namespace TerminalApp // See IDialogPresenter and TerminalPage's DialogPresenter for more // information. Windows.Foundation.IAsyncOperation ShowDialog(Windows.UI.Xaml.Controls.ContentDialog dialog); - void DismissDialog(); + Boolean CanShowDialog(); event Windows.Foundation.TypedEventHandler SetTitleBarContent; event Windows.Foundation.TypedEventHandler TitleChanged; diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 630378a5d07..9c50f30454c 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -688,7 +688,10 @@ namespace winrt::TerminalApp::implementation { if (auto presenter{ _dialogPresenter.get() }) { - presenter.ShowDialog(FindName(L"AboutDialog").try_as()); + if (presenter.CanShowDialog()) + { + presenter.ShowDialog(FindName(L"AboutDialog").try_as()); + } } } @@ -709,6 +712,15 @@ namespace winrt::TerminalApp::implementation ShellExecute(nullptr, nullptr, currentPath.c_str(), nullptr, nullptr, SW_SHOW); } + void TerminalPage::_DialogCloseClick(const IInspectable&, + const ContentDialogButtonClickEventArgs&) + { + if (auto presenter{ _dialogPresenter.get() }) + { + presenter.DismissDialog(); + } + } + // 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. @@ -719,7 +731,10 @@ namespace winrt::TerminalApp::implementation { if (auto presenter{ _dialogPresenter.get() }) { - co_return co_await presenter.ShowDialog(FindName(L"QuitDialog").try_as()); + if (presenter.CanShowDialog()) + { + co_return co_await presenter.ShowDialog(FindName(L"QuitDialog").try_as()); + } } co_return ContentDialogResult::None; } @@ -735,7 +750,10 @@ namespace winrt::TerminalApp::implementation { if (auto presenter{ _dialogPresenter.get() }) { - co_return co_await presenter.ShowDialog(FindName(L"CloseAllDialog").try_as()); + if (presenter.CanShowDialog()) + { + co_return co_await presenter.ShowDialog(FindName(L"CloseAllDialog").try_as()); + } } co_return ContentDialogResult::None; } @@ -746,7 +764,10 @@ namespace winrt::TerminalApp::implementation { if (auto presenter{ _dialogPresenter.get() }) { - co_return co_await presenter.ShowDialog(FindName(L"CloseReadOnlyDialog").try_as()); + if (presenter.CanShowDialog()) + { + co_return co_await presenter.ShowDialog(FindName(L"CloseReadOnlyDialog").try_as()); + } } co_return ContentDialogResult::None; } @@ -763,7 +784,10 @@ namespace winrt::TerminalApp::implementation { if (auto presenter{ _dialogPresenter.get() }) { - co_return co_await presenter.ShowDialog(FindName(L"MultiLinePasteDialog").try_as()); + if (presenter.CanShowDialog()) + { + co_return co_await presenter.ShowDialog(FindName(L"MultiLinePasteDialog").try_as()); + } } co_return ContentDialogResult::None; } @@ -778,7 +802,10 @@ namespace winrt::TerminalApp::implementation { if (auto presenter{ _dialogPresenter.get() }) { - co_return co_await presenter.ShowDialog(FindName(L"LargePasteDialog").try_as()); + if (presenter.CanShowDialog()) + { + co_return co_await presenter.ShowDialog(FindName(L"LargePasteDialog").try_as()); + } } co_return ContentDialogResult::None; } diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index a1867178590..08d84a79ee3 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -221,6 +221,7 @@ namespace winrt::TerminalApp::implementation 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 @@ From 0cc7af4b01955ac119167a4db9fd65e51deed358 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 18 Feb 2022 10:01:21 -0800 Subject: [PATCH 2/4] show dialog helper --- src/cascadia/TerminalApp/TerminalPage.cpp | 61 ++++++++--------------- src/cascadia/TerminalApp/TerminalPage.h | 2 + 2 files changed, 24 insertions(+), 39 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 0bb6ae032fe..aab23bfe877 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -711,6 +711,9 @@ 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&) { @@ -721,23 +724,31 @@ namespace winrt::TerminalApp::implementation } // 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. - // If cancel is clicked, the dialog will close. - // - Only one dialog can be visible at a time. If another dialog is visible - // when this is called, nothing happens. See _ShowDialog for details - winrt::Windows::Foundation::IAsyncOperation TerminalPage::_ShowQuitDialog() + // - 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(L"QuitDialog").try_as()); + 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. + // If cancel is clicked, the dialog will close. + // - Only one dialog can be visible at a time. If another dialog is visible + // when this is called, nothing happens. See _ShowDialog for details + winrt::Windows::Foundation::IAsyncOperation TerminalPage::_ShowQuitDialog() + { + return _ShowDialogHelper(L"QuitDialog"); + } + // Method Description: // - Displays a dialog for warnings found while closing the terminal app using // key binding with multiple tabs opened. Display messages to warn user @@ -747,28 +758,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() }) - { - if (presenter.CanShowDialog()) - { - 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() }) - { - if (presenter.CanShowDialog()) - { - co_return co_await presenter.ShowDialog(FindName(L"CloseReadOnlyDialog").try_as()); - } - } - co_return ContentDialogResult::None; + return _ShowDialogHelper(L"CloseReadOnlyDialog"); } // Method Description: @@ -781,14 +778,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() }) - { - if (presenter.CanShowDialog()) - { - co_return co_await presenter.ShowDialog(FindName(L"MultiLinePasteDialog").try_as()); - } - } - co_return ContentDialogResult::None; + return _ShowDialogHelper(L"MultiLinePasteDialog"); } // Method Description: @@ -799,14 +789,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() }) - { - if (presenter.CanShowDialog()) - { - 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 08d84a79ee3..2e48d9db883 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -215,6 +215,8 @@ 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(); From c3ed3234007afe92970d708bde54a99479bafb25 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 18 Feb 2022 10:06:15 -0800 Subject: [PATCH 3/4] about dialog helper --- src/cascadia/TerminalApp/TerminalPage.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index aab23bfe877..8f79e9b8090 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -685,13 +685,7 @@ namespace winrt::TerminalApp::implementation // Notes link, and privacy policy link. void TerminalPage::_ShowAboutDialog() { - if (auto presenter{ _dialogPresenter.get() }) - { - if (presenter.CanShowDialog()) - { - presenter.ShowDialog(FindName(L"AboutDialog").try_as()); - } - } + _ShowDialogHelper(L"AboutDialog"); } winrt::hstring TerminalPage::ApplicationDisplayName() From 70f8d64b1150e58eae91957f153c1f3506bbaf62 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 18 Feb 2022 13:07:54 -0800 Subject: [PATCH 4/4] remove idialogpresenter methods --- src/cascadia/TerminalApp/AppLogic.idl | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/cascadia/TerminalApp/AppLogic.idl b/src/cascadia/TerminalApp/AppLogic.idl index ee8b4bee687..df0f7b3cd47 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(); @@ -106,11 +108,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); - Boolean CanShowDialog(); - event Windows.Foundation.TypedEventHandler SetTitleBarContent; event Windows.Foundation.TypedEventHandler TitleChanged; event Windows.Foundation.TypedEventHandler LastTabClosed;