Pass the window root to the profile page views, instead of the view m…

…odel (#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
microsoft · Feb 10, 2023 · e4bba3c · e4bba3c
1 parent 9bab7d5
commit e4bba3c
Show file tree

Hide file tree

Showing 13 changed files with 53 additions and 24 deletions.
diff --git 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<HWND>(Appearance().WindowRoot().GetHostingWindow()) };
+        const auto parentHwnd{ reinterpret_cast<HWND>(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
@@ -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
@@ -35,7 +35,6 @@ namespace Microsoft.Terminal.Settings.Editor
         void ClearColorScheme();
         ColorSchemeViewModel CurrentColorScheme;
         Windows.Foundation.Collections.IObservableVector<ColorSchemeViewModel> 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
@@ -348,14 +348,14 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
                 const auto currentPage = profile.CurrentPage();
                 if (currentPage == ProfileSubPage::Base)
                 {
-                    contentFrame().Navigate(xaml_typename<Editor::Profiles_Base>(), profile);
+                    contentFrame().Navigate(xaml_typename<Editor::Profiles_Base>(), winrt::make<implementation::NavigateToProfileArgs>(profile, *this));
                     _breadcrumbs.Clear();
                     const auto crumb = winrt::make<Breadcrumb>(breadcrumbTag, breadcrumbText, BreadcrumbSubPage::None);
                     _breadcrumbs.Append(crumb);
                 }
                 else if (currentPage == ProfileSubPage::Appearance)
                 {
-                    contentFrame().Navigate(xaml_typename<Editor::Profiles_Appearance>(), profile);
+                    contentFrame().Navigate(xaml_typename<Editor::Profiles_Appearance>(), winrt::make<implementation::NavigateToProfileArgs>(profile, *this));
                     const auto crumb = winrt::make<Breadcrumb>(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<Editor::Profiles_Base>(), profileVM);
+            contentFrame().Navigate(xaml_typename<Editor::Profiles_Base>(), winrt::make<implementation::NavigateToProfileArgs>(profileVM, *this));
             const auto crumb = winrt::make<Breadcrumb>(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<Editor::Profiles_Base>(), profile);
+        contentFrame().Navigate(xaml_typename<Editor::Profiles_Base>(), winrt::make<implementation::NavigateToProfileArgs>(profile, *this));
         const auto crumb = winrt::make<Breadcrumb>(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
@@ -248,7 +248,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
 
         _unfocusedAppearanceViewModel = winrt::make<implementation::AppearanceViewModel>(_profile.UnfocusedAppearance().try_as<AppearanceConfig>());
         _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<Editor::ColorSchemeViewModel> schemesList, Editor::IHostedInWindow windowRoot)
+    void ProfileViewModel::SetupAppearances(Windows::Foundation::Collections::IObservableVector<Editor::ColorSchemeViewModel> 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
@@ -4,13 +4,29 @@
 #pragma once
 
 #include "DeleteProfileEventArgs.g.h"
+#include "NavigateToProfileArgs.g.h"
 #include "ProfileViewModel.g.h"
 #include "Utils.h"
 #include "ViewModelHelpers.h"
 #include "Appearances.h"
 
 namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
 {
+    struct NavigateToProfileArgs : NavigateToProfileArgsT<NavigateToProfileArgs>
+    {
+    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<ProfileViewModel>, ViewModelHelper<ProfileViewModel>
     {
     public:
@@ -23,7 +39,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         Model::TerminalSettings TermSettings() const;
         void DeleteProfile();
 
-        void SetupAppearances(Windows::Foundation::Collections::IObservableVector<Editor::ColorSchemeViewModel> schemesList, Editor::IHostedInWindow windowRoot);
+        void SetupAppearances(Windows::Foundation::Collections::IObservableVector<Editor::ColorSchemeViewModel> 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
@@ -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<ProfileViewModel, DeleteProfileEventArgs> DeleteProfile;
 
-        void SetupAppearances(Windows.Foundation.Collections.IObservableVector<ColorSchemeViewModel> schemesList, IHostedInWindow windowRoot);
+        void SetupAppearances(Windows.Foundation.Collections.IObservableVector<ColorSchemeViewModel> 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
@@ -28,7 +28,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
 
     void Profiles_Appearance::OnNavigatedTo(const NavigationEventArgs& e)
     {
-        _Profile = e.Parameter().as<Editor::ProfileViewModel>();
+        const auto args = e.Parameter().as<Editor::NavigateToProfileArgs>();
+        _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
@@ -19,13 +19,16 @@ 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);
 
     private:
         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
@@ -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
@@ -56,7 +56,8 @@
                 </Border>
 
                 <local:Appearances Appearance="{x:Bind Profile.DefaultAppearance, Mode=OneWay}"
-                                   SourceProfile="{x:Bind Profile, Mode=OneWay}" />
+                                   SourceProfile="{x:Bind Profile, Mode=OneWay}"
+                                   WindowRoot="{x:Bind WindowRoot, Mode=OneTime}" />
 
                 <!--  Grouping: Transparency  -->
                 <StackPanel Style="{StaticResource PivotStackStyle}">
@@ -174,7 +175,8 @@
                     </StackPanel>
                     <local:Appearances Appearance="{x:Bind Profile.UnfocusedAppearance, Mode=OneWay}"
                                        SourceProfile="{x:Bind Profile, Mode=OneWay}"
-                                       Visibility="{x:Bind Profile.ShowUnfocusedAppearance, Mode=OneWay}" />
+                                       Visibility="{x:Bind Profile.ShowUnfocusedAppearance, Mode=OneWay}"
+                                       WindowRoot="{x:Bind WindowRoot, Mode=OneTime}" />
                 </StackPanel>
             </StackPanel>
         </ScrollViewer>

diff --git 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<Editor::ProfileViewModel>();
+        const auto args = e.Parameter().as<Editor::NavigateToProfileArgs>();
+        _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<HWND>(winrt::get_self<ProfileViewModel>(_Profile)->WindowRoot().GetHostingWindow()) };
+        const auto parentHwnd{ reinterpret_cast<HWND>(_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<HWND>(winrt::get_self<ProfileViewModel>(_Profile)->WindowRoot().GetHostingWindow()) };
+        const auto parentHwnd{ reinterpret_cast<HWND>(_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<HWND>(winrt::get_self<ProfileViewModel>(_Profile)->WindowRoot().GetHostingWindow()) };
+        const auto parentHwnd{ reinterpret_cast<HWND>(_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
@@ -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;
     };
 };