Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable changing the color of the active pane border #3752

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,17 @@
"additionalProperties": true,
"description": "Properties that affect the entire window, regardless of the profile settings.",
"properties": {

"activePaneBorderColor": {
"default": "accent",
"oneOf": [
{"$ref": "#/definitions/Color"},
{"type": "string", "pattern": "accent" }
],
"description": "The color to use for the active pane border. Should be null, \"accent\", or a color in format \"#RRGGBB\"",
"type": ["string", "null"]
},

"alwaysShowTabs": {
"default": true,
"description": "When set to true, tabs are always displayed. When set to false and showTabsInTitlebar is set to false, tabs only appear after opening a new tab.",
Expand Down
5 changes: 3 additions & 2 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ namespace winrt
// !!! IMPORTANT !!!
// Make sure that these keys are in the same order as the
// SettingsLoadWarnings/Errors enum is!
static const std::array<std::wstring_view, 3> settingsLoadWarningsLabels {
static const std::array<std::wstring_view, 4> settingsLoadWarningsLabels {
USES_RESOURCE(L"MissingDefaultProfileText"),
USES_RESOURCE(L"DuplicateProfileText"),
USES_RESOURCE(L"UnknownColorSchemeText")
USES_RESOURCE(L"UnknownColorSchemeText"),
USES_RESOURCE(L"BadActivePaneBorderColorValueText")
};
static const std::array<std::wstring_view, 2> settingsLoadErrorsLabels {
USES_RESOURCE(L"NoProfilesText"),
Expand Down
50 changes: 50 additions & 0 deletions src/cascadia/TerminalApp/CascadiaSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,17 @@ GlobalAppSettings& CascadiaSettings::GlobalSettings()
return _globals;
}

// Method Description:
// - Get a const reference to our global settings
// Arguments:
// - <none>
// Return Value:
// - a reference to our global settings
const GlobalAppSettings& CascadiaSettings::GlobalSettings() const
{
return _globals;
}

// Method Description:
// - Gets our list of warnings we found during loading. These are things that we
// knew were bad when we called `_ValidateSettings` last.
Expand Down Expand Up @@ -202,6 +213,9 @@ void CascadiaSettings::_ValidateSettings()
// TODO:GH#3522 With variable args to keybindings, it's possible that a user
// set a keybinding without all the required args for an action. Display a
// warning if an action didn't have a required arg.

// Validate that pane colors are either a color or "accent"
_ValidatePaneAccentColorValues();
}

// Method Description:
Expand Down Expand Up @@ -420,3 +434,39 @@ void CascadiaSettings::_ValidateAllSchemesExist()
_warnings.push_back(::TerminalApp::SettingsLoadWarnings::UnknownColorScheme);
}
}

// Method Description:
// - Ensures that the value set for "activePaneBorderColor" is a reasonable
// value. This should be one of:
// - `null`: to clear the active pane border color
// - `"accent"`:
// - a color string in format `"#RRGGBB"`
// - If the color is not set to one of these values, this will reset the value
// to null and display a warning.
// Arguments:
// - <none>
// Return Value:
// - <none>
// - Appends a SettingsLoadWarnings::BadActivePaneBorderColorValue to our list
// of warnings if the color is not an appropriate value.
void CascadiaSettings::_ValidatePaneAccentColorValues()
{
// The only real case we need to check here is if the pane border color was
// set, but it wasn't set to "accent", nor was it otherwise able to be
// parsed.
if (_globals.HasPaneFocusBorderColor() && !_globals.IsPaneFocusColorAccentColor())
{
// Try parsing the color. If we fail to parse the color, then add the
// warning. Otherwise, the setting is good to go.
try
{
const auto color = _globals.GetPaneFocusColor();
color;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment why you did this or use UNREFERENCED_PARAMETER macro to make it clear.

}
catch (...)
{
_warnings.push_back(::TerminalApp::SettingsLoadWarnings::BadActivePaneBorderColorValue);
_globals.SetPaneFocusColor(std::nullopt);
}
}
}
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/CascadiaSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class TerminalApp::CascadiaSettings final
winrt::Microsoft::Terminal::Settings::TerminalSettings MakeSettings(std::optional<GUID> profileGuid) const;

GlobalAppSettings& GlobalSettings();
const GlobalAppSettings& GlobalSettings() const;

std::basic_string_view<Profile> GetProfiles() const noexcept;

Expand Down Expand Up @@ -105,6 +106,7 @@ class TerminalApp::CascadiaSettings final
void _ReorderProfilesToMatchUserSettingsOrder();
void _RemoveHiddenProfiles();
void _ValidateAllSchemesExist();
void _ValidatePaneAccentColorValues();

friend class TerminalAppLocalTests::SettingsTests;
friend class TerminalAppLocalTests::ProfileTests;
Expand Down
55 changes: 55 additions & 0 deletions src/cascadia/TerminalApp/GlobalAppSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ static constexpr std::wstring_view MaximizedLaunchModeValue{ L"maximized" };
static constexpr std::wstring_view LightThemeValue{ L"light" };
static constexpr std::wstring_view DarkThemeValue{ L"dark" };
static constexpr std::wstring_view SystemThemeValue{ L"system" };
static constexpr std::string_view ActivePaneBorderColorKey{ "activePaneBorderColor" };
static constexpr std::wstring_view UseAccentColorValue{ L"accent" };

