From e4bba3cd9a3ecc99d2b89df54756beb89e37a196 Mon Sep 17 00:00:00 2001 From: PankajBhojwani Date: Fri, 10 Feb 2023 15:39:08 -0800 Subject: [PATCH] Pass the window root to the profile page views, instead of the view model (#14816) ## Summary of the Pull Request Let the profile pages' views have access to the window root, rather than the `ProfileViewModel`. The window root is passed along when the page is navigated to. ## Validation Steps Performed Clicking `Browse` no longer crashes. ## PR Checklist - [x] Closes #14808 --- .../TerminalSettingsEditor/Appearances.cpp | 2 +- .../TerminalSettingsEditor/Appearances.h | 2 +- .../TerminalSettingsEditor/Appearances.idl | 2 +- .../TerminalSettingsEditor/MainPage.cpp | 14 +++++++------- .../ProfileViewModel.cpp | 5 +---- .../TerminalSettingsEditor/ProfileViewModel.h | 19 +++++++++++++++++-- .../ProfileViewModel.idl | 8 +++++++- .../Profiles_Appearance.cpp | 4 +++- .../Profiles_Appearance.h | 3 +++ .../Profiles_Appearance.idl | 1 + .../Profiles_Appearance.xaml | 6 ++++-- .../TerminalSettingsEditor/Profiles_Base.cpp | 10 ++++++---- .../TerminalSettingsEditor/Profiles_Base.h | 1 + 13 files changed, 53 insertions(+), 24 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.cpp b/src/cascadia/TerminalSettingsEditor/Appearances.cpp index 724faa3b0d..8cf0079138 100644 --- a/src/cascadia/TerminalSettingsEditor/Appearances.cpp +++ b/src/cascadia/TerminalSettingsEditor/Appearances.cpp @@ -338,7 +338,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { auto lifetime = get_strong(); - const auto parentHwnd{ reinterpret_cast(Appearance().WindowRoot().GetHostingWindow()) }; + const auto parentHwnd{ reinterpret_cast(WindowRoot().GetHostingWindow()) }; auto file = co_await OpenImagePicker(parentHwnd); if (!file.empty()) { diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.h b/src/cascadia/TerminalSettingsEditor/Appearances.h index 1be2b1ea2c..13f4719a30 100644 --- a/src/cascadia/TerminalSettingsEditor/Appearances.h +++ b/src/cascadia/TerminalSettingsEditor/Appearances.h @@ -74,7 +74,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void CurrentColorScheme(const Editor::ColorSchemeViewModel& val); WINRT_PROPERTY(bool, IsDefault, false); - WINRT_PROPERTY(IHostedInWindow, WindowRoot, nullptr); // These settings are not defined in AppearanceConfig, so we grab them // from the source profile itself. The reason we still want them in the @@ -134,6 +133,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler); DEPENDENCY_PROPERTY(Editor::AppearanceViewModel, Appearance); WINRT_PROPERTY(Editor::ProfileViewModel, SourceProfile, nullptr); + WINRT_PROPERTY(IHostedInWindow, WindowRoot, nullptr); GETSET_BINDABLE_ENUM_SETTING(BackgroundImageStretchMode, Windows::UI::Xaml::Media::Stretch, Appearance().BackgroundImageStretchMode); diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.idl b/src/cascadia/TerminalSettingsEditor/Appearances.idl index 23bdfdecb6..2a83e44459 100644 --- a/src/cascadia/TerminalSettingsEditor/Appearances.idl +++ b/src/cascadia/TerminalSettingsEditor/Appearances.idl @@ -35,7 +35,6 @@ namespace Microsoft.Terminal.Settings.Editor void ClearColorScheme(); ColorSchemeViewModel CurrentColorScheme; Windows.Foundation.Collections.IObservableVector SchemesList; - IHostedInWindow WindowRoot; // necessary to send the right HWND into the file picker dialogs. OBSERVABLE_PROJECTED_APPEARANCE_SETTING(String, FontFace); OBSERVABLE_PROJECTED_APPEARANCE_SETTING(Single, FontSize); @@ -59,6 +58,7 @@ namespace Microsoft.Terminal.Settings.Editor Appearances(); AppearanceViewModel Appearance; ProfileViewModel SourceProfile; + IHostedInWindow WindowRoot; static Windows.UI.Xaml.DependencyProperty AppearanceProperty { get; }; Boolean UsingMonospaceFont { get; }; diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index 7140b83c8b..493347479c 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -348,14 +348,14 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation const auto currentPage = profile.CurrentPage(); if (currentPage == ProfileSubPage::Base) { - contentFrame().Navigate(xaml_typename(), profile); + contentFrame().Navigate(xaml_typename(), winrt::make(profile, *this)); _breadcrumbs.Clear(); const auto crumb = winrt::make(breadcrumbTag, breadcrumbText, BreadcrumbSubPage::None); _breadcrumbs.Append(crumb); } else if (currentPage == ProfileSubPage::Appearance) { - contentFrame().Navigate(xaml_typename(), profile); + contentFrame().Navigate(xaml_typename(), winrt::make(profile, *this)); const auto crumb = winrt::make(breadcrumbTag, RS_(L"Profile_Appearance/Header"), BreadcrumbSubPage::Profile_Appearance); _breadcrumbs.Append(crumb); } @@ -400,12 +400,12 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation else if (clickedItemTag == globalProfileTag) { auto profileVM{ _viewModelForProfile(_settingsClone.ProfileDefaults(), _settingsClone) }; - profileVM.SetupAppearances(_colorSchemesPageVM.AllColorSchemes(), *this); + profileVM.SetupAppearances(_colorSchemesPageVM.AllColorSchemes()); profileVM.IsBaseLayer(true); _SetupProfileEventHandling(profileVM); - contentFrame().Navigate(xaml_typename(), profileVM); + contentFrame().Navigate(xaml_typename(), winrt::make(profileVM, *this)); const auto crumb = winrt::make(box_value(clickedItemTag), RS_(L"Nav_ProfileDefaults/Content"), BreadcrumbSubPage::None); _breadcrumbs.Append(crumb); @@ -457,7 +457,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation _SetupProfileEventHandling(profile); - contentFrame().Navigate(xaml_typename(), profile); + contentFrame().Navigate(xaml_typename(), winrt::make(profile, *this)); const auto crumb = winrt::make(box_value(profile), profile.Name(), BreadcrumbSubPage::None); _breadcrumbs.Append(crumb); @@ -538,7 +538,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation if (!profile.Deleted()) { auto profileVM = _viewModelForProfile(profile, _settingsClone); - profileVM.SetupAppearances(_colorSchemesPageVM.AllColorSchemes(), *this); + profileVM.SetupAppearances(_colorSchemesPageVM.AllColorSchemes()); auto navItem = _CreateProfileNavViewItem(profileVM); menuItems.Append(navItem); } @@ -561,7 +561,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { const auto newProfile{ profile ? profile : _settingsClone.CreateNewProfile() }; const auto profileViewModel{ _viewModelForProfile(newProfile, _settingsClone) }; - profileViewModel.SetupAppearances(_colorSchemesPageVM.AllColorSchemes(), *this); + profileViewModel.SetupAppearances(_colorSchemesPageVM.AllColorSchemes()); const auto navItem{ _CreateProfileNavViewItem(profileViewModel) }; SettingsNav().MenuItems().InsertAt(index, navItem); diff --git a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp index 3ccdfbd214..58cb8a74a8 100644 --- a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp +++ b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp @@ -248,7 +248,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation _unfocusedAppearanceViewModel = winrt::make(_profile.UnfocusedAppearance().try_as()); _unfocusedAppearanceViewModel.SchemesList(DefaultAppearance().SchemesList()); - _unfocusedAppearanceViewModel.WindowRoot(DefaultAppearance().WindowRoot()); _NotifyChanges(L"UnfocusedAppearance", L"HasUnfocusedAppearance", L"ShowUnfocusedAppearance"); } @@ -350,14 +349,12 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation _DeleteProfileHandlers(*this, *deleteProfileArgs); } - void ProfileViewModel::SetupAppearances(Windows::Foundation::Collections::IObservableVector schemesList, Editor::IHostedInWindow windowRoot) + void ProfileViewModel::SetupAppearances(Windows::Foundation::Collections::IObservableVector schemesList) { DefaultAppearance().SchemesList(schemesList); - DefaultAppearance().WindowRoot(windowRoot); if (UnfocusedAppearance()) { UnfocusedAppearance().SchemesList(schemesList); - UnfocusedAppearance().WindowRoot(windowRoot); } } } diff --git a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.h b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.h index 4cd498b97e..386c4876a1 100644 --- a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.h +++ b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.h @@ -4,6 +4,7 @@ #pragma once #include "DeleteProfileEventArgs.g.h" +#include "NavigateToProfileArgs.g.h" #include "ProfileViewModel.g.h" #include "Utils.h" #include "ViewModelHelpers.h" @@ -11,6 +12,21 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { + struct NavigateToProfileArgs : NavigateToProfileArgsT + { + public: + NavigateToProfileArgs(ProfileViewModel profile, Editor::IHostedInWindow windowRoot) : + _Profile(profile), + _WindowRoot(windowRoot) {} + + Editor::IHostedInWindow WindowRoot() const noexcept { return _WindowRoot; } + Editor::ProfileViewModel Profile() const noexcept { return _Profile; } + + private: + Editor::IHostedInWindow _WindowRoot; + Editor::ProfileViewModel _Profile{ nullptr }; + }; + struct ProfileViewModel : ProfileViewModelT, ViewModelHelper { public: @@ -23,7 +39,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation Model::TerminalSettings TermSettings() const; void DeleteProfile(); - void SetupAppearances(Windows::Foundation::Collections::IObservableVector schemesList, Editor::IHostedInWindow windowRoot); + void SetupAppearances(Windows::Foundation::Collections::IObservableVector schemesList); // bell style bits bool IsBellStyleFlagSet(const uint32_t flag); @@ -91,7 +107,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation WINRT_PROPERTY(bool, IsBaseLayer, false); WINRT_PROPERTY(bool, FocusDeleteButton, false); - WINRT_PROPERTY(IHostedInWindow, WindowRoot, nullptr); GETSET_BINDABLE_ENUM_SETTING(AntiAliasingMode, Microsoft::Terminal::Control::TextAntialiasingMode, AntialiasingMode); GETSET_BINDABLE_ENUM_SETTING(CloseOnExitMode, Microsoft::Terminal::Settings::Model::CloseOnExitMode, CloseOnExit); GETSET_BINDABLE_ENUM_SETTING(ScrollState, Microsoft::Terminal::Control::ScrollbarState, ScrollState); diff --git a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.idl b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.idl index fd4ceaa675..f4dd21b9c7 100644 --- a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.idl +++ b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.idl @@ -14,6 +14,12 @@ import "ColorSchemesPageViewModel.idl"; namespace Microsoft.Terminal.Settings.Editor { + runtimeclass NavigateToProfileArgs + { + ProfileViewModel Profile { get; }; + IHostedInWindow WindowRoot { get; }; + } + runtimeclass DeleteProfileEventArgs { Guid ProfileGuid { get; }; @@ -35,7 +41,7 @@ namespace Microsoft.Terminal.Settings.Editor event Windows.Foundation.TypedEventHandler DeleteProfile; - void SetupAppearances(Windows.Foundation.Collections.IObservableVector schemesList, IHostedInWindow windowRoot); + void SetupAppearances(Windows.Foundation.Collections.IObservableVector schemesList); void SetAcrylicOpacityPercentageValue(Double value); void SetPadding(Double value); diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp index 9a62160374..a19a0b4e7d 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp @@ -28,7 +28,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void Profiles_Appearance::OnNavigatedTo(const NavigationEventArgs& e) { - _Profile = e.Parameter().as(); + const auto args = e.Parameter().as(); + _Profile = args.Profile(); + _windowRoot = args.WindowRoot(); // generate the font list, if we don't have one if (_Profile.CompleteFontList() || !_Profile.MonospaceFontList()) diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h index 2ddc393134..3b42e8afac 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h @@ -19,6 +19,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void CreateUnfocusedAppearance_Click(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& e); void DeleteUnfocusedAppearance_Click(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& e); + Editor::IHostedInWindow WindowRoot() { return _windowRoot; }; + WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler); WINRT_PROPERTY(Editor::ProfileViewModel, Profile, nullptr); @@ -26,6 +28,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation Microsoft::Terminal::Control::TermControl _previewControl; Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker _ViewModelChangedRevoker; Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker _AppearanceViewModelChangedRevoker; + Editor::IHostedInWindow _windowRoot; }; }; diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.idl b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.idl index b80311ca40..931edbe519 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.idl +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.idl @@ -9,6 +9,7 @@ namespace Microsoft.Terminal.Settings.Editor { Profiles_Appearance(); ProfileViewModel Profile { get; }; + IHostedInWindow WindowRoot { get; }; Windows.UI.Xaml.Controls.Slider OpacitySlider { get; }; } diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.xaml b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.xaml index b8b334bcaf..3af18b1223 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.xaml +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.xaml @@ -56,7 +56,8 @@ + SourceProfile="{x:Bind Profile, Mode=OneWay}" + WindowRoot="{x:Bind WindowRoot, Mode=OneTime}" /> @@ -174,7 +175,8 @@ + Visibility="{x:Bind Profile.ShowUnfocusedAppearance, Mode=OneWay}" + WindowRoot="{x:Bind WindowRoot, Mode=OneTime}" /> diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Base.cpp b/src/cascadia/TerminalSettingsEditor/Profiles_Base.cpp index 6344894e2f..32570a016b 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Base.cpp +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Base.cpp @@ -29,7 +29,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void Profiles_Base::OnNavigatedTo(const NavigationEventArgs& e) { - _Profile = e.Parameter().as(); + const auto args = e.Parameter().as(); + _Profile = args.Profile(); + _windowRoot = args.WindowRoot(); // Check the use parent directory box if the starting directory is empty if (_Profile.StartingDirectory().empty()) @@ -85,7 +87,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation }; static constexpr winrt::guid clientGuidExecutables{ 0x2E7E4331, 0x0800, 0x48E6, { 0xB0, 0x17, 0xA1, 0x4C, 0xD8, 0x73, 0xDD, 0x58 } }; - const auto parentHwnd{ reinterpret_cast(winrt::get_self(_Profile)->WindowRoot().GetHostingWindow()) }; + const auto parentHwnd{ reinterpret_cast(_windowRoot.GetHostingWindow()) }; auto path = co_await OpenFilePicker(parentHwnd, [](auto&& dialog) { THROW_IF_FAILED(dialog->SetClientGuid(clientGuidExecutables)); try @@ -109,7 +111,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { auto lifetime = get_strong(); - const auto parentHwnd{ reinterpret_cast(winrt::get_self(_Profile)->WindowRoot().GetHostingWindow()) }; + const auto parentHwnd{ reinterpret_cast(_windowRoot.GetHostingWindow()) }; auto file = co_await OpenImagePicker(parentHwnd); if (!file.empty()) { @@ -120,7 +122,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation fire_and_forget Profiles_Base::StartingDirectory_Click(const IInspectable&, const RoutedEventArgs&) { auto lifetime = get_strong(); - const auto parentHwnd{ reinterpret_cast(winrt::get_self(_Profile)->WindowRoot().GetHostingWindow()) }; + const auto parentHwnd{ reinterpret_cast(_windowRoot.GetHostingWindow()) }; auto folder = co_await OpenFilePicker(parentHwnd, [](auto&& dialog) { static constexpr winrt::guid clientGuidFolderPicker{ 0xAADAA433, 0xB04D, 0x4BAE, { 0xB1, 0xEA, 0x1E, 0x6C, 0xD1, 0xCD, 0xA6, 0x8B } }; THROW_IF_FAILED(dialog->SetClientGuid(clientGuidFolderPicker)); diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Base.h b/src/cascadia/TerminalSettingsEditor/Profiles_Base.h index 42067438ee..4f711f5bb1 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Base.h +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Base.h @@ -30,6 +30,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation private: Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker _ViewModelChangedRevoker; winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker; + Editor::IHostedInWindow _windowRoot; }; };