-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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.
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
Add the ability to show the tab bar in fullscreen #18171
base: main
Are you sure you want to change the base?
Add the ability to show the tab bar in fullscreen #18171
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks for doing this!
No problem at all, I enjoyed doing it 🙂 Thanks for the review |
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this! This looks much better without the extra event
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this!
I didn't see any notes about how this interoperates with "Hide the title bar". I think it will work given the tab view visibility work in TabManagement.cpp
, but would you mind confirming that?
Thanks!
src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw
Outdated
Show resolved
Hide resolved
…resw Co-authored-by: Dustin L. Howett <[email protected]>
Co-authored-by: Dustin L. Howett <[email protected]>
Sorry for the delay in responding! Full screen tabs show with or without the title bar hidden - I did two screenshots to show it before realising they prove nothing as there is no title bar in full screen either way 🤦 Probably doesn't hurt to show what they look like anyway, so here are those screenshots, first with the tite bar hidden: And with the title bar not hidden (in settings at least 😅): |
LOL, thanks for jumping through my hoops. Sorry if I made you re-break the spell checker. As for workflow: we squash all pull requests, so you are free to merge or rebase as you see fit. We usually use merges for in-progress work, because even though it makes for an uglier history it keeps GitHub from destroying PR comments attached to specific files and lines. |
Thank you for explaining 🙂 |
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
true
orfalse
.PR Checklist