profiles: fix BringIntoView (partially)

Author: carlos-zamora

src/cascadia/TerminalSettingsEditor/MainPage.cpp

@@ -297,7 +297,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
                 // extract ElementToFocus and clear it; we only want to use it once
                 auto vmImpl = get_self<ColorSchemesPageViewModel>(_colorSchemesPageVM);
                 const auto elementToFocus = vmImpl->ElementToFocus();
-                vmImpl->ElementToFocus(L"");
+                vmImpl->ElementToFocus({});
 
                 const auto currentScheme = _colorSchemesPageVM.CurrentScheme();
                 if (_colorSchemesPageVM.CurrentPage() == ColorSchemesSubPage::EditColorScheme && currentScheme)
@@ -634,31 +634,37 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             const auto settingName{ args.PropertyName() };
             if (settingName == L"CurrentPage")
             {
+                // extract ElementToFocus and clear it; we only want to use it once
+                auto vmImpl = get_self<ProfileViewModel>(profile);
+                const auto elementToFocus = vmImpl->ElementToFocus();
+                vmImpl->ElementToFocus({});
+
                 const auto currentPage = profile.CurrentPage();
                 if (currentPage == ProfileSubPage::Base)
                 {
-                    contentFrame().Navigate(xaml_typename<Editor::Profiles_Base>(), winrt::make<implementation::NavigateToProfileArgs>(profile, *this));
+                    contentFrame().Navigate(xaml_typename<Editor::Profiles_Base>(), winrt::make<implementation::NavigateToProfileArgs>(profile, *this, elementToFocus));
                     _breadcrumbs.Clear();
                     const auto crumb = winrt::make<Breadcrumb>(breadcrumbTag, breadcrumbText, BreadcrumbSubPage::None);
                     _breadcrumbs.Append(crumb);
+                    SettingsMainPage_ScrollViewer().ScrollToVerticalOffset(0);
                 }
                 else if (currentPage == ProfileSubPage::Appearance)
                 {
-                    contentFrame().Navigate(xaml_typename<Editor::Profiles_Appearance>(), winrt::make<implementation::NavigateToProfileArgs>(profile, *this));
+                    contentFrame().Navigate(xaml_typename<Editor::Profiles_Appearance>(), winrt::make<implementation::NavigateToProfileArgs>(profile, *this, elementToFocus));
                     const auto crumb = winrt::make<Breadcrumb>(breadcrumbTag, RS_(L"Profile_Appearance/Header"), BreadcrumbSubPage::Profile_Appearance);
                     _breadcrumbs.Append(crumb);
                     SettingsMainPage_ScrollViewer().ScrollToVerticalOffset(0);
                 }
                 else if (currentPage == ProfileSubPage::Terminal)
                 {
-                    contentFrame().Navigate(xaml_typename<Editor::Profiles_Terminal>(), winrt::make<implementation::NavigateToProfileArgs>(profile, *this));
+                    contentFrame().Navigate(xaml_typename<Editor::Profiles_Terminal>(), winrt::make<implementation::NavigateToProfileArgs>(profile, *this, elementToFocus));
                     const auto crumb = winrt::make<Breadcrumb>(breadcrumbTag, RS_(L"Profile_Terminal/Header"), BreadcrumbSubPage::Profile_Terminal);
                     _breadcrumbs.Append(crumb);
                     SettingsMainPage_ScrollViewer().ScrollToVerticalOffset(0);
                 }
                 else if (currentPage == ProfileSubPage::Advanced)
                 {
-                    contentFrame().Navigate(xaml_typename<Editor::Profiles_Advanced>(), winrt::make<implementation::NavigateToProfileArgs>(profile, *this));
+                    contentFrame().Navigate(xaml_typename<Editor::Profiles_Advanced>(), winrt::make<implementation::NavigateToProfileArgs>(profile, *this, elementToFocus));
                     const auto crumb = winrt::make<Breadcrumb>(breadcrumbTag, RS_(L"Profile_Advanced/Header"), BreadcrumbSubPage::Profile_Advanced);
                     _breadcrumbs.Append(crumb);
                     SettingsMainPage_ScrollViewer().ScrollToVerticalOffset(0);
@@ -844,23 +850,38 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             SettingsNav().SelectedItem(profileNavItem);
         }
 
+        // Pass along the element to focus to the ProfileViewModel.
+        // This will work as a staging area before we navigate to the correct sub-page
+        auto profileVMImpl = get_self<ProfileViewModel>(profile);
+        profileVMImpl->ElementToFocus(elementToFocus);
+
+        // convert the BreadcrumbSubPage to ProfileSubPage
+        const ProfileSubPage profileSubPage = [&](BreadcrumbSubPage subPage) {
+            switch (subPage)
+            {
+            case BreadcrumbSubPage::None:
+                return ProfileSubPage::Base;
+            case BreadcrumbSubPage::Profile_Appearance:
+                return ProfileSubPage::Appearance;
+            case BreadcrumbSubPage::Profile_Terminal:
+                return ProfileSubPage::Terminal;
+            case BreadcrumbSubPage::Profile_Advanced:
+                return ProfileSubPage::Advanced;
+            default:
+                // This should never happen
+                assert(false);
+            }
+        }(subPage);
+        const bool needsForcedRefresh = profile.CurrentPage() == profileSubPage;
+
         // Set the profile's 'CurrentPage' to the correct one, if this requires further navigation, the
         // event handler will do it
-        if (subPage == BreadcrumbSubPage::None)
-        {
-            profile.CurrentPage(ProfileSubPage::Base);
-        }
-        else if (subPage == BreadcrumbSubPage::Profile_Appearance)
-        {
-            profile.CurrentPage(ProfileSubPage::Appearance);
-        }
-        else if (subPage == BreadcrumbSubPage::Profile_Terminal)
-        {
-            profile.CurrentPage(ProfileSubPage::Terminal);
-        }
-        else if (subPage == BreadcrumbSubPage::Profile_Advanced)
+        profile.CurrentPage(profileSubPage);
+        if (needsForcedRefresh)
         {
-            profile.CurrentPage(ProfileSubPage::Advanced);
+            // If we're already on the correct sub-page, the PropertyChanged event won't fire.
+            // However, we still need to pass along the ElementToFocus, so we need to force a refresh.
+            profileVMImpl->ForceRefreshCurrentPage();   
         }
     }
 

