Skip to content
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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

GeekJosh
Copy link
Contributor

@GeekJosh GeekJosh commented Nov 9, 2024

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 is false (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 or false.

PR Checklist

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Task It's a feature request, but it doesn't really need a major design. Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. labels Nov 9, 2024

This comment has been minimized.

This comment has been minimized.

Copy link
Member

@carlos-zamora carlos-zamora left a 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!

@GeekJosh
Copy link
Contributor Author

Looks great! Thanks for doing this!

No problem at all, I enjoyed doing it 🙂

Thanks for the review

@GeekJosh GeekJosh force-pushed the dev/geekjosh/fullscreen_tab_visibility branch from c3dd1f8 to 2eea60b Compare November 16, 2024 16:14
@GeekJosh
Copy link
Contributor Author

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?

@zadjii-msft
Copy link
Member

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 ☺️

@zadjii-msft zadjii-msft assigned zadjii-msft and unassigned GeekJosh Nov 18, 2024
Copy link
Member

@zadjii-msft zadjii-msft left a 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 ☺️

@zadjii-msft zadjii-msft removed their assignment Nov 18, 2024
@zadjii-msft
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@DHowett DHowett left a 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!

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Dec 3, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Dec 10, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. labels Dec 14, 2024
@GeekJosh
Copy link
Contributor Author

GeekJosh commented Dec 14, 2024

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!

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:

hide_titlebar

And with the title bar not hidden (in settings at least 😅):

show_titlebar

@DHowett
Copy link
Member

DHowett commented Dec 14, 2024

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.

@GeekJosh
Copy link
Contributor Author

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 🙂

@GeekJosh GeekJosh requested a review from DHowett December 22, 2024 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No tabs in fullscreen, focus mode or not
4 participants