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 switching app theme based off of OS theme #14497

Merged
40 commits merged into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
3d5d88a
inintial buggy fix for syncing OS themes
Sep 23, 2022
5f54750
fixed control preview bug
Oct 3, 2022
43cc9fa
Code cleanup
Oct 3, 2022
e4c2d20
Code cleanup
Oct 3, 2022
b1c5342
updated bug, where custom themes would not update the color scheme
Oct 10, 2022
e4f3625
updated bug, where custom themes would not update the color scheme
Oct 11, 2022
c16b998
PR lhecker comments
Oct 13, 2022
4e40eff
formatting fix
Oct 19, 2022
367d38c
formatting fix
Oct 20, 2022
fdc2c80
Merge remote-tracking branch 'origin/main' into ThemeControlledColorS…
zadjii-msft Nov 14, 2022
8e08d75
I think this is going to be way easier to manage
zadjii-msft Nov 15, 2022
a0e28f1
UI is better. Not good, but better
zadjii-msft Nov 15, 2022
b64b032
hot reloading
zadjii-msft Nov 15, 2022
1b81da9
cleanup some todos
zadjii-msft Nov 15, 2022
d71c56e
schema too folks
zadjii-msft Nov 15, 2022
e70ac18
goodbye LoadSettings, ReloadSettings is my best friend now
zadjii-msft Nov 15, 2022
11460d6
yep. This is just as dumb as it seems
zadjii-msft Nov 16, 2022
b969353
tests man, tests (thread)
zadjii-msft Nov 16, 2022
c823fe6
these weren't me!
zadjii-msft Nov 16, 2022
4e51484
Merge remote-tracking branch 'origin/main' into ThemeControlledColorS…
zadjii-msft Nov 30, 2022
9f251dd
Merge remote-tracking branch 'origin/main' into ThemeControlledColorS…
zadjii-msft Dec 1, 2022
1260705
remove this scheme
zadjii-msft Dec 1, 2022
109ecd9
Merge remote-tracking branch 'origin/main' into ThemeControlledColorS…
zadjii-msft Dec 2, 2022
1aa3675
first off, plumbing
zadjii-msft Dec 2, 2022
a14574a
secondly, this is _awesome
zadjii-msft Dec 2, 2022
fc6ef60
Just a comment
zadjii-msft Dec 2, 2022
ef6f660
Merge remote-tracking branch 'origin/main' into ThemeControlledColorS…
zadjii-msft Dec 6, 2022
c56a424
yea I dig it
zadjii-msft Dec 6, 2022
156e952
Merge branch 'ThemeControlledColorSchemeSwitch' into dev/migrie/f/the…
zadjii-msft Dec 6, 2022
77d00ee
prep for review
zadjii-msft Dec 6, 2022
a10dfe4
spel
zadjii-msft Dec 6, 2022
14f38f3
Merge remote-tracking branch 'origin/main' into dev/migrie/f/theme-th…
zadjii-msft Dec 6, 2022
9ff12c8
Merge remote-tracking branch 'origin/main' into dev/migrie/f/theme-th…
zadjii-msft Dec 8, 2022
f11220f
dead code and spel
zadjii-msft Dec 8, 2022
0fac998
Merge remote-tracking branch 'origin/main' into dev/migrie/f/theme-th…
zadjii-msft Dec 9, 2022
85d6f92
why DONT we just not
zadjii-msft Dec 9, 2022
c176c71
Merge remote-tracking branch 'origin/main' into dev/migrie/f/theme-th…
zadjii-msft Dec 9, 2022
40afe5e
Merge remote-tracking branch 'origin/main' into dev/migrie/f/theme-th…
zadjii-msft Dec 12, 2022
bc1b385
Merge remote-tracking branch 'origin/main' into dev/migrie/f/theme-th…
zadjii-msft Jan 12, 2023
7666478
minor nits
zadjii-msft Jan 12, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
if (const auto& theme{ tag.try_as<Model::Theme>() })
{
_GlobalSettings.Theme(theme.Name());
_GlobalSettings.Theme(Model::ThemePair{ theme.Name() });
Copy link
Member

Choose a reason for hiding this comment

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

Clever

}
}

