From 1ecbc1884c787a245e68d43c583cfadbab2e5ac1 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 11 Apr 2023 11:43:49 -0500 Subject: [PATCH 1/8] fix not updating the nav view when add/removing profiles --- src/cascadia/TerminalSettingsEditor/MainPage.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index b1b2a834933..74e9597b488 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -163,7 +163,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // // By setting MenuItemsSource in its entirety, rather than manipulating // MenuItems, we avoid a crash in WinUI. - auto newSource = winrt::single_threaded_vector(std::move(menuItemsSTL)); + auto newSource = winrt::single_threaded_observable_vector(std::move(menuItemsSTL)); SettingsNav().MenuItemsSource(newSource); // Repopulate profile-related menu items @@ -251,7 +251,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { uint32_t insertIndex; auto selectedItem{ SettingsNav().SelectedItem() }; - auto menuItems{ SettingsNav().MenuItems() }; + auto menuItems{ SettingsNav().MenuItemsSource().try_as>() }; menuItems.IndexOf(selectedItem, insertIndex); if (profileGuid != winrt::guid{}) { @@ -602,7 +602,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation std::vector menuItemsSTL(menuItems.Size(), nullptr); menuItems.GetMany(0, menuItemsSTL); - auto newSource = winrt::single_threaded_vector(std::move(menuItemsSTL)); + auto newSource = winrt::single_threaded_observable_vector(std::move(menuItemsSTL)); SettingsNav().MenuItemsSource(newSource); } @@ -612,7 +612,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation const auto profileViewModel{ _viewModelForProfile(newProfile, _settingsClone) }; profileViewModel.SetupAppearances(_colorSchemesPageVM.AllColorSchemes()); const auto navItem{ _CreateProfileNavViewItem(profileViewModel) }; - SettingsNav().MenuItems().InsertAt(index, navItem); + + auto navItems{ SettingsNav().MenuItemsSource().try_as>() }; + navItems.InsertAt(index, navItem); + SettingsNav().MenuItemsSource(navItems); // Select and navigate to the new profile SettingsNav().SelectedItem(navItem); @@ -666,9 +669,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // remove selected item uint32_t index; auto selectedItem{ SettingsNav().SelectedItem() }; - auto menuItems{ SettingsNav().MenuItems() }; + auto menuItems{ SettingsNav().MenuItemsSource().try_as>() }; menuItems.IndexOf(selectedItem, index); menuItems.RemoveAt(index); + SettingsNav().MenuItemsSource(menuItems); // navigate to the profile next to this one const auto newSelectedItem{ menuItems.GetAt(index < menuItems.Size() - 1 ? index : index - 1) }; From a28a6aa811a69cbb96996ecb62f778aeb8637521 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 11 Apr 2023 14:27:17 -0500 Subject: [PATCH 2/8] add guards to menuitemsource.try_as --- .../TerminalSettingsEditor/MainPage.cpp | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index 74e9597b488..552e601dc4e 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -251,8 +251,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { uint32_t insertIndex; auto selectedItem{ SettingsNav().SelectedItem() }; - auto menuItems{ SettingsNav().MenuItemsSource().try_as>() }; - menuItems.IndexOf(selectedItem, insertIndex); + if (const auto& menuItems{ SettingsNav().MenuItemsSource().try_as>() }) + { + menuItems.IndexOf(selectedItem, insertIndex); + } if (profileGuid != winrt::guid{}) { // if we were given a non-empty guid, we want to duplicate the corresponding profile @@ -613,9 +615,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation profileViewModel.SetupAppearances(_colorSchemesPageVM.AllColorSchemes()); const auto navItem{ _CreateProfileNavViewItem(profileViewModel) }; - auto navItems{ SettingsNav().MenuItemsSource().try_as>() }; - navItems.InsertAt(index, navItem); - SettingsNav().MenuItemsSource(navItems); + if (const auto& navItems{ SettingsNav().MenuItemsSource().try_as>() }) + { + navItems.InsertAt(index, navItem); + } // Select and navigate to the new profile SettingsNav().SelectedItem(navItem); @@ -669,23 +672,24 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // remove selected item uint32_t index; auto selectedItem{ SettingsNav().SelectedItem() }; - auto menuItems{ SettingsNav().MenuItemsSource().try_as>() }; - menuItems.IndexOf(selectedItem, index); - menuItems.RemoveAt(index); - SettingsNav().MenuItemsSource(menuItems); - - // navigate to the profile next to this one - const auto newSelectedItem{ menuItems.GetAt(index < menuItems.Size() - 1 ? index : index - 1) }; - SettingsNav().SelectedItem(newSelectedItem); - const auto newTag = newSelectedItem.as().Tag(); - if (const auto profileViewModel = newTag.try_as()) - { - profileViewModel->FocusDeleteButton(true); - _Navigate(*profileViewModel, BreadcrumbSubPage::None); - } - else + if (const auto& menuItems{ SettingsNav().MenuItemsSource().try_as>() }) { - _Navigate(newTag.as(), BreadcrumbSubPage::None); + menuItems.IndexOf(selectedItem, index); + menuItems.RemoveAt(index); + + // navigate to the profile next to this one + const auto newSelectedItem{ menuItems.GetAt(index < menuItems.Size() - 1 ? index : index - 1) }; + SettingsNav().SelectedItem(newSelectedItem); + const auto newTag = newSelectedItem.as().Tag(); + if (const auto profileViewModel = newTag.try_as()) + { + profileViewModel->FocusDeleteButton(true); + _Navigate(*profileViewModel, BreadcrumbSubPage::None); + } + else + { + _Navigate(newTag.as(), BreadcrumbSubPage::None); + } } } From 66520ddeca910b759ba07b1be442fa132c4ac627 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 13 Apr 2023 06:16:36 -0500 Subject: [PATCH 3/8] WHAT THE HECK IS GOING ON --- .../TerminalSettingsEditor/MainPage.cpp | 133 ++++++++++++------ .../TerminalSettingsEditor/MainPage.h | 2 + 2 files changed, 91 insertions(+), 44 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index 552e601dc4e..62c2b785f65 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -117,45 +117,45 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation lastBreadcrumb = _breadcrumbs.GetAt(size - 1); } - // Collect all the values out of the old nav view item source - auto menuItems{ SettingsNav().MenuItems() }; - - // We'll remove a bunch of items and iterate over it twice. - // --> Copy it into an STL vector to simplify our code and reduce COM overhead. - std::vector menuItemsSTL(menuItems.Size(), nullptr); - menuItems.GetMany(0, menuItemsSTL); - - // We want to refresh the list of profiles in the NavigationView. - // In order to add profiles we can use _InitializeProfilesList(); - // But before we can do that we have to remove existing profiles first of course. - // This "erase-remove" idiom will achieve just that. - menuItemsSTL.erase( - std::remove_if( - menuItemsSTL.begin(), - menuItemsSTL.end(), - [](const auto& item) -> bool { - if (const auto& navViewItem{ item.try_as() }) - { - if (const auto& tag{ navViewItem.Tag() }) - { - if (tag.try_as()) - { - // remove NavViewItem pointing to a Profile - return true; - } - if (const auto& stringTag{ tag.try_as() }) - { - if (stringTag == addProfileTag) - { - // remove the "Add Profile" item - return true; - } - } - } - } - return false; - }), - menuItemsSTL.end()); + // // Collect all the values out of the old nav view item source + // auto menuItems{ SettingsNav().MenuItemsSource().try_as>() }; + + // // We'll remove a bunch of items and iterate over it twice. + // // --> Copy it into an STL vector to simplify our code and reduce COM overhead. + // std::vector menuItemsSTL(menuItems.Size(), nullptr); + // menuItems.GetMany(0, menuItemsSTL); + + // // We want to refresh the list of profiles in the NavigationView. + // // In order to add profiles we can use _InitializeProfilesList(); + // // But before we can do that we have to remove existing profiles first of course. + // // This "erase-remove" idiom will achieve just that. + // menuItemsSTL.erase( + // std::remove_if( + // menuItemsSTL.begin(), + // menuItemsSTL.end(), + // [](const auto& item) -> bool { + // if (const auto& navViewItem{ item.try_as() }) + // { + // if (const auto& tag{ navViewItem.Tag() }) + // { + // if (tag.try_as()) + // { + // // remove NavViewItem pointing to a Profile + // return true; + // } + // if (const auto& stringTag{ tag.try_as() }) + // { + // if (stringTag == addProfileTag) + // { + // // remove the "Add Profile" item + // return true; + // } + // } + // } + // } + // return false; + // }), + // menuItemsSTL.end()); // Now, we've got a list of just the static entries again. Lets take // those and stick them back into a new winrt vector, and set that as @@ -163,7 +163,32 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // // By setting MenuItemsSource in its entirety, rather than manipulating // MenuItems, we avoid a crash in WinUI. - auto newSource = winrt::single_threaded_observable_vector(std::move(menuItemsSTL)); + // auto newSource = winrt::single_threaded_observable_vector(std::move(menuItemsSTL)); + auto newSource = winrt::single_threaded_observable_vector(); + for (const auto& item : _originalMenuItems) + { + if (const auto& menuItem{ item.try_as() }) + { + if (const auto& tag{ menuItem.Tag() }) + { + if (const auto& breadcrumbStringTag{ tag.try_as() }) + { + breadcrumbStringTag; + auto a = 9; + a++; + a; + } + else + { + auto a = 9; + a++; + a; + + } + } + } + newSource.Append(item); + } SettingsNav().MenuItemsSource(newSource); // Repopulate profile-related menu items @@ -175,6 +200,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // now that the menuItems are repopulated, // refresh the current page using the breadcrumb data we collected before the refresh + auto menuItems{ SettingsNav().MenuItemsSource().try_as>() }; if (const auto& crumb{ lastBreadcrumb.try_as() }) { for (const auto& item : menuItems) @@ -217,7 +243,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // Couldn't find the selected item, fallback to first menu item // This happens when the selected item was a profile which doesn't exist in the new configuration // We can use menuItemsSTL here because the only things they miss are profile entries. - const auto& firstItem{ SettingsNav().MenuItems().GetAt(0).as() }; + const auto& firstItem{ menuItems.GetAt(0).as() }; SettingsNav().SelectedItem(firstItem); _Navigate(unbox_value(firstItem.Tag()), BreadcrumbSubPage::None); } @@ -549,18 +575,29 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation } const auto menuItems = SettingsNav().MenuItemsSource().try_as>(); + MUX::Controls::NavigationViewItem testItem; + testItem.Content(box_value(L"testItem")); + testItem.Tag(box_value(L"testItem")); + menuItems.Append(testItem); + // Manually create a NavigationViewItem for each profile // and keep a reference to them in a map so that we // can easily modify the correct one when the associated // profile changes. for (const auto& profile : _settingsClone.AllProfiles()) { + auto sizeBefore = menuItems.Size(); if (!profile.Deleted()) { auto profileVM = _viewModelForProfile(profile, _settingsClone); profileVM.SetupAppearances(_colorSchemesPageVM.AllColorSchemes()); auto navItem = _CreateProfileNavViewItem(profileVM); menuItems.Append(navItem); + auto sizeAfter = menuItems.Size(); + sizeBefore; + sizeAfter; + const bool wasSame = sizeBefore == sizeAfter; + wasSame; } } @@ -594,6 +631,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation } auto menuItems{ SettingsNav().MenuItems() }; + const auto originalNumItems = menuItems.Size(); // Remove all the existing items, and move them to a separate vector // that we'll use as a MenuItemsSource. By doing this, we avoid a WinUI // bug (MUX#6302) where modifying the NavView.Items() directly causes a @@ -601,10 +639,17 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // benefit of instantiating them from the XBF, rather than at runtime. // // --> Copy it into an STL vector to simplify our code and reduce COM overhead. - std::vector menuItemsSTL(menuItems.Size(), nullptr); - menuItems.GetMany(0, menuItemsSTL); + // std::vector menuItemsSTL(menuItems.Size(), nullptr); + // _originalMenuItems.reserve(menuItems.Size()); + _originalMenuItems = std::vector{ originalNumItems, nullptr }; - auto newSource = winrt::single_threaded_observable_vector(std::move(menuItemsSTL)); + menuItems.GetMany(0, _originalMenuItems); + + auto newSource = winrt::single_threaded_observable_vector(); + for (const auto& item : _originalMenuItems) + { + newSource.Append(item); + } SettingsNav().MenuItemsSource(newSource); } diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.h b/src/cascadia/TerminalSettingsEditor/MainPage.h index bdcfa5b974d..73253963488 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.h +++ b/src/cascadia/TerminalSettingsEditor/MainPage.h @@ -50,6 +50,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation private: Windows::Foundation::Collections::IObservableVector _breadcrumbs; + std::vector _originalMenuItems; + Model::CascadiaSettings _settingsSource; Model::CascadiaSettings _settingsClone; From 1720e854b3d35ebfff0727f9ed81351b4b899313 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 13 Apr 2023 08:12:47 -0500 Subject: [PATCH 4/8] yep this works --- .../TerminalSettingsEditor/MainPage.cpp | 95 ++++++++++--------- .../TerminalSettingsEditor/MainPage.h | 3 +- 2 files changed, 54 insertions(+), 44 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index 62c2b785f65..1a9190d396e 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -164,32 +164,41 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // By setting MenuItemsSource in its entirety, rather than manipulating // MenuItems, we avoid a crash in WinUI. // auto newSource = winrt::single_threaded_observable_vector(std::move(menuItemsSTL)); - auto newSource = winrt::single_threaded_observable_vector(); - for (const auto& item : _originalMenuItems) - { - if (const auto& menuItem{ item.try_as() }) - { - if (const auto& tag{ menuItem.Tag() }) - { - if (const auto& breadcrumbStringTag{ tag.try_as() }) - { - breadcrumbStringTag; - auto a = 9; - a++; - a; - } - else - { - auto a = 9; - a++; - a; - } - } - } - newSource.Append(item); - } - SettingsNav().MenuItemsSource(newSource); + std::vector menuItemsSTL(_originalNumItems, nullptr); + _originalMenuItems.GetMany(0, menuItemsSTL); + _originalMenuItems.Clear(); + // for (auto&& i : menuItemsSTL) + // { + // _originalMenuItems.Append(i); + // } + _originalMenuItems.ReplaceAll(menuItemsSTL); + // auto newSource = winrt::single_threaded_observable_vector(); + // for (const auto& item : _originalMenuItems) + // { + // if (const auto& menuItem{ item.try_as() }) + // { + // if (const auto& tag{ menuItem.Tag() }) + // { + // if (const auto& breadcrumbStringTag{ tag.try_as() }) + // { + // breadcrumbStringTag; + // auto a = 9; + // a++; + // a; + // } + // else + // { + // auto a = 9; + // a++; + // a; + + // } + // } + // } + // newSource.Append(item); + // } + // SettingsNav().MenuItemsSource(newSource); // Repopulate profile-related menu items _InitializeProfilesList(); @@ -200,7 +209,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // now that the menuItems are repopulated, // refresh the current page using the breadcrumb data we collected before the refresh - auto menuItems{ SettingsNav().MenuItemsSource().try_as>() }; + auto menuItems{ _originalMenuItems }; if (const auto& crumb{ lastBreadcrumb.try_as() }) { for (const auto& item : menuItems) @@ -277,9 +286,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { uint32_t insertIndex; auto selectedItem{ SettingsNav().SelectedItem() }; - if (const auto& menuItems{ SettingsNav().MenuItemsSource().try_as>() }) + if (_originalMenuItems) { - menuItems.IndexOf(selectedItem, insertIndex); + _originalMenuItems.IndexOf(selectedItem, insertIndex); } if (profileGuid != winrt::guid{}) { @@ -573,7 +582,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation _MoveXamlParsedNavItemsIntoItemSource(); } - const auto menuItems = SettingsNav().MenuItemsSource().try_as>(); + const auto menuItems = _originalMenuItems; MUX::Controls::NavigationViewItem testItem; testItem.Content(box_value(L"testItem")); @@ -631,7 +640,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation } auto menuItems{ SettingsNav().MenuItems() }; - const auto originalNumItems = menuItems.Size(); + _originalNumItems = menuItems.Size(); // Remove all the existing items, and move them to a separate vector // that we'll use as a MenuItemsSource. By doing this, we avoid a WinUI // bug (MUX#6302) where modifying the NavView.Items() directly causes a @@ -641,16 +650,16 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // --> Copy it into an STL vector to simplify our code and reduce COM overhead. // std::vector menuItemsSTL(menuItems.Size(), nullptr); // _originalMenuItems.reserve(menuItems.Size()); - _originalMenuItems = std::vector{ originalNumItems, nullptr }; + auto og = std::vector{ _originalNumItems, nullptr }; - menuItems.GetMany(0, _originalMenuItems); + menuItems.GetMany(0, og); - auto newSource = winrt::single_threaded_observable_vector(); - for (const auto& item : _originalMenuItems) - { - newSource.Append(item); - } - SettingsNav().MenuItemsSource(newSource); + _originalMenuItems = winrt::single_threaded_observable_vector(std::move(og)); + // for (const auto& item : _originalMenuItems) + // { + // newSource.Append(item); + // } + SettingsNav().MenuItemsSource(_originalMenuItems); } void MainPage::_CreateAndNavigateToNewProfile(const uint32_t index, const Model::Profile& profile) @@ -660,7 +669,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation profileViewModel.SetupAppearances(_colorSchemesPageVM.AllColorSchemes()); const auto navItem{ _CreateProfileNavViewItem(profileViewModel) }; - if (const auto& navItems{ SettingsNav().MenuItemsSource().try_as>() }) + if (const auto& navItems{ _originalMenuItems }) { navItems.InsertAt(index, navItem); } @@ -717,13 +726,13 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // remove selected item uint32_t index; auto selectedItem{ SettingsNav().SelectedItem() }; - if (const auto& menuItems{ SettingsNav().MenuItemsSource().try_as>() }) + if (_originalMenuItems) { - menuItems.IndexOf(selectedItem, index); - menuItems.RemoveAt(index); + _originalMenuItems.IndexOf(selectedItem, index); + _originalMenuItems.RemoveAt(index); // navigate to the profile next to this one - const auto newSelectedItem{ menuItems.GetAt(index < menuItems.Size() - 1 ? index : index - 1) }; + const auto newSelectedItem{ _originalMenuItems.GetAt(index < _originalMenuItems.Size() - 1 ? index : index - 1) }; SettingsNav().SelectedItem(newSelectedItem); const auto newTag = newSelectedItem.as().Tag(); if (const auto profileViewModel = newTag.try_as()) diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.h b/src/cascadia/TerminalSettingsEditor/MainPage.h index 73253963488..debcccb2ea4 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.h +++ b/src/cascadia/TerminalSettingsEditor/MainPage.h @@ -50,7 +50,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation private: Windows::Foundation::Collections::IObservableVector _breadcrumbs; - std::vector _originalMenuItems; + Windows::Foundation::Collections::IObservableVector _originalMenuItems; + size_t _originalNumItems = 0u; Model::CascadiaSettings _settingsSource; Model::CascadiaSettings _settingsClone; From e8c02249215a9f57a15496350a4d7cfbb1b2615d Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 13 Apr 2023 09:45:43 -0500 Subject: [PATCH 5/8] cleanup --- .../TerminalSettingsEditor/MainPage.cpp | 138 ++++-------------- .../TerminalSettingsEditor/MainPage.h | 2 +- 2 files changed, 28 insertions(+), 112 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index 1a9190d396e..84f422b33b6 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -117,88 +117,19 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation lastBreadcrumb = _breadcrumbs.GetAt(size - 1); } - // // Collect all the values out of the old nav view item source - // auto menuItems{ SettingsNav().MenuItemsSource().try_as>() }; - - // // We'll remove a bunch of items and iterate over it twice. - // // --> Copy it into an STL vector to simplify our code and reduce COM overhead. - // std::vector menuItemsSTL(menuItems.Size(), nullptr); - // menuItems.GetMany(0, menuItemsSTL); - - // // We want to refresh the list of profiles in the NavigationView. - // // In order to add profiles we can use _InitializeProfilesList(); - // // But before we can do that we have to remove existing profiles first of course. - // // This "erase-remove" idiom will achieve just that. - // menuItemsSTL.erase( - // std::remove_if( - // menuItemsSTL.begin(), - // menuItemsSTL.end(), - // [](const auto& item) -> bool { - // if (const auto& navViewItem{ item.try_as() }) - // { - // if (const auto& tag{ navViewItem.Tag() }) - // { - // if (tag.try_as()) - // { - // // remove NavViewItem pointing to a Profile - // return true; - // } - // if (const auto& stringTag{ tag.try_as() }) - // { - // if (stringTag == addProfileTag) - // { - // // remove the "Add Profile" item - // return true; - // } - // } - // } - // } - // return false; - // }), - // menuItemsSTL.end()); - - // Now, we've got a list of just the static entries again. Lets take - // those and stick them back into a new winrt vector, and set that as - // the source again. + // Collect only the first items out of the menu item source, the static + // ones that we don't want to regenerate. // - // By setting MenuItemsSource in its entirety, rather than manipulating - // MenuItems, we avoid a crash in WinUI. - // auto newSource = winrt::single_threaded_observable_vector(std::move(menuItemsSTL)); - + // By manipulating a MenuItemsSource this way, rather than manipulating the + // MenuItems directly, we avoid a crash in WinUI. + // + // By making the vector only _originalNumItems big to start, GetMany + // will only fill that number of elements out of the current source. std::vector menuItemsSTL(_originalNumItems, nullptr); - _originalMenuItems.GetMany(0, menuItemsSTL); - _originalMenuItems.Clear(); - // for (auto&& i : menuItemsSTL) - // { - // _originalMenuItems.Append(i); - // } - _originalMenuItems.ReplaceAll(menuItemsSTL); - // auto newSource = winrt::single_threaded_observable_vector(); - // for (const auto& item : _originalMenuItems) - // { - // if (const auto& menuItem{ item.try_as() }) - // { - // if (const auto& tag{ menuItem.Tag() }) - // { - // if (const auto& breadcrumbStringTag{ tag.try_as() }) - // { - // breadcrumbStringTag; - // auto a = 9; - // a++; - // a; - // } - // else - // { - // auto a = 9; - // a++; - // a; - - // } - // } - // } - // newSource.Append(item); - // } - // SettingsNav().MenuItemsSource(newSource); + _menuItemSource.GetMany(0, menuItemsSTL); + _menuItemSource.Clear(); + // now, just stick them back in. + _menuItemSource.ReplaceAll(menuItemsSTL); // Repopulate profile-related menu items _InitializeProfilesList(); @@ -209,7 +140,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // now that the menuItems are repopulated, // refresh the current page using the breadcrumb data we collected before the refresh - auto menuItems{ _originalMenuItems }; + auto menuItems{ _menuItemSource }; if (const auto& crumb{ lastBreadcrumb.try_as() }) { for (const auto& item : menuItems) @@ -286,9 +217,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { uint32_t insertIndex; auto selectedItem{ SettingsNav().SelectedItem() }; - if (_originalMenuItems) + if (_menuItemSource) { - _originalMenuItems.IndexOf(selectedItem, insertIndex); + _menuItemSource.IndexOf(selectedItem, insertIndex); } if (profileGuid != winrt::guid{}) { @@ -582,12 +513,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation _MoveXamlParsedNavItemsIntoItemSource(); } - const auto menuItems = _originalMenuItems; - - MUX::Controls::NavigationViewItem testItem; - testItem.Content(box_value(L"testItem")); - testItem.Tag(box_value(L"testItem")); - menuItems.Append(testItem); // Manually create a NavigationViewItem for each profile // and keep a reference to them in a map so that we @@ -595,18 +520,12 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // profile changes. for (const auto& profile : _settingsClone.AllProfiles()) { - auto sizeBefore = menuItems.Size(); if (!profile.Deleted()) { auto profileVM = _viewModelForProfile(profile, _settingsClone); profileVM.SetupAppearances(_colorSchemesPageVM.AllColorSchemes()); auto navItem = _CreateProfileNavViewItem(profileVM); - menuItems.Append(navItem); - auto sizeAfter = menuItems.Size(); - sizeBefore; - sizeAfter; - const bool wasSame = sizeBefore == sizeAfter; - wasSame; + _menuItemSource.Append(navItem); } } @@ -620,7 +539,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation icon.Glyph(L"\xE710"); addProfileItem.Icon(icon); - menuItems.Append(addProfileItem); + _menuItemSource.Append(addProfileItem); } // BODGY @@ -649,17 +568,14 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // // --> Copy it into an STL vector to simplify our code and reduce COM overhead. // std::vector menuItemsSTL(menuItems.Size(), nullptr); - // _originalMenuItems.reserve(menuItems.Size()); - auto og = std::vector{ _originalNumItems, nullptr }; + // _menuItemSource.reserve(menuItems.Size()); + auto original = std::vector{ _originalNumItems, nullptr }; + + menuItems.GetMany(0, original); - menuItems.GetMany(0, og); + _menuItemSource = winrt::single_threaded_observable_vector(std::move(original)); - _originalMenuItems = winrt::single_threaded_observable_vector(std::move(og)); - // for (const auto& item : _originalMenuItems) - // { - // newSource.Append(item); - // } - SettingsNav().MenuItemsSource(_originalMenuItems); + SettingsNav().MenuItemsSource(_menuItemSource); } void MainPage::_CreateAndNavigateToNewProfile(const uint32_t index, const Model::Profile& profile) @@ -669,7 +585,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation profileViewModel.SetupAppearances(_colorSchemesPageVM.AllColorSchemes()); const auto navItem{ _CreateProfileNavViewItem(profileViewModel) }; - if (const auto& navItems{ _originalMenuItems }) + if (const auto& navItems{ _menuItemSource }) { navItems.InsertAt(index, navItem); } @@ -726,13 +642,13 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // remove selected item uint32_t index; auto selectedItem{ SettingsNav().SelectedItem() }; - if (_originalMenuItems) + if (_menuItemSource) { - _originalMenuItems.IndexOf(selectedItem, index); - _originalMenuItems.RemoveAt(index); + _menuItemSource.IndexOf(selectedItem, index); + _menuItemSource.RemoveAt(index); // navigate to the profile next to this one - const auto newSelectedItem{ _originalMenuItems.GetAt(index < _originalMenuItems.Size() - 1 ? index : index - 1) }; + const auto newSelectedItem{ _menuItemSource.GetAt(index < _menuItemSource.Size() - 1 ? index : index - 1) }; SettingsNav().SelectedItem(newSelectedItem); const auto newTag = newSelectedItem.as().Tag(); if (const auto profileViewModel = newTag.try_as()) diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.h b/src/cascadia/TerminalSettingsEditor/MainPage.h index debcccb2ea4..90719eb09ad 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.h +++ b/src/cascadia/TerminalSettingsEditor/MainPage.h @@ -50,7 +50,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation private: Windows::Foundation::Collections::IObservableVector _breadcrumbs; - Windows::Foundation::Collections::IObservableVector _originalMenuItems; + Windows::Foundation::Collections::IObservableVector _menuItemSource; size_t _originalNumItems = 0u; Model::CascadiaSettings _settingsSource; From ab4ba7ca21e9e6aec59654b9c44557616a7abd85 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 13 Apr 2023 09:47:36 -0500 Subject: [PATCH 6/8] a bit of dead code --- src/cascadia/TerminalSettingsEditor/MainPage.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index 84f422b33b6..aba6b19146f 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -567,10 +567,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // benefit of instantiating them from the XBF, rather than at runtime. // // --> Copy it into an STL vector to simplify our code and reduce COM overhead. - // std::vector menuItemsSTL(menuItems.Size(), nullptr); - // _menuItemSource.reserve(menuItems.Size()); auto original = std::vector{ _originalNumItems, nullptr }; - menuItems.GetMany(0, original); _menuItemSource = winrt::single_threaded_observable_vector(std::move(original)); From 249ccb0b69db39734298a20ab8b370a94099a9cc Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 17 Apr 2023 09:13:01 -0500 Subject: [PATCH 7/8] cleanup --- src/cascadia/TerminalSettingsEditor/MainPage.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index aba6b19146f..7fe187c2094 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -140,10 +140,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // now that the menuItems are repopulated, // refresh the current page using the breadcrumb data we collected before the refresh - auto menuItems{ _menuItemSource }; if (const auto& crumb{ lastBreadcrumb.try_as() }) { - for (const auto& item : menuItems) + for (const auto& item : _menuItemSource) { if (const auto& menuItem{ item.try_as() }) { @@ -183,7 +182,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // Couldn't find the selected item, fallback to first menu item // This happens when the selected item was a profile which doesn't exist in the new configuration // We can use menuItemsSTL here because the only things they miss are profile entries. - const auto& firstItem{ menuItems.GetAt(0).as() }; + const auto& firstItem{ _menuItemSource.GetAt(0).as() }; SettingsNav().SelectedItem(firstItem); _Navigate(unbox_value(firstItem.Tag()), BreadcrumbSubPage::None); } @@ -582,9 +581,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation profileViewModel.SetupAppearances(_colorSchemesPageVM.AllColorSchemes()); const auto navItem{ _CreateProfileNavViewItem(profileViewModel) }; - if (const auto& navItems{ _menuItemSource }) + if (_menuItemSource) { - navItems.InsertAt(index, navItem); + _menuItemSource.InsertAt(index, navItem); } // Select and navigate to the new profile From 31b8e9d7db2e074d23108813f3c6d7d2965a363b Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 17 Apr 2023 09:21:05 -0500 Subject: [PATCH 8/8] correct, this was redundant --- src/cascadia/TerminalSettingsEditor/MainPage.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index 7fe187c2094..2c18ce11c95 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -127,7 +127,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // will only fill that number of elements out of the current source. std::vector menuItemsSTL(_originalNumItems, nullptr); _menuItemSource.GetMany(0, menuItemsSTL); - _menuItemSource.Clear(); // now, just stick them back in. _menuItemSource.ReplaceAll(menuItemsSTL);