From 4fc607a44d91a883444c95590ce27e2d2f2da89c Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 8 Oct 2020 11:29:04 -0700 Subject: [PATCH] Introduce IconConverter (#7830) ## Summary of the Pull Request Introduce the `IconPathConverter` to `TerminalApp`. `Command` and `Profile` now both return the unexpanded icon path. `IconPathConverter` is responsible for expanding the icon path and retrieving the appropriate icon source. This also removes `Profile`'s expanded icon path and uses the `IconPathConverter` when necessary. This allows users to set profile icons to emoji as well. However, emoji do not appear in the jumplist. ## References Based on #7667 ## PR Checklist * [X] Closes #7784 * [x] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [x] Schema updated. ## Validation Steps Performed Deploy succeeded. --- doc/cascadia/profiles.schema.json | 20 +- .../DeserializationTests.cpp | 23 -- .../LocalTests_SettingsModel/ProfileTests.cpp | 22 +- src/cascadia/TerminalApp/CommandPalette.xaml | 5 +- .../TerminalApp/IconPathConverter.cpp | 207 ++++++++++++++++++ src/cascadia/TerminalApp/IconPathConverter.h | 30 +++ .../TerminalApp/IconPathConverter.idl | 22 ++ src/cascadia/TerminalApp/Jumplist.cpp | 9 +- src/cascadia/TerminalApp/Tab.cpp | 8 +- .../TerminalApp/TerminalAppLib.vcxproj | 8 +- src/cascadia/TerminalApp/TerminalPage.cpp | 93 +------- src/cascadia/TerminalApp/Utils.h | 80 ------- .../CascadiaSettings.cpp | 16 +- .../TerminalSettingsModel/Command.cpp | 6 +- src/cascadia/TerminalSettingsModel/Command.h | 3 +- .../TerminalSettingsModel/Command.idl | 3 +- .../DefaultProfileUtils.cpp | 2 +- .../PowershellCoreProfileGenerator.cpp | 2 +- .../TerminalSettingsModel/Profile.cpp | 18 +- src/cascadia/TerminalSettingsModel/Profile.h | 3 +- .../TerminalSettingsModel/Profile.idl | 3 +- .../WslDistroGenerator.cpp | 2 +- src/cascadia/inc/cppwinrt_utils.h | 4 +- 23 files changed, 334 insertions(+), 255 deletions(-) create mode 100644 src/cascadia/TerminalApp/IconPathConverter.cpp create mode 100644 src/cascadia/TerminalApp/IconPathConverter.h create mode 100644 src/cascadia/TerminalApp/IconPathConverter.idl delete mode 100644 src/cascadia/TerminalApp/Utils.h diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index b41d82c58..db7e46cb9 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -31,6 +31,13 @@ "pattern": "^\\{[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}\\}$", "type": "string" }, + "Icon": { + "description": "Image file location or an emoji to be used as an icon. Displays within the tab, the dropdown menu, and jumplist.", + "type": [ + "string", + "null" + ] + }, "ShortcutActionName": { "enum": [ "adjustFontSize", @@ -465,6 +472,14 @@ "type": "array" } ] + }, + "icon": { "$ref": "#/definitions/Icon" }, + "name": { + "description": "The name that will appear in the command palette. If one isn't provided, the terminal will attempt to automatically generate a name.", + "type": [ + "string", + "null" + ] } }, "required": [ @@ -810,10 +825,7 @@ "minimum": -1, "type": "integer" }, - "icon": { - "description": "Image file location of the icon used in the profile. Displays within the tab and the dropdown menu.", - "type": ["string", "null"] - }, + "icon":{ "$ref": "#/definitions/Icon" }, "name": { "description": "Name of the profile. Displays in the dropdown menu.", "minLength": 1, diff --git a/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp b/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp index e6e903776..283646a6f 100644 --- a/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp @@ -54,7 +54,6 @@ namespace SettingsModelLocalTests TEST_METHOD(TestInvalidColorSchemeName); TEST_METHOD(TestHelperFunctions); - TEST_METHOD(TestProfileIconWithEnvVar); TEST_METHOD(TestProfileBackgroundImageWithEnvVar); TEST_METHOD(TestCloseOnExitParsing); @@ -1379,28 +1378,6 @@ namespace SettingsModelLocalTests VERIFY_ARE_EQUAL(name2, prof2.Name()); } - void DeserializationTests::TestProfileIconWithEnvVar() - { - const auto expectedPath = wil::ExpandEnvironmentStringsW(L"%WINDIR%\\System32\\x_80.png"); - - const std::string settingsJson{ R"( - { - "profiles": [ - { - "name": "profile0", - "icon": "%WINDIR%\\System32\\x_80.png" - } - ] - })" }; - - VerifyParseSucceeded(settingsJson); - - auto settings = winrt::make_self(); - settings->_ParseJsonString(settingsJson, false); - settings->LayerJson(settings->_userSettings); - VERIFY_ARE_NOT_EQUAL(0u, settings->_profiles.Size()); - VERIFY_ARE_EQUAL(expectedPath, settings->_profiles.GetAt(0).ExpandedIconPath()); - } void DeserializationTests::TestProfileBackgroundImageWithEnvVar() { const auto expectedPath = wil::ExpandEnvironmentStringsW(L"%WINDIR%\\System32\\x_80.png"); diff --git a/src/cascadia/LocalTests_SettingsModel/ProfileTests.cpp b/src/cascadia/LocalTests_SettingsModel/ProfileTests.cpp index 09f4fa053..03d2e7c08 100644 --- a/src/cascadia/LocalTests_SettingsModel/ProfileTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/ProfileTests.cpp @@ -195,32 +195,32 @@ namespace SettingsModelLocalTests const auto profile3Json = VerifyParseSucceeded(profile3String); auto profile0 = implementation::Profile::FromJson(profile0Json); - VERIFY_IS_FALSE(profile0->IconPath().empty()); - VERIFY_ARE_EQUAL(L"not-null.png", profile0->IconPath()); + VERIFY_IS_FALSE(profile0->Icon().empty()); + VERIFY_ARE_EQUAL(L"not-null.png", profile0->Icon()); Log::Comment(NoThrowString().Format( L"Verify that layering an object the key set to null will clear the key")); profile0->LayerJson(profile1Json); - VERIFY_IS_TRUE(profile0->IconPath().empty()); + VERIFY_IS_TRUE(profile0->Icon().empty()); profile0->LayerJson(profile2Json); - VERIFY_IS_TRUE(profile0->IconPath().empty()); + VERIFY_IS_TRUE(profile0->Icon().empty()); profile0->LayerJson(profile3Json); - VERIFY_IS_FALSE(profile0->IconPath().empty()); - VERIFY_ARE_EQUAL(L"another-real.png", profile0->IconPath()); + VERIFY_IS_FALSE(profile0->Icon().empty()); + VERIFY_ARE_EQUAL(L"another-real.png", profile0->Icon()); Log::Comment(NoThrowString().Format( L"Verify that layering an object _without_ the key will not clear the key")); profile0->LayerJson(profile2Json); - VERIFY_IS_FALSE(profile0->IconPath().empty()); - VERIFY_ARE_EQUAL(L"another-real.png", profile0->IconPath()); + VERIFY_IS_FALSE(profile0->Icon().empty()); + VERIFY_ARE_EQUAL(L"another-real.png", profile0->Icon()); auto profile1 = implementation::Profile::FromJson(profile1Json); - VERIFY_IS_TRUE(profile1->IconPath().empty()); + VERIFY_IS_TRUE(profile1->Icon().empty()); profile1->LayerJson(profile3Json); - VERIFY_IS_FALSE(profile1->IconPath().empty()); - VERIFY_ARE_EQUAL(L"another-real.png", profile1->IconPath()); + VERIFY_IS_FALSE(profile1->Icon().empty()); + VERIFY_ARE_EQUAL(L"another-real.png", profile1->Icon()); } void ProfileTests::LayerProfilesOnArray() diff --git a/src/cascadia/TerminalApp/CommandPalette.xaml b/src/cascadia/TerminalApp/CommandPalette.xaml index 212270878..1a3808cfa 100644 --- a/src/cascadia/TerminalApp/CommandPalette.xaml +++ b/src/cascadia/TerminalApp/CommandPalette.xaml @@ -31,6 +31,7 @@ the MIT License. See LICENSE in the project root for license information. --> + @@ -227,7 +228,9 @@ the MIT License. See LICENSE in the project root for license information. --> Grid.Column="0" Width="16" Height="16" - IconSource="{x:Bind IconSource, Mode=OneWay}"/> + IconSource="{x:Bind Icon, + Mode=OneWay, + Converter={StaticResource IconSourceConverter}}"/> + struct BitmapIconSource + { + }; + + template<> + struct BitmapIconSource + { + using type = winrt::Microsoft::UI::Xaml::Controls::BitmapIconSource; + }; + + template<> + struct BitmapIconSource + { + using type = winrt::Windows::UI::Xaml::Controls::BitmapIconSource; + }; +#pragma endregion + +#pragma region FontIconSource + template + struct FontIconSource + { + }; + + template<> + struct FontIconSource + { + using type = winrt::Microsoft::UI::Xaml::Controls::FontIconSource; + }; + + template<> + struct FontIconSource + { + using type = winrt::Windows::UI::Xaml::Controls::FontIconSource; + }; +#pragma endregion + + // Method Description: + // - Creates an IconSource for the given path. The icon returned is a colored + // icon. If we couldn't create the icon for any reason, we return an empty + // IconElement. + // Template Types: + // - : The type of IconSource (MUX, WUX) to generate. + // Arguments: + // - path: the full, expanded path to the icon. + // Return Value: + // - An IconElement with its IconSource set, if possible. + template + TIconSource _getColoredBitmapIcon(const winrt::hstring& path) + { + if (!path.empty()) + { + try + { + winrt::Windows::Foundation::Uri iconUri{ path }; + BitmapIconSource::type iconSource; + // Make sure to set this to false, so we keep the RGB data of the + // image. Otherwise, the icon will be white for all the + // non-transparent pixels in the image. + iconSource.ShowAsMonochrome(false); + iconSource.UriSource(iconUri); + return iconSource; + } + CATCH_LOG(); + } + + return nullptr; + } + + // Method Description: + // - Creates an IconSource for the given path. + // * If the icon is a path to an image, we'll use that. + // * If it isn't, then we'll try and use the text as a FontIcon. If the + // character is in the range of symbols reserved for the Segoe MDL2 + // Asserts, well treat it as such. Otherwise, we'll default to a Sego + // UI icon, so things like emoji will work. + // * If we couldn't create the icon for any reason, we return an empty + // IconElement. + // Template Types: + // - : The type of IconSource (MUX, WUX) to generate. + // Arguments: + // - path: the unprocessed path to the icon. + // Return Value: + // - An IconElement with its IconSource set, if possible. + template + TIconSource _getIconSource(const winrt::hstring& iconPath) + { + TIconSource iconSource{ nullptr }; + + if (iconPath.size() != 0) + { + const auto expandedIconPath{ _expandIconPath(iconPath) }; + iconSource = _getColoredBitmapIcon(expandedIconPath); + + // If we fail to set the icon source using the "icon" as a path, + // let's try it as a symbol/emoji. + // + // Anything longer than 2 wchar_t's _isn't_ an emoji or symbol, so + // don't do this if it's just an invalid path. + if (!iconSource && iconPath.size() <= 2) + { + try + { + FontIconSource::type icon; + const wchar_t ch = iconPath[0]; + + // The range of MDL2 Icons isn't explicitly defined, but + // we're using this based off the table on: + // https://docs.microsoft.com/en-us/windows/uwp/design/style/segoe-ui-symbol-font + const bool isMDL2Icon = ch >= L'\uE700' && ch <= L'\uF8FF'; + if (isMDL2Icon) + { + icon.FontFamily(winrt::Windows::UI::Xaml::Media::FontFamily{ L"Segoe MDL2 Assets" }); + } + else + { + // Note: you _do_ need to manually set the font here. + icon.FontFamily(winrt::Windows::UI::Xaml::Media::FontFamily{ L"Segoe UI" }); + } + icon.FontSize(12); + icon.Glyph(iconPath); + iconSource = icon; + } + CATCH_LOG(); + } + } + if (!iconSource) + { + // Set the default IconSource to a BitmapIconSource with a null source + // (instead of just nullptr) because there's a really weird crash when swapping + // data bound IconSourceElements in a ListViewTemplate (i.e. CommandPalette). + // Swapping between nullptr IconSources and non-null IconSources causes a crash + // to occur, but swapping between IconSources with a null source and non-null IconSources + // work perfectly fine :shrug:. + BitmapIconSource::type icon; + icon.UriSource(nullptr); + iconSource = icon; + } + + return iconSource; + } + + static winrt::hstring _expandIconPath(hstring iconPath) + { + if (iconPath.empty()) + { + return iconPath; + } + winrt::hstring envExpandedPath{ wil::ExpandEnvironmentStringsW(iconPath.c_str()) }; + return envExpandedPath; + } + + // Method Description: + // - Attempt to convert something into another type. For the + // IconPathConverter, we support a variety of icons: + // * If the icon is a path to an image, we'll use that. + // * If it isn't, then we'll try and use the text as a FontIcon. If the + // character is in the range of symbols reserved for the Segoe MDL2 + // Asserts, well treat it as such. Otherwise, we'll default to a Sego + // UI icon, so things like emoji will work. + // - MUST BE CALLED ON THE UI THREAD. + // Arguments: + // - value: the input object to attempt to convert into an IconSource. + // Return Value: + // - Visible if the object was a string and wasn't the empty string. + Foundation::IInspectable IconPathConverter::Convert(Foundation::IInspectable const& value, + Windows::UI::Xaml::Interop::TypeName const& /* targetType */, + Foundation::IInspectable const& /* parameter */, + hstring const& /* language */) + { + const auto& iconPath = winrt::unbox_value_or(value, L""); + return _getIconSource(iconPath); + } + + // unused for one-way bindings + Foundation::IInspectable IconPathConverter::ConvertBack(Foundation::IInspectable const& /* value */, + Windows::UI::Xaml::Interop::TypeName const& /* targetType */, + Foundation::IInspectable const& /* parameter */, + hstring const& /* language */) + { + throw hresult_not_implemented(); + } + + Windows::UI::Xaml::Controls::IconSource IconPathConverter::IconSourceWUX(hstring path) + { + return _getIconSource(path); + } + + Microsoft::UI::Xaml::Controls::IconSource IconPathConverter::IconSourceMUX(hstring path) + { + return _getIconSource(path); + } +} diff --git a/src/cascadia/TerminalApp/IconPathConverter.h b/src/cascadia/TerminalApp/IconPathConverter.h new file mode 100644 index 000000000..2dabd47f9 --- /dev/null +++ b/src/cascadia/TerminalApp/IconPathConverter.h @@ -0,0 +1,30 @@ +#pragma once + +#include "IconPathConverter.g.h" +#include "..\inc\cppwinrt_utils.h" + +namespace winrt::TerminalApp::implementation +{ + struct IconPathConverter : IconPathConverterT + { + IconPathConverter() = default; + + Windows::Foundation::IInspectable Convert(Windows::Foundation::IInspectable const& value, + Windows::UI::Xaml::Interop::TypeName const& targetType, + Windows::Foundation::IInspectable const& parameter, + hstring const& language); + + Windows::Foundation::IInspectable ConvertBack(Windows::Foundation::IInspectable const& value, + Windows::UI::Xaml::Interop::TypeName const& targetType, + Windows::Foundation::IInspectable const& parameter, + hstring const& language); + + static Windows::UI::Xaml::Controls::IconSource IconSourceWUX(hstring path); + static Microsoft::UI::Xaml::Controls::IconSource IconSourceMUX(hstring path); + }; +} + +namespace winrt::TerminalApp::factory_implementation +{ + BASIC_FACTORY(IconPathConverter); +} diff --git a/src/cascadia/TerminalApp/IconPathConverter.idl b/src/cascadia/TerminalApp/IconPathConverter.idl new file mode 100644 index 000000000..3e8dd1835 --- /dev/null +++ b/src/cascadia/TerminalApp/IconPathConverter.idl @@ -0,0 +1,22 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +namespace TerminalApp +{ + // See https://docs.microsoft.com/en-us/windows/uwp/data-binding/data-binding-quickstart + + // We use the default attribute to declare IValueConverter as the default + // interface. In the listing, IconPathConverter has only a + // constructor, and no methods, so no default interface is generated for it. + // The default attribute is optimal if you won't be adding instance members + // to IconPathConverter, because no QueryInterface will be + // required to call the IValueConverter methods + runtimeclass IconPathConverter : [default] Windows.UI.Xaml.Data.IValueConverter + { + IconPathConverter(); + + static Windows.UI.Xaml.Controls.IconSource IconSourceWUX(String path); + static Microsoft.UI.Xaml.Controls.IconSource IconSourceMUX(String path); + }; + +} diff --git a/src/cascadia/TerminalApp/Jumplist.cpp b/src/cascadia/TerminalApp/Jumplist.cpp index 291e71b6b..53bfc8e75 100644 --- a/src/cascadia/TerminalApp/Jumplist.cpp +++ b/src/cascadia/TerminalApp/Jumplist.cpp @@ -43,12 +43,13 @@ static constexpr bool _isProbableFilePath(std::wstring_view path) // paths to have the "correct" slash direction. static std::wstring _normalizeIconPath(std::wstring_view path) { - if (_isProbableFilePath(path)) + const auto fullPath{ wil::ExpandEnvironmentStringsW(path.data()) }; + if (_isProbableFilePath(fullPath)) { - std::filesystem::path asPath{ path }; + std::filesystem::path asPath{ fullPath }; return asPath.make_preferred().wstring(); } - return std::wstring{ path }; + return std::wstring{ fullPath }; } // Function Description: @@ -168,7 +169,7 @@ HRESULT Jumplist::UpdateJumplist(const CascadiaSettings& settings) noexcept // Create the shell link object for the profile winrt::com_ptr shLink; - const auto normalizedIconPath{ _normalizeIconPath(profile.ExpandedIconPath()) }; + const auto normalizedIconPath{ _normalizeIconPath(profile.Icon()) }; RETURN_IF_FAILED(_createShellLink(profile.Name(), normalizedIconPath, args, shLink.put())); RETURN_IF_FAILED(jumplistItems->AddObject(shLink.get())); diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp index cd494049f..f571457d5 100644 --- a/src/cascadia/TerminalApp/Tab.cpp +++ b/src/cascadia/TerminalApp/Tab.cpp @@ -229,11 +229,11 @@ namespace winrt::TerminalApp::implementation if (auto tab{ weakThis.get() }) { // The TabViewItem Icon needs MUX while the IconSourceElement in the CommandPalette needs WUX... - IconSource(GetColoredIcon(_lastIconPath)); - _tabViewItem.IconSource(GetColoredIcon(_lastIconPath)); + IconSource(IconPathConverter::IconSourceWUX(_lastIconPath)); + _tabViewItem.IconSource(IconPathConverter::IconSourceMUX(_lastIconPath)); // Update SwitchToTab command's icon - SwitchToTabCommand().IconSource(IconSource()); + SwitchToTabCommand().Icon(_lastIconPath); } } @@ -1048,7 +1048,7 @@ namespace winrt::TerminalApp::implementation Command command; command.Action(focusTabAction); command.Name(Title()); - command.IconSource(IconSource()); + command.Icon(_lastIconPath); SwitchToTabCommand(command); } diff --git a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj index af7afce7e..706be36e3 100644 --- a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj +++ b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj @@ -98,11 +98,13 @@ HasNestedCommandsVisibilityConverter.idl + + IconPathConverter.idl + Tab.idl - TerminalSettings.idl @@ -153,6 +155,9 @@ HasNestedCommandsVisibilityConverter.idl + + IconPathConverter.idl + Tab.idl @@ -220,6 +225,7 @@ + diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index d3395051a..fad50d694 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -81,88 +81,6 @@ namespace winrt::TerminalApp::implementation } } - // Method Description: - // - Set the command's IconSource. Supports a variety of icons: - // * If the icon is a path to an image, we'll use that. - // * If it isn't, then we'll try and use the text as a FontIcon. If the - // character is in the range of symbols reserved for the Segoe MDL2 - // Asserts, well treat it as such. Otherwise, we'll default to a Sego - // UI icon, so things like emoji will work. - // - MUST BE CALLED ON THE UI THREAD. - // Arguments: - // - cmd - the command that we're updating IconSource for - // Return Value: - // - - static void _refreshIcon(const Command& cmd) - { - if (cmd.IconPath().size() != 0) - { - cmd.IconSource(GetColoredIcon(cmd.IconPath())); - - // If we fail to set the icon source using the "icon" as a path, - // let's try it as a symbol/emoji. - // - // Anything longer that 2 wchar_t's _isn't_ an emoji or symbol, so - // don't do this if it's just an invalid path. - if (cmd.IconSource() == nullptr && cmd.IconPath().size() <= 2) - { - try - { - WUX::Controls::FontIconSource icon; - const wchar_t ch = cmd.IconPath()[0]; - - // The range of MDL2 Icons isn't explicitly defined, but - // we're using this based off the table on: - // https://docs.microsoft.com/en-us/windows/uwp/design/style/segoe-ui-symbol-font - const bool isMDL2Icon = ch >= L'\uE700' && ch <= L'\uF8FF'; - if (isMDL2Icon) - { - icon.FontFamily(WUX::Media::FontFamily{ L"Segoe MDL2 Assets" }); - } - else - { - // Note: you _do_ need to manually set the font here. - icon.FontFamily(WUX::Media::FontFamily{ L"Segoe UI" }); - } - icon.FontSize(12); - icon.Glyph(cmd.IconPath()); - cmd.IconSource(icon); - } - CATCH_LOG(); - } - } - if (cmd.IconSource() == nullptr) - { - // Set the default IconSource to a BitmapIconSource with a null source - // (instead of just nullptr) because there's a really weird crash when swapping - // data bound IconSourceElements in a ListViewTemplate (i.e. CommandPalette). - // Swapping between nullptr IconSources and non-null IconSources causes a crash - // to occur, but swapping between IconSources with a null source and non-null IconSources - // work perfectly fine :shrug:. - winrt::Windows::UI::Xaml::Controls::BitmapIconSource icon; - icon.UriSource(nullptr); - cmd.IconSource(icon); - } - } - - static void _recursiveUpdateCommandIcons(IMapView commands) - { - for (const auto& nameAndCmd : commands) - { - const auto& command = nameAndCmd.Value(); - - // !!! LOAD-BEARING !!! If this is never called, then Commands will - // have a nullptr icon. If they do, a really weird crash can occur. - // MAKE SURE this is called once after a settings load. - _refreshIcon(command); - - if (command.HasNestedCommands()) - { - _recursiveUpdateCommandIcons(command.NestedCommands()); - } - } - } - winrt::fire_and_forget TerminalPage::SetSettings(CascadiaSettings settings, bool needRefreshUI) { _settings = settings; @@ -530,9 +448,9 @@ namespace winrt::TerminalApp::implementation // If there's an icon set for this profile, set it as the icon for // this flyout item. - if (!profile.IconPath().empty()) + if (!profile.Icon().empty()) { - auto iconSource = GetColoredIcon(profile.ExpandedIconPath()); + const auto iconSource{ IconPathConverter().IconSourceWUX(profile.Icon()) }; WUX::Controls::IconSourceElement iconElement; iconElement.IconSource(iconSource); @@ -776,9 +694,9 @@ namespace winrt::TerminalApp::implementation // Set this tab's icon to the icon from the user's profile const auto profile = _settings.FindProfile(profileGuid); - if (profile != nullptr && !profile.IconPath().empty()) + if (profile != nullptr && !profile.Icon().empty()) { - newTabImpl->UpdateIcon(profile.ExpandedIconPath()); + newTabImpl->UpdateIcon(profile.Icon()); } tabViewItem.PointerPressed({ this, &TerminalPage::_OnTabClick }); @@ -1020,7 +938,7 @@ namespace winrt::TerminalApp::implementation const auto matchingProfile = _settings.FindProfile(lastFocusedProfile); if (matchingProfile) { - tab.UpdateIcon(matchingProfile.ExpandedIconPath()); + tab.UpdateIcon(matchingProfile.Icon()); } else { @@ -2236,7 +2154,6 @@ namespace winrt::TerminalApp::implementation _settings.GlobalSettings().ColorSchemes()); _recursiveUpdateCommandKeybindingLabels(_settings, copyOfCommands.GetView()); - _recursiveUpdateCommandIcons(copyOfCommands.GetView()); // Update the command palette when settings reload auto commandsCollection = winrt::single_threaded_vector(); diff --git a/src/cascadia/TerminalApp/Utils.h b/src/cascadia/TerminalApp/Utils.h deleted file mode 100644 index dee4e6596..000000000 --- a/src/cascadia/TerminalApp/Utils.h +++ /dev/null @@ -1,80 +0,0 @@ -/*++ -Copyright (c) Microsoft Corporation -Licensed under the MIT license. - -Module Name: -- Utils.h - -Abstract: -- Helpers for the TerminalApp project -Author(s): -- Mike Griese - May 2019 - ---*/ -#pragma once - -namespace winrt::Microsoft::UI::Xaml::Controls -{ - struct IconSource; - struct BitmapIconSource; -} - -namespace winrt::Windows::UI::Xaml::Controls -{ - struct IconSource; - struct BitmapIconSource; -} - -namespace Microsoft::TerminalApp::details -{ - // This is a template that helps us figure out which BitmapIconSource to use for a given IconSource. - // We have to do this because some of our code still wants to use WUX IconSources. - template - struct BitmapIconSource - { - }; - - template<> - struct BitmapIconSource - { - using type = winrt::Microsoft::UI::Xaml::Controls::BitmapIconSource; - }; - - template<> - struct BitmapIconSource - { - using type = winrt::Windows::UI::Xaml::Controls::BitmapIconSource; - }; -} - -// Method Description: -// - Creates an IconElement for the given path. The icon returned is a colored -// icon. If we couldn't create the icon for any reason, we return an empty -// IconElement. -// Template Types: -// - : The type of IconSource (MUX, WUX) to generate. -// Arguments: -// - path: the full, expanded path to the icon. -// Return Value: -// - An IconElement with its IconSource set, if possible. -template -TIconSource GetColoredIcon(const winrt::hstring& path) -{ - if (!path.empty()) - { - try - { - winrt::Windows::Foundation::Uri iconUri{ path }; - ::Microsoft::TerminalApp::details::BitmapIconSource::type iconSource; - // Make sure to set this to false, so we keep the RGB data of the - // image. Otherwise, the icon will be white for all the - // non-transparent pixels in the image. - iconSource.ShowAsMonochrome(false); - iconSource.UriSource(iconUri); - return iconSource; - } - CATCH_LOG(); - } - - return nullptr; -} diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp index c8d1364b9..b54c2daea 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp @@ -471,17 +471,23 @@ void CascadiaSettings::_ValidateMediaResources() } } - if (!profile.IconPath().empty()) + if (!profile.Icon().empty()) { + const auto iconPath{ wil::ExpandEnvironmentStringsW(profile.Icon().c_str()) }; try { - winrt::Windows::Foundation::Uri imagePath{ profile.ExpandedIconPath() }; + winrt::Windows::Foundation::Uri imagePath{ iconPath }; } catch (...) { - // reset icon path - profile.IconPath(L""); - invalidIcon = true; + // Anything longer than 2 wchar_t's _isn't_ an emoji or symbol, + // so treat it as an invalid path. + if (iconPath.size() > 2) + { + // reset icon path + profile.Icon(L""); + invalidIcon = true; + } } } } diff --git a/src/cascadia/TerminalSettingsModel/Command.cpp b/src/cascadia/TerminalSettingsModel/Command.cpp index e0cc54672..c060886b6 100644 --- a/src/cascadia/TerminalSettingsModel/Command.cpp +++ b/src/cascadia/TerminalSettingsModel/Command.cpp @@ -149,9 +149,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation return nullptr; } - // Only get the icon path right now. The icon needs to be resolved into - // an IconSource on the UI thread, which will be done by RefreshIcon. - JsonUtils::GetValueForKey(json, IconKey, result->_IconPath); + JsonUtils::GetValueForKey(json, IconKey, result->_Icon); // If we're a nested command, we can ignore the current action. if (!nested) @@ -404,7 +402,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // - Escape the profile name for JSON appropriately auto escapedProfileName = _escapeForJson(til::u16u8(p.Name())); - auto escapedProfileIcon = _escapeForJson(til::u16u8(p.ExpandedIconPath())); + auto escapedProfileIcon = _escapeForJson(til::u16u8(p.Icon())); auto newJsonString = til::replace_needle_in_haystack(oldJsonString, ProfileNameToken, escapedProfileName); diff --git a/src/cascadia/TerminalSettingsModel/Command.h b/src/cascadia/TerminalSettingsModel/Command.h index d73058cc6..01579f1ac 100644 --- a/src/cascadia/TerminalSettingsModel/Command.h +++ b/src/cascadia/TerminalSettingsModel/Command.h @@ -56,8 +56,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation OBSERVABLE_GETSET_PROPERTY(winrt::hstring, Name, _PropertyChangedHandlers); OBSERVABLE_GETSET_PROPERTY(Model::ActionAndArgs, Action, _PropertyChangedHandlers); OBSERVABLE_GETSET_PROPERTY(winrt::hstring, KeyChordText, _PropertyChangedHandlers); - OBSERVABLE_GETSET_PROPERTY(winrt::Windows::UI::Xaml::Controls::IconSource, IconSource, _PropertyChangedHandlers, nullptr); - GETSET_PROPERTY(winrt::hstring, IconPath); + OBSERVABLE_GETSET_PROPERTY(winrt::hstring, Icon, _PropertyChangedHandlers); GETSET_PROPERTY(ExpandCommandType, IterateOn, ExpandCommandType::None); diff --git a/src/cascadia/TerminalSettingsModel/Command.idl b/src/cascadia/TerminalSettingsModel/Command.idl index 98d3bc27a..13ea7a210 100644 --- a/src/cascadia/TerminalSettingsModel/Command.idl +++ b/src/cascadia/TerminalSettingsModel/Command.idl @@ -16,8 +16,7 @@ namespace Microsoft.Terminal.Settings.Model ActionAndArgs Action; String KeyChordText; - Windows.UI.Xaml.Controls.IconSource IconSource; - String IconPath { get; }; + String Icon; Boolean HasNestedCommands { get; }; Windows.Foundation.Collections.IMapView NestedCommands { get; }; diff --git a/src/cascadia/TerminalSettingsModel/DefaultProfileUtils.cpp b/src/cascadia/TerminalSettingsModel/DefaultProfileUtils.cpp index 57b370114..32ba251c7 100644 --- a/src/cascadia/TerminalSettingsModel/DefaultProfileUtils.cpp +++ b/src/cascadia/TerminalSettingsModel/DefaultProfileUtils.cpp @@ -26,7 +26,7 @@ winrt::Microsoft::Terminal::Settings::Model::Profile CreateDefaultProfile(const iconPath.append(Microsoft::Console::Utils::GuidToString(profileGuid)); iconPath.append(PACKAGED_PROFILE_ICON_EXTENSION); - newProfile.IconPath(iconPath); + newProfile.Icon(iconPath); return newProfile; } diff --git a/src/cascadia/TerminalSettingsModel/PowershellCoreProfileGenerator.cpp b/src/cascadia/TerminalSettingsModel/PowershellCoreProfileGenerator.cpp index a6bc2d50d..f882a9290 100644 --- a/src/cascadia/TerminalSettingsModel/PowershellCoreProfileGenerator.cpp +++ b/src/cascadia/TerminalSettingsModel/PowershellCoreProfileGenerator.cpp @@ -313,7 +313,7 @@ std::vector PowershellCoreProfileGenerator::GenerateProfiles() profile.StartingDirectory(DEFAULT_STARTING_DIRECTORY); profile.ColorSchemeName(L"Campbell"); - profile.IconPath(WI_IsFlagSet(psI.flags, PowerShellFlags::Preview) ? POWERSHELL_PREVIEW_ICON : POWERSHELL_ICON); + profile.Icon(WI_IsFlagSet(psI.flags, PowerShellFlags::Preview) ? POWERSHELL_PREVIEW_ICON : POWERSHELL_ICON); profiles.emplace_back(std::move(profile)); } diff --git a/src/cascadia/TerminalSettingsModel/Profile.cpp b/src/cascadia/TerminalSettingsModel/Profile.cpp index 249a3bc46..d3419d342 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.cpp +++ b/src/cascadia/TerminalSettingsModel/Profile.cpp @@ -232,7 +232,7 @@ void Profile::LayerJson(const Json::Value& json) JsonUtils::GetValueForKey(json, ScrollbarStateKey, _ScrollState); JsonUtils::GetValueForKey(json, StartingDirectoryKey, _StartingDirectory); - JsonUtils::GetValueForKey(json, IconKey, _IconPath); + JsonUtils::GetValueForKey(json, IconKey, _Icon); JsonUtils::GetValueForKey(json, BackgroundImageKey, _BackgroundImagePath); JsonUtils::GetValueForKey(json, BackgroundImageOpacityKey, _BackgroundImageOpacity); JsonUtils::GetValueForKey(json, BackgroundImageStretchModeKey, _BackgroundImageStretchMode); @@ -243,22 +243,6 @@ void Profile::LayerJson(const Json::Value& json) JsonUtils::GetValueForKey(json, TabColorKey, _TabColor); } -// Method Description: -// - Returns this profile's icon path, if one is set. Otherwise returns the -// empty string. This method will expand any environment variables in the -// path, if there are any. -// Return Value: -// - this profile's icon path, if one is set. Otherwise returns the empty string. -winrt::hstring Profile::ExpandedIconPath() const -{ - if (_IconPath.empty()) - { - return _IconPath; - } - winrt::hstring envExpandedPath{ wil::ExpandEnvironmentStringsW(_IconPath.c_str()) }; - return envExpandedPath; -} - // Method Description: // - Returns this profile's background image path, if one is set, expanding // any environment variables in the path, if there are any. diff --git a/src/cascadia/TerminalSettingsModel/Profile.h b/src/cascadia/TerminalSettingsModel/Profile.h index 7fac38028..eec8d1dda 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.h +++ b/src/cascadia/TerminalSettingsModel/Profile.h @@ -52,7 +52,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation static bool IsDynamicProfileObject(const Json::Value& json); hstring EvaluatedStartingDirectory() const; - hstring ExpandedIconPath() const; hstring ExpandedBackgroundImagePath() const; void GenerateGuidIfNecessary() noexcept; static guid GetGuidOrGenerateForJson(const Json::Value& json) noexcept; @@ -75,7 +74,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation GETSET_PROPERTY(hstring, Source); GETSET_PROPERTY(bool, Hidden, false); - GETSET_PROPERTY(hstring, IconPath); + GETSET_PROPERTY(hstring, Icon); GETSET_PROPERTY(CloseOnExitMode, CloseOnExit, CloseOnExitMode::Graceful); GETSET_PROPERTY(hstring, TabTitle); diff --git a/src/cascadia/TerminalSettingsModel/Profile.idl b/src/cascadia/TerminalSettingsModel/Profile.idl index 2391a162f..a81c95bfa 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.idl +++ b/src/cascadia/TerminalSettingsModel/Profile.idl @@ -22,8 +22,7 @@ namespace Microsoft.Terminal.Settings.Model Guid ConnectionType; Boolean Hidden; - String IconPath; - String ExpandedIconPath { get; }; + String Icon; CloseOnExitMode CloseOnExit; String TabTitle; diff --git a/src/cascadia/TerminalSettingsModel/WslDistroGenerator.cpp b/src/cascadia/TerminalSettingsModel/WslDistroGenerator.cpp index 1aae93754..3cd4b7b0d 100644 --- a/src/cascadia/TerminalSettingsModel/WslDistroGenerator.cpp +++ b/src/cascadia/TerminalSettingsModel/WslDistroGenerator.cpp @@ -129,7 +129,7 @@ std::vector WslDistroGenerator::GenerateProfiles() WSLDistro.Commandline(L"wsl.exe -d " + distName); WSLDistro.ColorSchemeName(L"Campbell"); WSLDistro.StartingDirectory(DEFAULT_STARTING_DIRECTORY); - WSLDistro.IconPath(L"ms-appx:///ProfileIcons/{9acb9455-ca41-5af7-950f-6bca1bc9722f}.png"); + WSLDistro.Icon(L"ms-appx:///ProfileIcons/{9acb9455-ca41-5af7-950f-6bca1bc9722f}.png"); profiles.emplace_back(WSLDistro); } } diff --git a/src/cascadia/inc/cppwinrt_utils.h b/src/cascadia/inc/cppwinrt_utils.h index 72f0526d3..a1ac58fb8 100644 --- a/src/cascadia/inc/cppwinrt_utils.h +++ b/src/cascadia/inc/cppwinrt_utils.h @@ -133,13 +133,13 @@ public: { \ if (_##name != value) \ { \ - const_cast(_##name) = value; \ + _##name = value; \ event(*this, Windows::UI::Xaml::Data::PropertyChangedEventArgs{ L#name }); \ } \ }; \ \ private: \ - const type _##name{ __VA_ARGS__ }; \ + type _##name{ __VA_ARGS__ }; \ void _set##name(const type& value) \ { \ const_cast(_##name) = value; \