Expand Down
34 changes: 27 additions & 7 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1167,7 +1167,8 @@ void CascadiaSettings::ExportFile(winrt::hstring path, winrt::hstring content)

void CascadiaSettings::_validateThemeExists()
{
if (_globals->Themes().Size() == 0)
const auto& themes{ _globals->Themes() };
if (themes.Size() == 0)
{
// We didn't even load the default themes. This should only be possible
// if the defaults.json didn't include any themes, or if no
Expand All @@ -1178,14 +1179,33 @@ void CascadiaSettings::_validateThemeExists()
auto newTheme = winrt::make_self<Theme>();
newTheme->Name(L"system");
_globals->AddTheme(*newTheme);
_globals->Theme(L"system");
_globals->Theme(Model::ThemePair{ L"system" });
}

if (!_globals->Themes().HasKey(_globals->Theme()))
const auto& theme{ _globals->Theme() };
if (theme.DarkName() == theme.LightName())
{
_warnings.Append(SettingsLoadWarnings::UnknownTheme);

// safely fall back to system as the theme.
_globals->Theme(L"system");
// Only one theme. We'll treat it as such.
if (!themes.HasKey(theme.DarkName()))
{
_warnings.Append(SettingsLoadWarnings::UnknownTheme);
// safely fall back to system as the theme.
_globals->Theme(*winrt::make_self<ThemePair>(L"system"));
}
}
else
{
// Two different themes. Check each separately, and fall back to a
// reasonable default contextually
if (!themes.HasKey(theme.LightName()))
{
_warnings.Append(SettingsLoadWarnings::UnknownTheme);
theme.LightName(L"light");
}
if (!themes.HasKey(theme.DarkName()))
{
_warnings.Append(SettingsLoadWarnings::UnknownTheme);
theme.DarkName(L"dark");
}
}
}
18 changes: 17 additions & 1 deletion src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "../../types/inc/Utils.hpp"
#include "JsonUtils.h"
#include "KeyChordSerialization.h"
#include "SettingsUtils.h"

#include "GlobalAppSettings.g.cpp"

Expand Down Expand Up @@ -212,7 +213,22 @@ Json::Value GlobalAppSettings::ToJson() const

winrt::Microsoft::Terminal::Settings::Model::Theme GlobalAppSettings::CurrentTheme() noexcept
{
return _themes.TryLookup(Theme());
auto requestedTheme = IsSystemInDarkTheme() ?
winrt::Windows::UI::Xaml::ElementTheme::Dark :
winrt::Windows::UI::Xaml::ElementTheme::Light;

switch (requestedTheme)
{
case winrt::Windows::UI::Xaml::ElementTheme::Light:
return _themes.TryLookup(Theme().LightName());

case winrt::Windows::UI::Xaml::ElementTheme::Dark:
return _themes.TryLookup(Theme().DarkName());

case winrt::Windows::UI::Xaml::ElementTheme::Default:
default:
return nullptr;
}
}

void GlobalAppSettings::AddTheme(const Model::Theme& theme)
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ namespace Microsoft.Terminal.Settings.Model

Windows.Foundation.Collections.IMapView<String, Theme> Themes();
void AddTheme(Theme theme);
INHERITABLE_SETTING(String, Theme);
INHERITABLE_SETTING(ThemePair, Theme);
Theme CurrentTheme { get; };
}
}
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsModel/MTSMSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Author(s):
X(Model::NewTabPosition, NewTabPosition, "newTabPosition", Model::NewTabPosition::AfterLastTab) \
X(bool, ShowTitleInTitlebar, "showTerminalTitleInTitlebar", true) \
X(bool, ConfirmCloseAllTabs, "confirmCloseAllTabs", true) \
X(hstring, Theme, "theme") \
X(Model::ThemePair, Theme, "theme") \
X(hstring, Language, "language") \
X(winrt::Microsoft::UI::Xaml::Controls::TabViewWidthMode, TabWidthMode, "tabWidthMode", winrt::Microsoft::UI::Xaml::Controls::TabViewWidthMode::Equal) \
X(bool, UseAcrylicInTabRow, "useAcrylicInTabRow", false) \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
<ClInclude Include="WslDistroGenerator.h" />
<ClInclude Include="SshHostGenerator.h" />
<ClInclude Include="ModelSerializationHelpers.h" />
<ClInclude Include="SettingsUtils.h" />
</ItemGroup>
<!-- ========================= Cpp Files ======================== -->
<ItemGroup>
Expand Down
6 changes: 6 additions & 0 deletions src/cascadia/TerminalSettingsModel/SettingsUtils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Copyright (c) Microsoft Corporation.
Copy link
Member

