Add the ability to show the tab bar in fullscreen #18171
Conversation
microsoft-github-policy-service
bot
added
Issue-Task
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
No problem at all, I enjoyed doing it 🙂 Thanks for the review |
| @@ -337,6 +338,7 @@ void AppHost::Initialize() | |||
| _revokers.FullscreenChanged = _windowLogic.FullscreenChanged(winrt::auto_revoke, { this, &AppHost::_FullscreenChanged }); | |||
| _revokers.FocusModeChanged = _windowLogic.FocusModeChanged(winrt::auto_revoke, { this, &AppHost::_FocusModeChanged }); | |||
| _revokers.AlwaysOnTopChanged = _windowLogic.AlwaysOnTopChanged(winrt::auto_revoke, { this, &AppHost::_AlwaysOnTopChanged }); | |||
| _revokers.ShowTabsFullscreenChanged = _windowLogic.ShowTabsFullscreenChanged(winrt::auto_revoke, { this, &AppHost::_ShowTabsFullscreenChanged }); | |||
There was a problem hiding this comment.
qq: rather than adding another specific event for this, could we just use the existing _requestUpdateSettings handler to update this value when the settings change? I'm not sure the value would ever change outside of a settings change. If that works, you can probably get rid of a lot of plumbing
There was a problem hiding this comment.
you're absolutely right, there's no need for this additional event - removed 😊
GeekJosh
force-pushed
the
dev/geekjosh/fullscreen_tab_visibility
branch
from
c3dd1f8
to
2eea60b
Compare
|
I'm not sure what's causing the Release/x86 build failure here; I can build locally without any issues. I did a bit of searching and found that there was some issue with the pipeline previously which resulted in the same error: #14581 In that thread, @zadjii-msft found that the GUID used for a dependency project of the TerminalAzBridge project was wrong (PR: #15008) and, sure enough, that's the project which is failing to build for this PR too. I can't see that anything has changed recently in that project, nor any recent changes to the solution (other than the addition of a new project), so not sure how to proceed from here? |
|
Thanks for digging in! Alas, I think you just found one of our various transient build failures. The "circular dependency" thing crops up once every 25 builds or so. You just got unlucky 🤷 I kicked the build though, so hopefully it'll pop up green this time |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Summary of the Pull Request
This PR allows users to enable the tab bar in fullscreen mode.
References and Relevant Issues
This PR is in response to #11130
Detailed Description of the Pull Request / Additional comments
A new setting; "showTabsFullscreen"; has been added which accepts a boolean value. When
true, then the tab bar will remain visible when the terminal app is fullscreen. If the value isfalse(default), then the tab bar is hidden in fullscreen.When the tab bar is visible in fullscreen, the min/max/close controls are hidden to maintain the expected behaviour of a fullscreen app.
Validation Steps Performed
All unit tests are passing.
Manually verified that when the "launchMode" setting is "fullscreen" and the "showTabsFullscreen" setting is
true, the tab bar is visible on launch.Manually verified that changing the setting at runtime causes the tab bar to be shown/hidden immediately (if the terminal is currently fullscreen).
Manually verified that the new "showTabsFullscreen" setting is honoured regardless of whether "showTabsInTitlebar" is set to
trueorfalse.PR Checklist