GlobalAppSettings::GlobalAppSettings() :
_keybindings{ winrt::make_self<winrt::TerminalApp::implementation::AppKeyBindings>() },
Expand Down Expand Up @@ -277,6 +279,8 @@ void GlobalAppSettings::LayerJson(const Json::Value& json)
{
_keybindings->LayerJson(keybindings);
}

JsonUtils::GetOptionalString(json, ActivePaneBorderColorKey, _activePaneBorderColor);
}

// Method Description:
Expand Down Expand Up @@ -442,3 +446,54 @@ void GlobalAppSettings::AddColorScheme(ColorScheme scheme)
std::wstring name{ scheme.GetName() };
_colorSchemes[name] = std::move(scheme);
}

// Method Description:
// - Returns true if the user has set the pane border to an actual value. If
// they've set the active pane border color to `null`, then this will return
// false.
// Arguments:
// - <none>
// Return Value:
// - true iff there's a active pane border color set.
bool GlobalAppSettings::HasPaneFocusBorderColor() const noexcept
{
return _activePaneBorderColor.has_value();
}

// Method Description:
// - Returns true if the user has set the pane border to the system accent
// color, using the value "accent".
// Arguments:
// - <none>
// Return Value:
// - true iff the active pane border color is set to "accent"
bool GlobalAppSettings::IsPaneFocusColorAccentColor() const noexcept
{
return HasPaneFocusBorderColor() && _activePaneBorderColor.value() == UseAccentColorValue;
}

// Method Description:
// - Retrieve the color the user has set the pane border to. If the user has set
// the color to "accent", or set the color to "null", or any other invalid
// string, this method will throw an exception.
// Arguments:
// - <none>
// Return Value:
// - the parsed color the user has set the active pane border to.
COLORREF GlobalAppSettings::GetPaneFocusColor() const
{
return ::Microsoft::Console::Utils::ColorFromHexString(_activePaneBorderColor.value());
}

// Method Description:
// - Sets the active pane border value. Used by
// CascadiaSettings::_ValidatePaneAccentColorValues to reset the active pane
// border color, in case the color was set to something invalid.
// Arguments:
// - newValue: the new string to use for this setting, or nullopt to clear the value.
// Return Value:
// - <none>
void GlobalAppSettings::SetPaneFocusColor(std::optional<std::wstring> newValue)
{
_activePaneBorderColor = newValue;
}
7 changes: 6 additions & 1 deletion src/cascadia/TerminalApp/GlobalAppSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ class TerminalApp::GlobalAppSettings final

void ApplyToSettings(winrt::Microsoft::Terminal::Settings::TerminalSettings& settings) const noexcept;

bool HasPaneFocusBorderColor() const noexcept;
bool IsPaneFocusColorAccentColor() const noexcept;
COLORREF GetPaneFocusColor() const;
void SetPaneFocusColor(std::optional<std::wstring> newValue);

private:
GUID _defaultProfile;
winrt::com_ptr<winrt::TerminalApp::implementation::AppKeyBindings> _keybindings;
Expand All @@ -96,8 +101,8 @@ class TerminalApp::GlobalAppSettings final
std::wstring _wordDelimiters;
bool _copyOnSelect;
winrt::Windows::UI::Xaml::ElementTheme _requestedTheme;

winrt::TerminalApp::LaunchMode _launchMode;
std::optional<std::wstring> _activePaneBorderColor{ std::nullopt };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of the rest of these private variables are initialized here. Are they initialized in the constructor instead?


static winrt::Windows::UI::Xaml::ElementTheme _ParseTheme(const std::wstring& themeString) noexcept;
static std::wstring_view _SerializeTheme(const winrt::Windows::UI::Xaml::ElementTheme theme) noexcept;
Expand Down
67 changes: 50 additions & 17 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "Pane.h"
#include "Profile.h"
#include "CascadiaSettings.h"
#include "../../WinRTUtils/inc/Utils.h"

#include "winrt/Microsoft.Terminal.TerminalConnection.h"

Expand Down Expand Up @@ -545,6 +546,13 @@ void Pane::UpdateSettings(const TerminalSettings& settings, const GUID& profile)
{
_control.UpdateSettings(settings);
}

_root.Dispatcher().RunAsync(CoreDispatcherPriority::Low, [this]() {
// Call _SetupResources to potentially reload the active pane brush,
// and UpdateVisuals to apply the brush
_SetupResources();
UpdateVisuals();
});
Comment on lines +550 to +555
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a good use for the co_await pattern.

}
}