Choose a reason for hiding this comment

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

Curious, why is this a separate file? When, in the future, should I add something here instead of, say, Terminal Settings?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just a utils header - there was no good place for shared code in the settings binary. It didn't compile in the WinRTUtils binary (cause dependencies on Windows.UI.Xaml or something), so I just stuck it here

Copy link
Member

Choose a reason for hiding this comment

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

I feel like this might be good opportunity to rename FileUtils to just Utils and put it there.

// Licensed under the MIT license.

#pragma once

bool IsSystemInDarkTheme();
23 changes: 12 additions & 11 deletions src/cascadia/TerminalSettingsModel/TerminalSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,21 @@

#include "TerminalSettings.g.cpp"
#include "TerminalSettingsCreateResult.g.cpp"
#include "SettingsUtils.h"

using namespace winrt::Microsoft::Terminal::Control;
using namespace Microsoft::Console::Utils;

// I'm not even joking, this is the recommended way to do this:
// https://learn.microsoft.com/en-us/windows/apps/desktop/modernize/apply-windows-themes#know-when-dark-mode-is-enabled
bool IsSystemInDarkTheme()
{
static auto isColorLight = [](const winrt::Windows::UI::Color& clr) -> bool {
return (((5 * clr.G) + (2 * clr.R) + clr.B) > (8 * 128));
};
return isColorLight(winrt::Windows::UI::ViewManagement::UISettings().GetColorValue(winrt::Windows::UI::ViewManagement::UIColorType::Foreground));
};

namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
static std::tuple<Windows::UI::Xaml::HorizontalAlignment, Windows::UI::Xaml::VerticalAlignment> ConvertConvergedAlignment(ConvergedAlignment alignment)
Expand Down Expand Up @@ -183,16 +194,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
return settingsPair;
}

// I'm not even joking, this is the recommended way to do this:
// https://learn.microsoft.com/en-us/windows/apps/desktop/modernize/apply-windows-themes#know-when-dark-mode-is-enabled
bool _isSystemInDarkTheme()
{
static auto isColorLight = [](const Windows::UI::Color& clr) -> bool {
return (((5 * clr.G) + (2 * clr.R) + clr.B) > (8 * 128));
};
return isColorLight(Windows::UI::ViewManagement::UISettings().GetColorValue(Windows::UI::ViewManagement::UIColorType::Foreground));
}

void TerminalSettings::_ApplyAppearanceSettings(const IAppearanceConfig& appearance,
const Windows::Foundation::Collections::IMapView<winrt::hstring, ColorScheme>& schemes,
const winrt::Microsoft::Terminal::Settings::Model::Theme currentTheme)
Expand All @@ -203,7 +204,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
auto requestedTheme = currentTheme.RequestedTheme();
if (requestedTheme == winrt::Windows::UI::Xaml::ElementTheme::Default)
{
requestedTheme = _isSystemInDarkTheme() ?
requestedTheme = IsSystemInDarkTheme() ?
winrt::Windows::UI::Xaml::ElementTheme::Dark :
winrt::Windows::UI::Xaml::ElementTheme::Light;
}
Expand Down
42 changes: 42 additions & 0 deletions src/cascadia/TerminalSettingsModel/Theme.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "WindowTheme.g.cpp"
#include "TabRowTheme.g.cpp"
#include "TabTheme.g.cpp"
#include "ThemePair.g.cpp"
#include "Theme.g.cpp"

using namespace ::Microsoft::Console;
Expand All @@ -27,6 +28,8 @@ namespace winrt
}

static constexpr std::string_view NameKey{ "name" };
static constexpr std::string_view LightNameKey{ "light" };
static constexpr std::string_view DarkNameKey{ "dark" };

