color schemes: fix BringIntoView

Author: carlos-zamora

src/cascadia/TerminalSettingsEditor/ColorSchemesPageViewModel.h

@@ -37,6 +37,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
 
         WINRT_OBSERVABLE_PROPERTY(ColorSchemesSubPage, CurrentPage, _propertyChangedHandlers, ColorSchemesSubPage::Base);
         WINRT_OBSERVABLE_PROPERTY(Windows::Foundation::Collections::IObservableVector<Editor::ColorSchemeViewModel>, AllColorSchemes, _propertyChangedHandlers, nullptr);
+        WINRT_PROPERTY(hstring, ElementToFocus);
 
     private:
         Editor::ColorSchemeViewModel _CurrentScheme{ nullptr };

src/cascadia/TerminalSettingsEditor/EditColorScheme.cpp

@@ -40,7 +40,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
 
     void EditColorScheme::OnNavigatedTo(const NavigationEventArgs& e)
     {
-        _ViewModel = e.Parameter().as<Editor::ColorSchemeViewModel>();
+        const auto args = e.Parameter().as<Editor::NavigateToEditColorSchemeArgs>();
+        _ViewModel = args.ViewModel();
+
+        BringIntoViewWhenLoaded(args.ElementToFocus());
 
         const auto schemeName = _ViewModel.Name();
         NameBox().Text(schemeName);

src/cascadia/TerminalSettingsEditor/EditColorScheme.h

@@ -3,12 +3,27 @@
 
 #pragma once
 
+#include "NavigateToEditColorSchemeArgs.g.h"
 #include "EditColorScheme.g.h"
 #include "ColorSchemeViewModel.h"
 #include "Utils.h"
 
 namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
 {
+    struct NavigateToEditColorSchemeArgs : NavigateToEditColorSchemeArgsT<NavigateToEditColorSchemeArgs>
+    {
+        NavigateToEditColorSchemeArgs(const Editor::ColorSchemeViewModel& vm, const hstring& elementToFocus = {}) :
+            _ViewModel(vm),
+            _ElementToFocus(elementToFocus) {}
+
+        Editor::ColorSchemeViewModel ViewModel() const noexcept { return _ViewModel; }
+        hstring ElementToFocus() const noexcept { return _ElementToFocus; }
+
+    private:
+        Editor::ColorSchemeViewModel _ViewModel{ nullptr };
+        hstring _ElementToFocus{};
+    };
+
     struct EditColorScheme : public HasScrollViewer<EditColorScheme>, EditColorSchemeT<EditColorScheme>
     {
         EditColorScheme();

src/cascadia/TerminalSettingsEditor/EditColorScheme.idl

@@ -5,6 +5,12 @@ import "ColorSchemeViewModel.idl";
 
 namespace Microsoft.Terminal.Settings.Editor
 {
+    runtimeclass NavigateToEditColorSchemeArgs
+    {
+        ColorSchemeViewModel ViewModel { get; };
+        String ElementToFocus { get; };
+    }
+
     [default_interface] runtimeclass EditColorScheme : Windows.UI.Xaml.Controls.Page, Windows.UI.Xaml.Data.INotifyPropertyChanged
     {
         EditColorScheme();

src/cascadia/TerminalSettingsEditor/MainPage.cpp

@@ -15,6 +15,7 @@
 #include "GlobalAppearance.h"
 #include "GlobalAppearanceViewModel.h"
 #include "ColorSchemes.h"
+#include "EditColorScheme.h"
 #include "AddProfile.h"
 #include "InteractionViewModel.h"
 #include "LaunchViewModel.h"
@@ -293,16 +294,21 @@ 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<ColorSchemesPageViewModel>(_colorSchemesPageVM);
+                const auto elementToFocus = vmImpl->ElementToFocus();
+                vmImpl->ElementToFocus(L"");
+
                 const auto currentScheme = _colorSchemesPageVM.CurrentScheme();
                 if (_colorSchemesPageVM.CurrentPage() == ColorSchemesSubPage::EditColorScheme && currentScheme)
                 {
-                    contentFrame().Navigate(xaml_typename<Editor::EditColorScheme>(), currentScheme);
+                    contentFrame().Navigate(xaml_typename<Editor::EditColorScheme>(), winrt::make<NavigateToEditColorSchemeArgs>(currentScheme, elementToFocus));
                     const auto crumb = winrt::make<Breadcrumb>(box_value(colorSchemesTag), currentScheme.Name(), BreadcrumbSubPage::ColorSchemes_Edit);
                     _breadcrumbs.Append(crumb);
                 }
                 else if (_colorSchemesPageVM.CurrentPage() == ColorSchemesSubPage::Base)
                 {
-                    _Navigate(winrt::hstring{ colorSchemesTag }, BreadcrumbSubPage::None);
+                    _Navigate(winrt::hstring{ colorSchemesTag }, BreadcrumbSubPage::None, elementToFocus);
                 }
             }
             else if (settingName == L"CurrentSchemeName")
@@ -862,16 +868,17 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
     {
         _PreNavigateHelper();
 
-        // TODO CARLOS:
-        // - should navigate to EditColorScheme, not ColorSchemes
-        // - EditColorScheme::OnNavigatedTo needs to accept NavigateToColorSchemesArgs (or similar)
-        // - EditColorScheme::OnNavigatedTo needs BringIntoViewWhenLoaded(args.ElementToFocus())
         const auto crumb = winrt::make<Breadcrumb>(box_value(colorSchemesTag), RS_(L"Nav_ColorSchemes/Content"), BreadcrumbSubPage::None);
         _breadcrumbs.Append(crumb);
         contentFrame().Navigate(xaml_typename<Editor::ColorSchemes>(), winrt::make<NavigateToColorSchemesArgs>(_colorSchemesPageVM, elementToFocus));
         SettingsNav().SelectedItem(ColorSchemesNavItem());
 
+        // Pass along the element to focus to the ColorSchemesPageViewModel.
+        // This will work as a staging area before we navigate to EditColorScheme
+        get_self<ColorSchemesPageViewModel>(_colorSchemesPageVM)->ElementToFocus(elementToFocus);
+
         // Set CurrentScheme BEFORE the CurrentPage!
+        // Doing so triggers the PropertyChanged event which performs the navigation to EditColorScheme
         if (subPage == BreadcrumbSubPage::None)
         {
             _colorSchemesPageVM.CurrentScheme(nullptr);

src/cascadia/TerminalSettingsEditor/Utils.h

@@ -131,15 +131,15 @@ struct HasScrollViewer
         _loadedRevoker = pThis->Loaded(winrt::auto_revoke, [weakThis{ pThis->get_weak() }, elementName](auto&&, auto&&) {
             if (auto page{ weakThis.get() })
             {
-                if (const auto& elementToFocus{ page->FindName(elementName).try_as<winrt::Microsoft::Terminal::Settings::Editor::SettingContainer>() })
+                if (const auto& controlToFocus{ page->FindName(elementName).try_as<winrt::Windows::UI::Xaml::Controls::Control>() })
                 {
                     // We need to wait for the page to be loaded
                     // or else the call to StartBringIntoView()
                     // will end up doing nothing
-                    elementToFocus.StartBringIntoView();
+                    controlToFocus.StartBringIntoView();
 
                     // TODO CARLOS: ensure this works in all scenarios (easiest to test when navigating to page by keyboard)
-                    elementToFocus.Focus(winrt::Windows::UI::Xaml::FocusState::Programmatic);
+                    controlToFocus.Focus(winrt::Windows::UI::Xaml::FocusState::Programmatic);
                 }
                 page->_loadedRevoker.revoke();
             }