From e46ed40bd9286e168c3ec57473d55e5d357712ef Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 20 Nov 2024 13:24:38 -0800 Subject: [PATCH] Fix revert button for Icon --- .../TerminalSettingsEditor/MainPage.cpp | 4 +- .../ProfileViewModel.cpp | 61 +++++++++++-------- .../TerminalSettingsEditor/Profiles_Base.xaml | 33 +++++----- .../TerminalSettingsModel/Profile.cpp | 55 +++++++++++++++++ src/cascadia/TerminalSettingsModel/Profile.h | 8 ++- 5 files changed, 115 insertions(+), 46 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index 696579ae950..62d7614cd62 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -634,9 +634,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation if (auto menuItem{ weakMenuItem.get() }) { const auto& tag{ menuItem.Tag().as() }; - if (args.PropertyName() == L"Icon") + if (args.PropertyName() == L"Icon" || args.PropertyName() == L"EvaluatedIcon") { - menuItem.Icon(UI::IconPathConverter::IconWUX(tag.Icon())); + menuItem.Icon(UI::IconPathConverter::IconWUX(tag.EvaluatedIcon())); } else if (args.PropertyName() == L"Name") { diff --git a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp index 10dbaa951c9..44d1431baaa 100644 --- a/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp +++ b/src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp @@ -43,12 +43,15 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation INITIALIZE_BINDABLE_ENUM_SETTING(ScrollState, ScrollbarState, winrt::Microsoft::Terminal::Control::ScrollbarState, L"Profile_ScrollbarVisibility", L"Content"); // set up IconTypes - _IconTypes = winrt::single_threaded_vector(); - _IconTypes.Append(make(RS_(L"Profile_IconTypeNone"), box_value(IconType::None))); - _IconTypes.Append(make(RS_(L"Profile_IconTypeFontIcon"), box_value(IconType::FontIcon))); - _IconTypes.Append(make(RS_(L"Profile_IconTypeEmoji"), box_value(IconType::Emoji))); - _IconTypes.Append(make(RS_(L"Profile_IconTypeImage"), box_value(IconType::Image))); + std::vector iconTypes; + iconTypes.reserve(4); + iconTypes.emplace_back(make(RS_(L"Profile_IconTypeNone"), box_value(IconType::None))); + iconTypes.emplace_back(make(RS_(L"Profile_IconTypeFontIcon"), box_value(IconType::FontIcon))); + iconTypes.emplace_back(make(RS_(L"Profile_IconTypeEmoji"), box_value(IconType::Emoji))); + iconTypes.emplace_back(make(RS_(L"Profile_IconTypeImage"), box_value(IconType::Image))); + _IconTypes = winrt::single_threaded_vector(std::move(iconTypes)); _DeduceCurrentIconType(); + _DeduceCurrentBuiltInIcon(); // Add a property changed handler to our own property changed event. // This propagates changes from the settings model to anybody listening to our @@ -84,12 +87,21 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation } else if (viewModelProperty == L"Icon") { + // _DeduceCurrentIconType() ends with a "CurrentIconType" notification + // so we don't need to call _UpdateIconPreview() here _DeduceCurrentIconType(); - _NotifyChanges(L"LocalizedIcon", L"EvaluatedIcon", L"IconPreview"); } else if (viewModelProperty == L"CurrentIconType") { - _NotifyChanges(L"UsingNoIcon", L"UsingBuiltInIcon", L"UsingEmojiIcon", L"UsingImageIcon"); + // "Using*" handles the visibility of the IconType-related UI. + // The others propagate the rendered icon into a preview (i.e. nav view, container item) + _NotifyChanges(L"UsingNoIcon", + L"UsingBuiltInIcon", + L"UsingEmojiIcon", + L"UsingImageIcon", + L"LocalizedIcon", + L"IconPreview", + L"EvaluatedIcon"); } else if (viewModelProperty == L"CurrentBuiltInIcon") { @@ -113,12 +125,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation UpdateFontList(); } - if (!_BuiltInIcons) - { - _UpdateBuiltInIcons(); - } - _DeduceCurrentBuiltInIcon(); - if (profile.HasUnfocusedAppearance()) { _unfocusedAppearanceViewModel = winrt::make(profile.UnfocusedAppearance().try_as()); @@ -129,11 +135,12 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void ProfileViewModel::_UpdateBuiltInIcons() { - _BuiltInIcons = single_threaded_vector(); + std::vector builtInIcons; for (auto& [val, name] : s_SegoeFluentIcons) { - _BuiltInIcons.Append(make(hstring{ name }, box_value(val))); + builtInIcons.emplace_back(make(hstring{ name }, box_value(val))); } + _BuiltInIcons = single_threaded_vector(std::move(builtInIcons)); } void ProfileViewModel::_DeduceCurrentIconType() @@ -141,23 +148,24 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation const auto& profileIcon = _profile.Icon(); if (profileIcon == HideIconValue) { - CurrentIconType(_IconTypes.GetAt(0)); + _currentIconType = _IconTypes.GetAt(0); } else if (L"\uE700" <= profileIcon && profileIcon <= L"\uF8B3") { - CurrentIconType(_IconTypes.GetAt(1)); + _currentIconType = _IconTypes.GetAt(1); _DeduceCurrentBuiltInIcon(); } else if (profileIcon.size() <= 2) { // We already did a range check for MDL2 Assets in the previous one, // so if we're out of that range but still short, assume we're an emoji - CurrentIconType(_IconTypes.GetAt(2)); + _currentIconType = _IconTypes.GetAt(2); } else { - CurrentIconType(_IconTypes.GetAt(3)); + _currentIconType = _IconTypes.GetAt(3); } + _NotifyChanges(L"CurrentIconType"); } void ProfileViewModel::_DeduceCurrentBuiltInIcon() @@ -177,6 +185,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation } } _CurrentBuiltInIcon = _BuiltInIcons.GetAt(0); + _NotifyChanges(L"CurrentBuiltInIcon"); } Model::TerminalSettings ProfileViewModel::TermSettings() const @@ -452,7 +461,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { case IconType::None: { - Icon(HideIconValue); + _profile.Icon(HideIconValue); break; } case IconType::Image: @@ -461,7 +470,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { // Conversely, if we switch to Image, // retrieve that saved value and apply it - Icon(_lastIconPath); + _profile.Icon(_lastIconPath); } break; } @@ -469,18 +478,22 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { if (_CurrentBuiltInIcon) { - Icon(unbox_value(_CurrentBuiltInIcon.as().EnumValue())); + _profile.Icon(unbox_value(_CurrentBuiltInIcon.as().EnumValue())); } break; } case IconType::Emoji: { + // Don't set Icon here! + // Clear out the text box so we direct the user to use the emoji picker. CurrentEmojiIcon({}); } } - _NotifyChanges(L"CurrentIconType"); + // We're not using the VM's Icon() setter above, + // so notify HasIcon changed manually + _NotifyChanges(L"CurrentIconType", L"HasIcon"); } - }; + } bool ProfileViewModel::UsingNoIcon() const { diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Base.xaml b/src/cascadia/TerminalSettingsEditor/Profiles_Base.xaml index 44c3dba7e2f..75c4f31420f 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Base.xaml +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Base.xaml @@ -102,27 +102,24 @@ - - - - - - - - + + + + + + diff --git a/src/cascadia/TerminalSettingsModel/Profile.cpp b/src/cascadia/TerminalSettingsModel/Profile.cpp index 561a21017fc..73530361719 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.cpp +++ b/src/cascadia/TerminalSettingsModel/Profile.cpp @@ -408,6 +408,61 @@ winrt::hstring Profile::_evaluateIcon() const return winrt::hstring{ cmdline.c_str() }; } +bool Profile::HasIcon() const +{ + return _Icon.has_value(); +} + +winrt::Microsoft::Terminal::Settings::Model::Profile Profile::IconOverrideSource() +{ + for (const auto& parent : _parents) + { + if (auto source{ parent->_getIconOverrideSourceImpl() }) + { + return source; + } + } + return nullptr; +} + +void Profile::ClearIcon() +{ + _Icon = std::nullopt; + _evaluatedIcon = std::nullopt; +} + +std::optional Profile::_getIconImpl() const +{ + if (_Icon) + { + return _Icon; + } + for (const auto& parent : _parents) + { + if (auto val{ parent->_getIconImpl() }) + { + return val; + } + } + return std::nullopt; +} + +winrt::Microsoft::Terminal::Settings::Model::Profile Profile::_getIconOverrideSourceImpl() const +{ + if (_Icon) + { + return *this; + } + for (const auto& parent : _parents) + { + if (auto source{ parent->_getIconOverrideSourceImpl() }) + { + return source; + } + } + return nullptr; +} + // Given a commandLine like the following: // * "C:\WINDOWS\System32\cmd.exe" // * "pwsh -WorkingDirectory ~" diff --git a/src/cascadia/TerminalSettingsModel/Profile.h b/src/cascadia/TerminalSettingsModel/Profile.h index 24fbb7d1f2d..281dcabe0ad 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.h +++ b/src/cascadia/TerminalSettingsModel/Profile.h @@ -113,6 +113,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // Special fields hstring Icon() const; void Icon(const hstring& value); + bool HasIcon() const; + Model::Profile IconOverrideSource(); + void ClearIcon(); WINRT_PROPERTY(bool, Deleted, false); WINRT_PROPERTY(OriginTag, Origin, OriginTag::None); @@ -128,8 +131,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation INHERITABLE_SETTING(Model::Profile, bool, Hidden, false); INHERITABLE_SETTING(Model::Profile, guid, Guid, _GenerateGuidForProfile(Name(), Source())); INHERITABLE_SETTING(Model::Profile, hstring, Padding, DEFAULT_PADDING); - // Icon is _very special_ because we want to customize its setter - _BASE_INHERITABLE_SETTING(Model::Profile, std::optional, Icon, L"\uE756"); public: #define PROFILE_SETTINGS_INITIALIZE(type, name, jsonKey, ...) \ @@ -141,6 +142,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation Model::IAppearanceConfig _DefaultAppearance{ winrt::make(weak_ref(*this)) }; Model::FontConfig _FontInfo{ winrt::make(weak_ref(*this)) }; + std::optional _Icon{ std::nullopt }; std::optional _evaluatedIcon{ std::nullopt }; std::set _changeLog; @@ -149,6 +151,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation static guid _GenerateGuidForProfile(const std::wstring_view& name, const std::wstring_view& source) noexcept; winrt::hstring _evaluateIcon() const; + std::optional _getIconImpl() const; + Model::Profile _getIconOverrideSourceImpl() const; void _logSettingSet(const std::string_view& setting); void _logSettingIfSet(const std::string_view& setting, const bool isSet);