Skip to content

Commit

Permalink
Fix opacity restore for command palette previews (#12229)
Browse files Browse the repository at this point in the history
This commit correctly restores the previous opacity when the command palette
preview is cancelled. It includes an additional change in order to make the
`AdjustOpacity` setter consistent and symmetric with the `Opacity` getter.

## PR Checklist
* [x] Closes #12228
* [x] I work here
* [x] Tests added/passed

## Validation Steps Performed

* Open Windows Terminal command palette
* Choose "Set background opacity..."
* Cycle through the items without selecting one
* Press Escape
* Previous opacity is restored ✅
  • Loading branch information
lhecker authored Jan 24, 2022
1 parent 27c4a84 commit 5258fea
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 26 deletions.
44 changes: 26 additions & 18 deletions src/cascadia/TerminalApp/ActionPreviewHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,14 @@ namespace winrt::TerminalApp::implementation
// Apply the reverts in reverse order - If we had multiple previews
// stacked on top of each other, then this will ensure the first one in
// is the last one out.
for (auto i{ _restorePreviewFuncs.rbegin() }; i < _restorePreviewFuncs.rend(); i++)
const auto cleanup = wil::scope_exit([this]() {
_restorePreviewFuncs.clear();
});

for (const auto& f : _restorePreviewFuncs)
{
auto f = *i;
f();
}
_restorePreviewFuncs.clear();
}

// Method Description:
Expand All @@ -94,41 +96,47 @@ namespace winrt::TerminalApp::implementation
{
if (const auto& scheme{ _settings.GlobalSettings().ColorSchemes().TryLookup(args.SchemeName()) })
{
const auto backup = _restorePreviewFuncs.empty();

_ApplyToActiveControls([&](const auto& control) {
// Stash a copy of the current scheme.
auto originalScheme{ control.ColorScheme() };

// Apply the new scheme.
control.ColorScheme(scheme.ToCoreScheme());

// Each control will emplace a revert into the
// _restorePreviewFuncs for itself.
_restorePreviewFuncs.emplace_back([=]() {
// On dismiss, restore the original scheme.
control.ColorScheme(originalScheme);
});
if (backup)
{
// Each control will emplace a revert into the
// _restorePreviewFuncs for itself.
_restorePreviewFuncs.emplace_back([=]() {
// On dismiss, restore the original scheme.
control.ColorScheme(originalScheme);
});
}
});
}
}

void TerminalPage::_PreviewAdjustOpacity(const Settings::Model::AdjustOpacityArgs& args)
{
// Clear the saved preview funcs because we don't need to add a restore each time
// the preview changes, we only need to be able to restore the last one.
_restorePreviewFuncs.clear();
const auto backup = _restorePreviewFuncs.empty();

_ApplyToActiveControls([&](const auto& control) {
// Stash a copy of the original opacity.
auto originalOpacity{ control.BackgroundOpacity() };

// Apply the new opacity
control.AdjustOpacity(args.Opacity(), args.Relative());
control.AdjustOpacity(args.Opacity() / 100.0, args.Relative());

_restorePreviewFuncs.emplace_back([=]() {
// On dismiss:
// Don't adjust relatively, just set outright.
control.AdjustOpacity(::base::saturated_cast<int>(originalOpacity * 100), false);
});
if (backup)
{
_restorePreviewFuncs.emplace_back([=]() {
// On dismiss:
// Don't adjust relatively, just set outright.
control.AdjustOpacity(originalOpacity, false);
});
}
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -970,7 +970,7 @@ namespace winrt::TerminalApp::implementation
if (const auto& realArgs = args.ActionArgs().try_as<AdjustOpacityArgs>())
{
const auto res = _ApplyToActiveControls([&](auto& control) {
control.AdjustOpacity(realArgs.Opacity(), realArgs.Relative());
control.AdjustOpacity(realArgs.Opacity() / 100.0, realArgs.Relative());
});
args.Handled(res);
}
Expand Down
3 changes: 1 addition & 2 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1718,9 +1718,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
return _settings->HasUnfocusedAppearance();
}

void ControlCore::AdjustOpacity(const int32_t& opacity, const bool& relative)
void ControlCore::AdjustOpacity(const double opacityAdjust, const bool relative)
{
const double opacityAdjust = static_cast<double>(opacity) / 100.0;
if (relative)
{
AdjustOpacity(opacityAdjust);
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/ControlCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

static bool IsVintageOpacityAvailable() noexcept;

void AdjustOpacity(const int32_t& opacity, const bool& relative);
void AdjustOpacity(const double opacity, const bool relative);

RUNTIME_SETTING(double, Opacity, _settings->Opacity());
RUNTIME_SETTING(bool, UseAcrylic, _settings->UseAcrylic());
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/ControlCore.idl
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ namespace Microsoft.Terminal.Control

String ReadEntireBuffer();

void AdjustOpacity(Int32 Opacity, Boolean relative);
void AdjustOpacity(Double Opacity, Boolean relative);

event FontSizeChangedEventArgs FontSizeChanged;

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2713,7 +2713,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_core.ColorScheme(scheme);
}

void TermControl::AdjustOpacity(const int32_t& opacity, const bool& relative)
void TermControl::AdjustOpacity(const double opacity, const bool relative)
{
_core.AdjustOpacity(opacity, relative);
}
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
winrt::Microsoft::Terminal::Core::Scheme ColorScheme() const noexcept;
void ColorScheme(const winrt::Microsoft::Terminal::Core::Scheme& scheme) const noexcept;

void AdjustOpacity(const int32_t& opacity, const bool& relative);
void AdjustOpacity(const double opacity, const bool relative);

// -------------------------------- WinRT Events ---------------------------------
// clang-format off
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/TermControl.idl
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ namespace Microsoft.Terminal.Control

String ReadEntireBuffer();

void AdjustOpacity(Int32 Opacity, Boolean relative);
void AdjustOpacity(Double Opacity, Boolean relative);

// You'd think this should just be "Opacity", but UIElement already
// defines an "Opacity", which we're actually not setting at all. We're
Expand Down

0 comments on commit 5258fea

Please sign in to comment.