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 support for focus mode #6804

Merged
16 commits merged into from
Jul 13, 2020
Merged

Add support for focus mode #6804

16 commits merged into from
Jul 13, 2020

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Jul 6, 2020

Summary of the Pull Request

Add support for "focus" mode, which only displays the actual terminal content, no tabs or titlebar. The edges of the window are draggable to resize, but the window can't be moved in borderless mode.

The window looks slightly different bewteen different values for showTabsInTitlebar, because switching between the NonClientIslandWindow and the IslandWindow is hard.

showTabsInTitlebar Preview
true image
false image

PR Checklist

Detailed Description of the Pull Request / Additional comments

  • KNOWN ISSUE: Upon resizing the NCIW, the top frame margin disappears, making that border disappear entirely. 6356aaf has a bunch of WIP work for me trying to fix that, but I couldn't get it quite right.

Validation Steps Performed

  • Toggled between focus and fullscreen a bunch in both modes.

…rless in either mode. Now double check NCIW...
… an acceptable known issue, since the alternative is having a white line on top of the window when it loses focus
…ll-just-do-it-myself

# Conflicts:
#	src/cascadia/TerminalApp/ActionAndArgs.cpp
#	src/cascadia/TerminalApp/AppActionHandlers.cpp
#	src/cascadia/TerminalApp/ShortcutActionDispatch.cpp
#	src/cascadia/TerminalApp/ShortcutActionDispatch.h
#	src/cascadia/TerminalApp/ShortcutActionDispatch.idl
#	src/cascadia/TerminalApp/TerminalPage.cpp
#	src/cascadia/TerminalApp/TerminalPage.h
@ghost ghost added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal. labels Jul 6, 2020
Copy link
Member

@miniksa miniksa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handful of things I saw browsing through.

I know someone's going to whine that this isn't "truly" borderless because we're still going to have the 1px border around the window. It's more of frameless or chrome-less. It feels like a giant pain to change it all over, but now's probably the time to consider that before the setting is set.

(I know we briefly mentioned it in the sync meeting today, but I'm still worried about the "border" terminology.)

// - <none>
void NonClientIslandWindow::_SetIsBorderless(const bool borderlessEnabled)
{
_borderless = borderlessEnabled;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be testing on the way in that the state is actually changing before dispatching the window messages? Is there a way to set borderless = true on top of an already borderless window and dispatch a bunch of window messages that will make things jump around but end up back where they were in a weird way?

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.

I don't really have anything to add here. Really just everything Michael said. I guess I'll approve with suggestions? 🤷‍♂️

@WSLUser
Copy link
Contributor

WSLUser commented Jul 7, 2020

because we're still going to have the 1px border around the window

He's right. I think borderless and I think of how a few games appear that support the borderless mode (or game mods that enable the functionality). I doubt MUX is used (probably something in the DWRITE layer) so not sure how it's handled but might be worth investigating if we can get a truly borderless window. Otherwise I'd agree that calling it "chromeless" or "frameless" (preference) would be a better description.

Here's something that may help: https://stackoverflow.com/questions/43818022/borderless-window-with-drop-shadow Just ignore the GDI since this can still be mostly done using DirectX: https://docs.microsoft.com/en-us/windows/win32/direct3d12/directx-12-programming-guide

@zadjii-msft

This comment has been minimized.

@WSLUser
Copy link
Contributor

WSLUser commented Jul 7, 2020

I would actually prefer completely borderless, usually there are some perf bonuses when switching between applications though not sure if that would be the case here. I do agree we'd need resizing and drop shadows. Any way we could do it in terminal/src/renderer/dx instead? Most of that SO code could likely be re-used but would require some plumbing to hook into the terminal settings (which we sort of already do for the retro terminal effect). I think it should be a bit more straightforward and less effort than attempting to do it in the Island Window.

@zadjii-msft
Copy link
Member Author

It would be infinitely harder to do it in the DX layer. The DX layer is fully encapsulated in the SwapChainPanel, which is within the bounds of the XAML island. It's exclusively used for drawing the contents of a single terminal instance, and doesn't know anything about the window that's hosting it. We're definitely going to need to do this at a higher level than that.

We used to do the whole DwmExtendFrameIntoClient dance before, and it was a disaster that I absolutely hated, and the NCIW is way better off without it. I'm highly reluctant to reintroduce it lightly, even for something like this.

@WSLUser
Copy link
Contributor

WSLUser commented Jul 7, 2020

In that case, I'd say for now we just call it frameless mode and file a follow on to to get full borderless with resize and shadows still working (and throw a "Help Wanted" label on it).

@DHowett
Copy link
Member

DHowett commented Jul 7, 2020

I’m not sure the use case for actual borderless mode really exists. It sounds like a strawman. :)

This one feels to me like a floating or detached mode. I’d rather we name it by what it accomplishes rather than how it accomplishes it...

@carlos-zamora
Copy link
Member

@zadjii-msft Don't forget to update the docs and link the PR!

@zadjii-msft zadjii-msft changed the title Add support for borderless mode Add support for focus mode Jul 9, 2020
@DHowett
Copy link
Member

DHowett commented Jul 9, 2020

So, pursuant to 610c267 (#6804).. are we going with a mode that suppresses resize and drop shadows? I'm not sure I like that.

@zadjii-msft
Copy link
Member Author

So, pursuant to 610c267 (#6804).. are we going with a mode that suppresses resize and drop shadows? I'm not sure I like that.

Guh, I was supposed to revert that before pushing 🤦‍♂️

@WSLUser
Copy link
Contributor

WSLUser commented Jul 10, 2020

I hope one day we can enable that setting without losing resize and drop shadows.

@zadjii-msft
Copy link
Member Author

We're definitely doing the mode with the resizing and the drop shadows. I mistakenly didn't revert the commit where I was playing around with the "frameless" version.

@WSLUser
Copy link
Contributor

WSLUser commented Jul 10, 2020

We shouldn't have to work around that setting. It's a reasonable expectation that functionality won't drop simply due to enabling the setting. Glad we could work around it though.

@WSLUser
Copy link
Contributor

WSLUser commented Jul 10, 2020

I filed an feature request so we can more easily set borderless mode with the xaml setting. That would hopefully replace much of what's here with a simpler, more elegant solution.

@zadjii-msft
Copy link
Member Author

Wait what are you talking about? The borderless Focus Mode that's being implemented here is not "frameless, resizeless, shadowless" mode. It's exactly as in the OP. We're not doing anything with any sort of MUX or UWP borderless mode in this PR. We're just manually hiding the TabView. For the window with the Win32 titlebar, we're also setting some other window styles.

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.

changing to NAK so github logs it

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 10, 2020
@cinnamon-msft
Copy link
Contributor

For the default key binding, what do we think about Shift+F11?

@DHowett
Copy link
Member

DHowett commented Jul 11, 2020

With the command palette, I'm not sure we need a default key binding right now. We can see if one grows organically!

@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 13, 2020
@DHowett DHowett removed their assignment Jul 13, 2020
@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jul 13, 2020
@ghost
Copy link

ghost commented Jul 13, 2020

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 1c8e83d into master Jul 13, 2020
@ghost ghost deleted the dev/migrie/f/2238-ill-just-do-it-myself branch July 13, 2020 17:40
@ghost
Copy link

ghost commented Jul 22, 2020

🎉Windows Terminal Preview v1.2.2022.0 has been released which incorporates this pull request.:tada:

Handy links:

@NicTanghe
Copy link

O M G this is awsome 💯

This pull request was closed.
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 AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Border-/Titleless windows for displaying dynamic information with the Terminal
7 participants