Expand Down Expand Up @@ -1111,36 +1119,61 @@ void Pane::_ControlGotFocusHandler(winrt::Windows::Foundation::IInspectable cons
void Pane::_SetupResources()
{
const auto res = Application::Current().Resources();
const auto accentColorKey = winrt::box_value(L"SystemAccentColor");
if (res.HasKey(accentColorKey))

// First setup the pane border TabViewBackground color
const auto tabViewBackgroundKey = winrt::box_value(L"TabViewBackground");
if (res.HasKey(tabViewBackgroundKey))
{
const auto colorFromResources = res.Lookup(accentColorKey);
// If SystemAccentColor is _not_ a Color for some reason, use
// Transparent as the color, so we don't do this process again on
// the next pane (by leaving s_focusedBorderBrush nullptr)
auto actualColor = winrt::unbox_value_or<Color>(colorFromResources, Colors::Black());
s_focusedBorderBrush = SolidColorBrush(actualColor);
winrt::Windows::Foundation::IInspectable obj = res.Lookup(tabViewBackgroundKey);
s_unfocusedBorderBrush = obj.try_as<winrt::Windows::UI::Xaml::Media::SolidColorBrush>();
}
else
{
// DON'T use Transparent here - if it's "Transparent", then it won't
// be able to hittest for clicks, and then clicking on the border
// will eat focus.
s_focusedBorderBrush = SolidColorBrush{ Colors::Black() };
s_unfocusedBorderBrush = SolidColorBrush{ Colors::Black() };
}

const auto tabViewBackgroundKey = winrt::box_value(L"TabViewBackground");
if (res.HasKey(accentColorKey))
// Now try and set the pane border color.
// - If it's null, we'll use the TabViewBackground color.
// - if it's "accent", we'll use the accent color
// - if it's "#rrbbgg", we'll use the given color
const auto& settings = CascadiaSettings::GetCurrentAppSettings();
const auto& globals = settings.GlobalSettings();
if (!globals.HasPaneFocusBorderColor())
{
winrt::Windows::Foundation::IInspectable obj = res.Lookup(tabViewBackgroundKey);
s_unfocusedBorderBrush = obj.try_as<winrt::Windows::UI::Xaml::Media::SolidColorBrush>();
s_focusedBorderBrush = s_unfocusedBorderBrush;
}
else
{
// DON'T use Transparent here - if it's "Transparent", then it won't
// be able to hittest for clicks, and then clicking on the border
// will eat focus.
s_unfocusedBorderBrush = SolidColorBrush{ Colors::Black() };
if (globals.IsPaneFocusColorAccentColor())
{
// Use the accent color for the pane border

const auto accentColorKey = winrt::box_value(L"SystemAccentColor");
if (res.HasKey(accentColorKey))
{
const auto colorFromResources = res.Lookup(accentColorKey);
// If SystemAccentColor is _not_ a Color for some reason, use
// Transparent as the color, so we don't do this process again on
// the next pane (by leaving s_focusedBorderBrush nullptr)
auto actualColor = winrt::unbox_value_or<Color>(colorFromResources, Colors::Black());
s_focusedBorderBrush = SolidColorBrush(actualColor);
}
else
{
// DON'T use Transparent here - see earlier comment for why
s_focusedBorderBrush = s_unfocusedBorderBrush;
}
}
else
{
// Create a brush for the color the user specified
const COLORREF focusColor = globals.GetPaneFocusColor();
s_focusedBorderBrush = Media::SolidColorBrush{};
s_focusedBorderBrush.Color(ColorRefToColor(focusColor));
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@
</data>
<data name="UnknownColorSchemeText" xml:space="preserve">
<value>Found a profile with an invalid "colorScheme". Defaulting that profile to the default colors. Make sure that when setting a "colorScheme", the value matches the "name" of a color scheme in the "schemes" list.
</value>
</data>
<data name="BadActivePaneBorderColorValueText" xml:space="preserve">
<value>Found an invalid value for "activePaneBorderColor". This must either be set to null, "accent", or a hex color code in format "#RRGGBB". Defaulting to no color.
</value>
</data>
<data name="NoProfilesText" xml:space="preserve">
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/TerminalWarnings.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ namespace TerminalApp
{
MissingDefaultProfile = 0,
DuplicateProfile = 1,
UnknownColorScheme = 2
UnknownColorScheme = 2,
BadActivePaneBorderColorValue = 3
};

// SettingsLoadWarnings are scenarios where the settings had invalid state
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/defaults.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// THIS IS AN AUTO-GENERATED FILE! Changes to this file will be ignored.
{
"alwaysShowTabs": true,
"activePaneBorderColor": "accent",
"defaultProfile": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}",
"initialCols": 120,
"initialRows": 30,
Expand Down
3 changes: 2 additions & 1 deletion src/types/inc/utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ namespace Microsoft::Console::Utils
GUID CreateGuid();

std::string ColorToHexString(const COLORREF color);
COLORREF ColorFromHexString(const std::string wstr);
COLORREF ColorFromHexString(const std::string& wstr);
COLORREF ColorFromHexString(const std::wstring& wstr);

void InitializeCampbellColorTable(const gsl::span<COLORREF> table);
void InitializeCampbellColorTableForConhost(const gsl::span<COLORREF> table);
Expand Down
Loading