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

Use standard 1px window borders on NC Island Window #3394

Merged
merged 36 commits into from
Nov 4, 2019
Merged

Use standard 1px window borders on NC Island Window #3394

merged 36 commits into from
Nov 4, 2019

Conversation

beviu
Copy link
Contributor

@beviu beviu commented Oct 31, 2019

Summary of the Pull Request

I put the standard Windows window borders on the Windows Terminal window (can you read that?).
(It's only on the left, bottom and right though, on the top I had to use a hack because of the title bar)

Light mode, color in title bar disabled:
frame

Light mode, color in title bar enabled:
frame color

Dark mode, color in title bar disabled:
frame dark

Dark mode, color in title bar enabled:
frame dark color

High contrast mode (with Settings app on right to compare):
high contrast

Note: the title bar should probably be purple like in Win32 apps but it isn't either in the UWP apps (Settings, classic Edge, …) so I don't really know.

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

We take the standard window frame except that we remove the top part (see NonClientIslandWindow::_OnNcCalcSize), then we put little 1 pixel wide top border back in the client area using DwmExtendFrameIntoClientArea and then we put the XAML island and the drag bar on top.

Most of this PR is comments to explain how the code works and also removing complex code that was needed to handle the weird cases when the borders were custom.

I've also refactored a little bit the NonClientIslandWindow class.

Validation Steps Performed

@beviu beviu changed the title Use standard window frame Use standard window borders Oct 31, 2019
@beviu
Copy link
Contributor Author

beviu commented Nov 1, 2019

With this change there is no resize handle directly on top of the tab row. This is the same behavior as classic Edge for reference (but it doesn't mean that it's good). This might be fixed by adding space on top of the tab row and excluding it from the XAML island region.

@miniksa
Copy link
Member

miniksa commented Nov 1, 2019

I don't know enough about the non-client area and XAML stuff to go through this. @DHowett-MSFT is the non-client expert and @zadjii-msft is the current UI king. So I'll let them stab first.

@musm
Copy link

musm commented Nov 6, 2019

bravo!

@ghost
Copy link

ghost commented Nov 26, 2019

🎉Windows Terminal Preview v0.7.3291.0 has been released which incorporates this pull request.:tada:

Handy links:

@Stanzilla
Copy link
Contributor

So...any way to color this new 1px border? I don't think there is a setting in Windows to color inactive window borders differently anymore, is there?

@adrianghc
Copy link

I'd be most comfortable merging this without the dark theme fix (which I know will cause there to be white borders around the entire window instead of just at the bottom) and then bringing in the dark theme changes when the other fix has flowed back down from Windows.

I couldn't find more details about this here, so does this mean the white border will be fixed with a new release or does the fix need to come from Windows, i.e. we would have to get 20H1 for the fix?

@beviu
Copy link
Contributor Author

beviu commented Nov 26, 2019

@Stanzilla The border behaves the same way as other windows. When it is active it will be either the accent color that you chose in Windows Settings if "Show accent color on the following surfaces -> Title bars and windows borders" is checked or a gray color otherwise. When it is inactive, it will always be gray. I don't think that there is a setting to color inactive window borders in Windows.

@adrianghc 20H1 changed the way to make the border dark. I think it is possible to detect if 20H1 is running and use the new way and if it's older than 20H1, use the old way to make the border dark so it could be fixed without waiting for 20H1 if this is done.

@Stanzilla
Copy link
Contributor

Well yeah, I don't really want to use my accent color on every window, was just wondering if there is a way to overwrite. But if that is black in 20H1 I'm okay with that

@danielniccoli
Copy link

danielniccoli commented Nov 27, 2019

In addition to the bottom white border, I now have 3 more borders, making the issue officially 4 times worse 😂

grafik

@michaelhthomas
Copy link

Yes indeed, the border in dark mode is still white I've noticed, which is quite bothersome to look at.
image

@Bebotron
Copy link

Version 0.9 has rolled out and my Windows Terminal still has this problem. Any ideas as to what the status of this issue is?
WT white borders
WT version

@beviu
Copy link
Contributor Author

beviu commented Feb 15, 2020

Yes, see #3425 and maybe PR #4577.

@Bebotron
Copy link

Thanks @greg904, so it sounds like its not a stable solution yet correct? Wondering if it might be best to sit tight a little longer until a new version of WT fixes this, or if it's worthwhile to attempt your solution in #4577

@beviu
Copy link
Contributor Author

beviu commented Feb 15, 2020

If you run the fix:

  • you are running the dev or master version which is not stable and can have bugs
  • on top of it you are running my fix which is not very tested (I made it quickly because I, too, want that bright border to go away :))

If you wait:

  • last release was 2 days ago so I'm guessing the new release won't come in a few days
  • maybe more stable

BTW, just to be clear:

With the fix, if you disable:
setting

You get this:
image

And if you enable it, you will have a border but now it will be blue:
image

So there still is a blue border but it is just like other apps:
image

@beviu beviu deleted the better-window-frame branch March 1, 2020 19:24
@zadjii-msft zadjii-msft mentioned this pull request May 4, 2022
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment