Skip to content

Commit

Permalink
7996: Always on Top setting does not persist (#8125)
Browse files Browse the repository at this point in the history
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request

<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> 
## References

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #7996
* [x] CLA signed.
* [ ] Documentation updated - irrelevant
* [ ] Schema updated - irrelevant
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
Currently the value of AlwaysOnTop is read by the AppHost from AppLogic that takes this value from the root TerminalPage. However at this stage neither AppLogic nor TerminalPage are initialized, and thus the return value is always false.

This PR introduces a "GetInitialAlwaysOnTop" method to AppLogic that returns a value that is configured in the settings.
In addition, the TerminalPage creation was fixed to read the configuration value upon creation (and not just after settings reload).

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
* Only manual testing
* Starting the system with both initial value set to true and false
* Verifying that dynamic toggling on / off is not affected
  • Loading branch information
Don-Vito authored Nov 2, 2020
1 parent d1e58bd commit 5b2fd70
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 1 deletion.
11 changes: 11 additions & 0 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,17 @@ namespace winrt::TerminalApp::implementation
return _settings.GlobalSettings().ShowTabsInTitlebar();
}

bool AppLogic::GetInitialAlwaysOnTop()
{
if (!_loadedInitialSettings)
{
// Load settings if we haven't already
LoadSettings();
}

return _settings.GlobalSettings().AlwaysOnTop();
}

// Method Description:
// - See Pane::CalcSnappedDimension
float AppLogic::CalcSnappedDimension(const bool widthOrHeight, const float dimension) const
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ namespace winrt::TerminalApp::implementation
winrt::Windows::UI::Xaml::ElementTheme GetRequestedTheme();
Microsoft::Terminal::Settings::Model::LaunchMode GetLaunchMode();
bool GetShowTabsInTitlebar();
bool GetInitialAlwaysOnTop();
float CalcSnappedDimension(const bool widthOrHeight, const float dimension) const;

Windows::UI::Xaml::UIElement GetRoot() noexcept;
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/AppLogic.idl
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ namespace TerminalApp
Windows.UI.Xaml.ElementTheme GetRequestedTheme();
Microsoft.Terminal.Settings.Model.LaunchMode GetLaunchMode();
Boolean GetShowTabsInTitlebar();
Boolean GetInitialAlwaysOnTop();
Single CalcSnappedDimension(Boolean widthOrHeight, Single dimension);
void TitlebarClicked();
void WindowCloseButtonClicked();
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ namespace winrt::TerminalApp::implementation
//
// _OnFirstLayout will remove this handler so it doesn't get called more than once.
_layoutUpdatedRevoker = _tabContent.LayoutUpdated(winrt::auto_revoke, { this, &TerminalPage::_OnFirstLayout });

_isAlwaysOnTop = _settings.GlobalSettings().AlwaysOnTop();
}

// Method Description:
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ AppHost::AppHost() noexcept :
std::placeholders::_1,
std::placeholders::_2));
_window->MouseScrolled({ this, &AppHost::_WindowMouseWheeled });
_window->SetAlwaysOnTop(_logic.AlwaysOnTop());
_window->SetAlwaysOnTop(_logic.GetInitialAlwaysOnTop());
_window->MakeWindow();
}

Expand Down

0 comments on commit 5b2fd70

Please sign in to comment.