src/cascadia/TerminalSettingsEditor/ProfileViewModel.h

@@ -59,6 +59,12 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         void DeleteProfile();
 
         void SetupAppearances(Windows::Foundation::Collections::IObservableVector<Editor::ColorSchemeViewModel> schemesList);
+        void ForceRefreshCurrentPage()
+        {
+            // Used to trigger the PropertyChanged handler in MainPage.cpp
+            // This forces the page to refresh
+            _NotifyChanges(L"CurrentPage");
+        }
 
         // bell style bits
         hstring BellStylePreview() const;
@@ -177,6 +183,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
 
         WINRT_PROPERTY(bool, IsBaseLayer, false);
         WINRT_PROPERTY(bool, FocusDeleteButton, false);
+        WINRT_PROPERTY(hstring, ElementToFocus);
         WINRT_PROPERTY(Windows::Foundation::Collections::IVector<Windows::Foundation::IInspectable>, IconTypes);
         GETSET_BINDABLE_ENUM_SETTING(AntiAliasingMode, Microsoft::Terminal::Control::TextAntialiasingMode, AntialiasingMode);
         GETSET_BINDABLE_ENUM_SETTING(CloseOnExitMode, Microsoft::Terminal::Settings::Model::CloseOnExitMode, CloseOnExit);

src/cascadia/TerminalSettingsEditor/Profiles_Appearance.xaml

@@ -75,7 +75,8 @@
                         CornerRadius="{StaticResource ControlCornerRadius}" />
             </Border>
 
-            <local:Appearances Appearance="{x:Bind Profile.DefaultAppearance, Mode=OneWay}"
+            <local:Appearances x:Name="DefaultAppearanceView"
+                               Appearance="{x:Bind Profile.DefaultAppearance, Mode=OneWay}"
                                SourceProfile="{x:Bind Profile, Mode=OneWay}"
                                WindowRoot="{x:Bind WindowRoot, Mode=OneTime}" />
 
@@ -251,7 +252,8 @@
                 </StackPanel>
 
                 <!--  Unfocused Appearance  -->
-                <local:Appearances Appearance="{x:Bind Profile.UnfocusedAppearance, Mode=OneWay}"
+                <local:Appearances x:Name="UnfocusedAppearanceView"
+                                   Appearance="{x:Bind Profile.UnfocusedAppearance, Mode=OneWay}"
                                    SourceProfile="{x:Bind Profile, Mode=OneWay}"
                                    Visibility="{x:Bind Profile.ShowUnfocusedAppearance, Mode=OneWay}"
                                    WindowRoot="{x:Bind WindowRoot, Mode=OneTime}" />