Enable changing the color of the active pane border #3752
Conversation
zadjii-msft
added
Area-Settings
zadjii-msft
requested review from
carlos-zamora,
DHowett-MSFT,
miniksa and
leonMSFT
| try | ||
| { | ||
| const auto color = _globals.GetPaneFocusColor(); | ||
| color; |
There was a problem hiding this comment.
comment why you did this or use UNREFERENCED_PARAMETER macro to make it clear.
| winrt::TerminalApp::LaunchMode _launchMode; | ||
| std::optional<std::wstring> _activePaneBorderColor{ std::nullopt }; |
There was a problem hiding this comment.
None of the rest of these private variables are initialized here. Are they initialized in the constructor instead?
| _root.Dispatcher().RunAsync(CoreDispatcherPriority::Low, [this]() { | ||
| // Call _SetupResources to potentially reload the active pane brush, | ||
| // and UpdateVisuals to apply the brush | ||
| _SetupResources(); | ||
| UpdateVisuals(); | ||
| }); |
There was a problem hiding this comment.
Probably a good use for the co_await pattern.
| // Return Value: | ||
| // - A COLORREF if the string could successfully be parsed. If the string is not | ||
| // the correct format, throws E_INVALIDARG | ||
| COLORREF Utils::ColorFromHexString(const std::wstring& str) |
There was a problem hiding this comment.
This has template potential.
Utils::ColorFromHexString<T>(const T& str).
ghost
added
the
Needs-Author-Feedback
|
@miniksa good catch, I forgot this was still open. I'm actually going to abandon this PR in favor of a more comprehensive solution to #3327. I've got a spec that I'm working on with @cinnamon-msft that should actually more holistically solve this feature. Thanks for the review though! |
ghost
removed
the
Needs-Author-Feedback
OK no problem. |
Summary of the Pull Request
Adds a new setting,
"activePaneBorderColor", which lets the user change the color of the active pane border. This setting can have 3 values:null: to clear the active pane border color (and have no color there)"accent": to use the accent color"#RRGGBB", to use that colorIf it's not one of those values, it will display a warning, and default to
null.PR Checklist