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

Window border width does not respect system theme #1859

Closed
c0d3h4x0r opened this issue Jul 7, 2019 · 11 comments · Fixed by #3394
Closed

Window border width does not respect system theme #1859

c0d3h4x0r opened this issue Jul 7, 2019 · 11 comments · Fixed by #3394
Assignees
Labels
Area-User Interface 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. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@c0d3h4x0r
Copy link

Environment

Windows build number: 10.0.18362.207
Windows Terminal version (if applicable): 0.2.1831.0

Steps to reproduce

  1. Install the third-party Aerolite.theme to get window borders back to being usable (i.e. not zero-width).
  2. Note that Command Prompt, Windows PowerShell, and Ubuntu for Windows 10 command shell windows all respect the nice new border width.
  3. Launch Windows Terminal.

Expected behavior

Windows Terminal should also respect the theme's window border width.

Actual behavior

Windows Terminal has a zero-width window border in defiance of the system theme, making it nearly impossible to resize.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jul 7, 2019
@zadjii-msft
Copy link
Member

Is there a way to change the width of the window borders without installing a third-party theme installer?

I'm going to guess that this is a subset of #1625, assuming that the border width is in fact a system theme property we can query

@zadjii-msft zadjii-msft added Area-User Interface 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. labels Jul 8, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jul 8, 2019
@c0d3h4x0r
Copy link
Author

That’s the wrong question to ask. The right question is, if every other app seems able to honor the third-party theme, why won’t Windows Terminal?

@DHowett-MSFT
Copy link
Contributor

DHowett-MSFT commented Jul 8, 2019

Unfortunately, it is the right question to ask. There's a lot of history here for us to unravel, the earliest part of which is "why is it so hard to draw in the titlebar" and the latest part of which is "now that we're drawing in the titlebar, why was it decided by the win32 kernel driver and desktop window compositor that we absolutely must handle all of this ourselves? it's like this system was never actually designed but instead organically grown."

We're a fairly simple UI with fairly simple requirements, but one of those requirements is "have a custom color in our title bar." Unfortunately, that means:

  • We need to draw the titlebar ourselves, in its entirety, which means:
  • We need to have the UI framework (here Xaml Islands) go edge-to-edge, which means:
  • we need to draw our own window borders
  • we need to draw our own caption buttons
  • we need to account for the theme part size of the window's elements
  • we lose access to DWM shadows
    • dwm shadows are a trick, the window border is actually 8px wide and the compositor eats 7 pixels off the edge to make a shadow
    • when we try to draw our own title bar, dwm gives up and says "you don't want X? I'll take away Y, Z, A, B and C as well. Enjoy."
    • there's some weird magic hit testing that we cannot replicate for the resize handles "off the edge of the window"
  • we need to cut a hole in the Xaml island to allow titlebar and drag input to come back through (yes, this is silly)
  • we are driving dependencies on our internal partners to make this less of a heckscape.

I spent almost my entire sunday trying to answer all of the questions that arise from this list. It's not due to ignorance that we're asking the questions that we are.

@DHowett-MSFT
Copy link
Contributor

DHowett-MSFT commented Jul 8, 2019

Incidentally, the work merged in #929 does bring back theme-sized drag handles, they just look like a black hole opened up and consumed the contents of your display. ;)

Under all of the theming slapped down by the desktop compositor is actually just the Windows Vista Aero Basic theme with all of its rounded corners and weird thick window borders.

image

@c0d3h4x0r
Copy link
Author

c0d3h4x0r commented Jul 8, 2019

We're a fairly simple UI with fairly simple requirements, but one of those requirements is "have a custom color in our title bar."

And from that "requirement" stems the plethora of other problems. Instead, revisit that requirement -- why is it a requirement? Why does this app think it needs to be "more special" than other Windows applications and thus inconsistent with them (and likely less compatible with other system-wide UI changes that may come in future major Windows revisions)? Can the problem that "requirement" is trying to solve be better solved instead via some other design choice that avoids all the myriad technical issues created by the current approach?

@DHowett-MSFT
Copy link
Contributor

This might be better with recent dev builds. We're giving DWM a lot more control over the window frame than we were prior.

@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jul 20, 2019
@c0d3h4x0r
Copy link
Author

That sounds like a wise approach to me. 👍

@0xE1
Copy link

0xE1 commented Jul 27, 2019

Border width issue becomes worse the higher resolution is used, at 4K (even with 200% dpi scaling) you still have 2 pixel area to place mouse over to catch the resizer.

@ExE-Boss
Copy link

@c0d3h4x0r

We're a fairly simple UI with fairly simple requirements, but one of those requirements is "have a custom color in our title bar."

And from that "requirement" stems the plethora of other problems. Instead, revisit that requirement -- why is it a requirement? Why does this app think it needs to be "more special" than other Windows applications and thus inconsistent with them (and likely less compatible with other system-wide UI changes that may come in future major Windows revisions)? Can the problem that "requirement" is trying to solve be better solved instead via some other design choice that avoids all the myriad technical issues created by the current approach?

Probably because the Windows Console is a hot mess of legacy spaghetti code (it used to be a whole lot worse before 2014) and the Windows Terminal has to interface with it, so it needs to be a Win32 executable, as UWP apps are sandboxed, and therefore don’t have access to system internals (e.g. the Windows Console).

@zadjii-msft
Copy link
Member

(We think that this is going to be closely related to a bucket of things, #3136 #1859 #3064 #1307)

@ghost ghost added the In-PR This issue has a related PR label Nov 4, 2019
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Nov 4, 2019
DHowett-MSFT pushed a commit that referenced this issue Nov 4, 2019
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.

* Fix DwmExtendFrameIntoClientArea values
* Fix WM_NCHITTEST handling
* Position the XAML island window correctly
* Fix weird colors in drag bar and hide old title bar buttons
* Fix the window's position when maximized
* Add support for dark theme on the frame
* DRY shared code between conhost and new terminal
* Fix drag bar and remove dead code
* Remove dead code and use cached DPI
* Refactor code
* Remove impossible TODO
* Use system metrics instead of hardcoding resize border height
* Use theme from app settings instead of system theme. Improve comments. Remove unused DWM frame on maximize.
* Fix initial position DPI handling bug and apply review changes
* Fix thick borders with DPI > 96

Closes #3064.
Closes #1307.
Closes #3136.
Closes #1897.
Closes #3222.
Closes #1859.
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Nov 4, 2019
@ghost
Copy link

ghost commented Nov 26, 2019

🎉This issue was addressed in #3394, which has now been successfully released as Windows Terminal Preview v0.7.3291.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface 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. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants