Skip to content

Commit

Permalink
Add icons to commands in the Command Palette (#7368)
Browse files Browse the repository at this point in the history 
## Summary of the Pull Request

![cmdpal-icons](https://user-images.githubusercontent.com/18356694/90916410-97dada00-e3a6-11ea-9fb0-755938a68a05.gif)

Adds support for setting a command's `icon`. This supports a couple different scenarios:
* setting a path to an image
* on `"iterateOn": "profiles"` commands, setting the icon to `${profile.icon}` (to use the profile's icon)
* setting the icon to a symbol from [Segoe MDL2 Assets](https://docs.microsoft.com/en-us/windows/uwp/design/style/segoe-ui-symbol-font)
* setting the icon to an emoji
* setting the icon to a character (what is an emoji other than a character, after all?)

## References
* Big s/o to @leonMSFT in #6732, who really did all the hard work here.

## PR Checklist
* [x] Closes #6644 
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

Importantly, the creation of these icons must occur on the UI thread. That's why it's done in a "load the path from json", then "get the actual IconSource" structure.

## Validation Steps Performed
see the gif
  • Loading branch information
@zadjii-msft
zadjii-msft authored Aug 21, 2020
1 parent 3d370dc commit 58efe79
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 31 deletions.
83 changes: 79 additions & 4 deletions src/cascadia/TerminalApp/Command.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,21 @@ using namespace winrt::TerminalApp;
using namespace winrt::Windows::Foundation;
using namespace ::TerminalApp;

namespace winrt
{
namespace MUX = Microsoft::UI::Xaml;
namespace WUX = Windows::UI::Xaml;
}

static constexpr std::string_view NameKey{ "name" };
static constexpr std::string_view IconPathKey{ "iconPath" };
static constexpr std::string_view IconKey{ "icon" };
static constexpr std::string_view ActionKey{ "command" };
static constexpr std::string_view ArgsKey{ "args" };
static constexpr std::string_view IterateOnKey{ "iterateOn" };
static constexpr std::string_view CommandsKey{ "commands" };

static constexpr std::string_view ProfileNameToken{ "${profile.name}" };
static constexpr std::string_view ProfileIconToken{ "${profile.icon}" };
static constexpr std::string_view SchemeNameToken{ "${scheme.name}" };

namespace winrt::TerminalApp::implementation
Expand Down Expand Up @@ -100,6 +107,70 @@ namespace winrt::TerminalApp::implementation
return actionAndArgs->GenerateName();
}

// Method Description:
// - Actually initialize our IconSource for our _lastIconPath. 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:
// - <none>
// Return Value:
// - <none>
void Command::RefreshIcon()
{
if (!_lastIconPath.empty())
{
_setIconSource(GetColoredIcon<winrt::WUX::Controls::IconSource>(_lastIconPath));

// 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 (IconSource() == nullptr && _lastIconPath.size() <= 2)
{
try
{
WUX::Controls::FontIconSource icon;
const wchar_t ch = _lastIconPath[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(_lastIconPath);
_setIconSource(icon);
}
CATCH_LOG();
}
}
if (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);
_setIconSource(icon);
}
}

// Method Description:
// - Deserialize a Command from the `json` object. The json object should
// contain a "name" and "action", and optionally an "icon".
Expand Down Expand Up @@ -143,9 +214,9 @@ namespace winrt::TerminalApp::implementation
return nullptr;
}

// TODO GH#6644: iconPath not implemented quite yet. Can't seem to get
// the binding quite right. Additionally, do we want it to be an image,
// or a FontIcon? I've had difficulty binding either/or.
// 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->_lastIconPath);

// If we're a nested command, we can ignore the current action.
if (!nested)
Expand Down Expand Up @@ -396,9 +467,13 @@ namespace winrt::TerminalApp::implementation

// - Escape the profile name for JSON appropriately
auto escapedProfileName = _escapeForJson(til::u16u8(p.GetName()));
auto escapedProfileIcon = _escapeForJson(til::u16u8(p.GetExpandedIconPath()));
auto newJsonString = til::replace_needle_in_haystack(oldJsonString,
ProfileNameToken,
escapedProfileName);
til::replace_needle_in_haystack_inplace(newJsonString,
ProfileIconToken,
escapedProfileIcon);

// If we encounter a re-parsing error, just stop processing the rest of the commands.
if (!reParseJson(newJsonString))
Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/TerminalApp/Command.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ namespace winrt::TerminalApp::implementation
bool HasNestedCommands();
Windows::Foundation::Collections::IMapView<winrt::hstring, TerminalApp::Command> NestedCommands();

void RefreshIcon();

winrt::Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker propertyChangedRevoker;

WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
Expand All @@ -64,6 +66,8 @@ namespace winrt::TerminalApp::implementation
Json::Value _originalJson;
Windows::Foundation::Collections::IMap<winrt::hstring, winrt::TerminalApp::Command> _subcommands{ nullptr };

winrt::hstring _lastIconPath{};

static std::vector<winrt::TerminalApp::Command> _expandCommand(Command* const expandable,
gsl::span<const ::TerminalApp::Profile> profiles,
gsl::span<winrt::TerminalApp::ColorScheme> schemes,
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/Command.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace TerminalApp
String KeyChordText;

Windows.UI.Xaml.Controls.IconSource IconSource;
void RefreshIcon();

Boolean HasNestedCommands { get; };
Windows.Foundation.Collections.IMapView<String, Command> NestedCommands { get; };
Expand Down
34 changes: 18 additions & 16 deletions src/cascadia/TerminalApp/CommandPalette.xaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ the MIT License. See LICENSE in the project root for license information. -->
<DataTemplate x:DataType="local:Command">

<!-- This HorizontalContentAlignment="Stretch" is important
to make sure it takes the entire width of the line -->
to make sure it takes the entire width of the line -->
<ListViewItem HorizontalContentAlignment="Stretch"
AutomationProperties.Name="{x:Bind Name, Mode=OneWay}"
AutomationProperties.AcceleratorKey="{x:Bind KeyChordText, Mode=OneWay}">
Expand All @@ -223,30 +223,32 @@ the MIT License. See LICENSE in the project root for license information. -->
</Grid.ColumnDefinitions>

<IconSourceElement
Grid.Column="0"
IconSource="{x:Bind IconSource, Mode=OneWay}"/>
Grid.Column="0"
Width="16"
Height="16"
IconSource="{x:Bind IconSource, Mode=OneWay}"/>

<TextBlock Grid.Column="1"
HorizontalAlignment="Left"
Text="{x:Bind Name, Mode=OneWay}" />

<!-- The block for the key chord is only visible
when there's actual text set as the label. See
CommandKeyChordVisibilityConverter for details. -->
when there's actual text set as the label. See
CommandKeyChordVisibilityConverter for details. -->
<Border
Grid.Column="2"
Visibility="{x:Bind KeyChordText,
Mode=OneWay,
Converter={StaticResource CommandKeyChordVisibilityConverter}}"
Style="{ThemeResource KeyChordBorderStyle}"
Padding="2,0,2,0"
HorizontalAlignment="Right"
VerticalAlignment="Center">
Grid.Column="2"
Visibility="{x:Bind KeyChordText,
Mode=OneWay,
Converter={StaticResource CommandKeyChordVisibilityConverter}}"
Style="{ThemeResource KeyChordBorderStyle}"
Padding="2,0,2,0"
HorizontalAlignment="Right"
VerticalAlignment="Center">

<TextBlock
Style="{ThemeResource KeyChordTextBlockStyle}"
FontSize="12"
Text="{x:Bind KeyChordText, Mode=OneWay}" />
Style="{ThemeResource KeyChordTextBlockStyle}"
FontSize="12"
Text="{x:Bind KeyChordText, Mode=OneWay}" />
</Border>

<!-- xE70E is ChevronUp. Rotated 90 degrees, it's _ChevronRight_ -->
Expand Down
18 changes: 7 additions & 11 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ namespace winrt::TerminalApp::implementation
// - settings: The settings who's keybindings we should use to look up the key chords from
// - commands: The list of commands to label.
static void _recursiveUpdateCommandKeybindingLabels(std::shared_ptr<::TerminalApp::CascadiaSettings> settings,
Windows::Foundation::Collections::IMapView<winrt::hstring, winrt::TerminalApp::Command> commands)
IMapView<winrt::hstring, winrt::TerminalApp::Command> commands)
{
for (const auto& nameAndCmd : commands)
{
Expand All @@ -83,20 +83,16 @@ namespace winrt::TerminalApp::implementation
}
}

static void _recursiveUpdateCommandIcons(Windows::Foundation::Collections::IMapView<winrt::hstring, winrt::TerminalApp::Command> commands)
static void _recursiveUpdateCommandIcons(IMapView<winrt::hstring, winrt::TerminalApp::Command> commands)
{
for (const auto& nameAndCmd : commands)
{
const auto& command = nameAndCmd.Value();
// 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);
command.IconSource(icon);

// !!! 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.
command.RefreshIcon();

if (command.HasNestedCommands())
{
Expand Down

0 comments on commit 58efe79

Please sign in to comment.