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.

Sign up for GitHub

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

Jump to bottom

Make the Pane inactive border respect the theme.window.applicationTheme #14486

Merged
2 commits merged into from
Dec 9, 2022
Merged

Make the Pane inactive border respect the theme.window.applicationTheme #14486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b1017b9
fixes #3917
zadjii-msft Dec 2, 2022
4f2a3a7
Update src/cascadia/TerminalApp/TerminalPage.cpp
zadjii-msft Dec 9, 2022
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
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/App.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@
Color="#2e2e2e" />

<StaticResource x:Key="UnfocusedBorderBrush"
ResourceKey="ApplicationPageBackgroundThemeBrush" />
ResourceKey="TabViewBackground" />

<SolidColorBrush x:Key="SettingsUiTabBrush"
Color="#0c0c0c" />
Expand All @@ -169,7 +169,7 @@
Color="#e8e8e8" />

<StaticResource x:Key="UnfocusedBorderBrush"
ResourceKey="ApplicationPageBackgroundThemeBrush" />
ResourceKey="TabViewBackground" />

<SolidColorBrush x:Key="SettingsUiTabBrush"
Color="#ffffff" />
Expand Down
21 changes: 9 additions & 12 deletions src/cascadia/TerminalApp/Pane.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include "Pane.h"
#include "AppLogic.h"

#include "Utils.h"

#include <Mmsystem.h>

using namespace winrt::Windows::Foundation;
Expand Down Expand Up @@ -45,14 +47,6 @@ Pane::Pane(const Profile& profile, const TermControl& control, const bool lastFo
_connectionStateChangedToken = _control.ConnectionStateChanged({ this, &Pane::_ControlConnectionStateChangedHandler });
_warningBellToken = _control.WarningBell({ this, &Pane::_ControlWarningBellHandler });

// On the first Pane's creation, lookup resources we'll use to theme the
// Pane, including the brushed to use for the focused/unfocused border
// color.
if (s_focusedBorderBrush == nullptr || s_unfocusedBorderBrush == nullptr)
{
_SetupResources();
}

// Register an event with the control to have it inform us when it gains focus.
_gotFocusRevoker = _control.GotFocus(winrt::auto_revoke, { this, &Pane::_ControlGotFocusHandler });
_lostFocusRevoker = _control.LostFocus(winrt::auto_revoke, { this, &Pane::_ControlLostFocusHandler });
Expand Down Expand Up @@ -3077,16 +3071,16 @@ float Pane::_ClampSplitPosition(const bool widthOrHeight, const float requestedV
// * The Brush we'll use for inactive Panes - TabViewBackground (to match the
// color of the titlebar)
// Arguments:
// - <none>
// - requestedTheme: this should be the currently active Theme for the app
// Return Value:
// - <none>
void Pane::_SetupResources()
void Pane::SetupResources(const winrt::Windows::UI::Xaml::ElementTheme& requestedTheme)
{
const auto res = Application::Current().Resources();
const auto accentColorKey = winrt::box_value(L"SystemAccentColor");
if (res.HasKey(accentColorKey))
{
const auto colorFromResources = res.Lookup(accentColorKey);
const auto colorFromResources = ThemeLookup(res, requestedTheme, 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)
Expand All @@ -3104,7 +3098,10 @@ void Pane::_SetupResources()
const auto unfocusedBorderBrushKey = winrt::box_value(L"UnfocusedBorderBrush");
if (res.HasKey(unfocusedBorderBrushKey))
{
auto obj = res.Lookup(unfocusedBorderBrushKey);
// MAKE SURE TO USE ThemeLookup, so that we get the correct resource for
// the requestedTheme, not just the value from the resources (which
// might not respect the settings' requested theme)
auto obj = ThemeLookup(res, requestedTheme, unfocusedBorderBrushKey);
s_unfocusedBorderBrush = obj.try_as<winrt::Windows::UI::Xaml::Media::SolidColorBrush>();
}
else
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/Pane.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ class Pane : public std::enable_shared_from_this<Pane>

bool ContainsReadOnly() const;

static void SetupResources(const winrt::Windows::UI::Xaml::ElementTheme& requestedTheme);

// Method Description:
// - A helper method for ad-hoc recursion on a pane tree. Walks the pane
// tree, calling a function on each pane in a depth-first pattern.
Expand Down Expand Up @@ -337,8 +339,6 @@ class Pane : public std::enable_shared_from_this<Pane>
return false;
}

static void _SetupResources();

struct PanePoint
{
float x;
Expand Down
16 changes: 16 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4175,6 +4175,22 @@ namespace winrt::TerminalApp::implementation
const auto theme = _settings.GlobalSettings().CurrentTheme();
auto requestedTheme{ theme.RequestedTheme() };

{
// Update the brushes that Pane's use...
Pane::SetupResources(requestedTheme);
Copy link
Member

Choose a reason for hiding this comment

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

Scary, updating globals like that, but I think I get how it is safe!

// ... then trigger a visual update for all the pane borders to
// apply the new ones.
for (const auto& tab : _tabs)
{
if (auto terminalTab{ _GetTerminalTabImpl(tab) })
{
terminalTab->GetRootPane()->WalkTree([&](auto&& pane) {
pane->UpdateVisuals();
});
}
}
}

const auto res = Application::Current().Resources();

// Use our helper to lookup the theme-aware version of the resource.
Expand Down