static constexpr wchar_t RegKeyDwm[] = L"Software\\Microsoft\\Windows\\DWM";
static constexpr wchar_t RegKeyAccentColor[] = L"AccentColor";
Expand Down Expand Up @@ -320,3 +323,42 @@ winrt::WUX::ElementTheme Theme::RequestedTheme() const noexcept
{
return _Window ? _Window.RequestedTheme() : winrt::WUX::ElementTheme::Default;
}

winrt::com_ptr<ThemePair> ThemePair::FromJson(const Json::Value& json)
{
auto result = winrt::make_self<ThemePair>(L"dark");

if (json.isString())
{
result->_DarkName = result->_LightName = JsonUtils::GetValue<winrt::hstring>(json);
Copy link
Member

Choose a reason for hiding this comment

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

"Is that legal?" meme

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, operator= returns a reference to this. But I wouldn't use it and write separate statements instead.

}
else if (json.isObject())
{
JsonUtils::GetValueForKey(json, DarkNameKey, result->_DarkName);
JsonUtils::GetValueForKey(json, LightNameKey, result->_LightName);
}
return result;
}

Json::Value ThemePair::ToJson() const
{
if (_DarkName == _LightName)
{
return JsonUtils::ConversionTrait<winrt::hstring>().ToJson(DarkName());
}
else
{
Json::Value json{ Json::ValueType::objectValue };

JsonUtils::SetValueForKey(json, DarkNameKey, _DarkName);
JsonUtils::SetValueForKey(json, LightNameKey, _LightName);
return json;
}
}
winrt::com_ptr<ThemePair> ThemePair::Copy() const
{
auto pair{ winrt::make_self<ThemePair>() };
pair->_DarkName = _DarkName;
pair->_LightName = _LightName;
return pair;
}
51 changes: 51 additions & 0 deletions src/cascadia/TerminalSettingsModel/Theme.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,33 @@ Author(s):
#include "WindowTheme.g.h"
#include "TabRowTheme.g.h"
#include "TabTheme.g.h"
#include "ThemePair.g.h"
#include "Theme.g.h"

#include "JsonUtils.h"

namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
struct ThemePair : ThemePairT<ThemePair>
{
public:
ThemePair() = default;
explicit ThemePair(const winrt::hstring& name) noexcept :
_DarkName{ name },
_LightName{ name } {};

explicit ThemePair(const winrt::hstring& lightName, const winrt::hstring& darkName) noexcept :
_DarkName{ darkName },
_LightName{ lightName } {};

static com_ptr<ThemePair> FromJson(const Json::Value& json);
Json::Value ToJson() const;
com_ptr<ThemePair> Copy() const;

WINRT_PROPERTY(winrt::hstring, DarkName);
WINRT_PROPERTY(winrt::hstring, LightName);
};

struct ThemeColor : ThemeColorT<ThemeColor>
{
public:
Expand Down Expand Up @@ -92,4 +115,32 @@ namespace winrt::Microsoft::Terminal::Settings::Model::factory_implementation
{
BASIC_FACTORY(ThemeColor);
BASIC_FACTORY(Theme);
BASIC_FACTORY(ThemePair);
}

namespace Microsoft::Terminal::Settings::Model::JsonUtils
{
template<>
struct ConversionTrait<winrt::Microsoft::Terminal::Settings::Model::ThemePair>
{
winrt::Microsoft::Terminal::Settings::Model::ThemePair FromJson(const Json::Value& json)
{
return *winrt::Microsoft::Terminal::Settings::Model::implementation::ThemePair::FromJson(json);
}

bool CanConvert(const Json::Value& json) const
{
return json.isObject() || json.isString();
}

Json::Value ToJson(const winrt::Microsoft::Terminal::Settings::Model::ThemePair& val)
{
return winrt::get_self<winrt::Microsoft::Terminal::Settings::Model::implementation::ThemePair>(val)->ToJson();
}

std::string TypeDescription() const
{
return "ThemePair{ string, string }";
}
};
}
10 changes: 10 additions & 0 deletions src/cascadia/TerminalSettingsModel/Theme.idl
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ namespace Microsoft.Terminal.Settings.Model
Never
};

[default_interface] runtimeclass ThemePair
{
ThemePair();
ThemePair(String name);
ThemePair(String darkName, String lightName);

String DarkName;
String LightName;
};

runtimeclass ThemeColor
{
ThemeColor();
Expand Down