terminal icon indicating copy to clipboard operation
terminal copied to clipboard

White borders are back in 1.17

Open zadjii-msft opened this issue 2 years ago • 2 comments

This problem is back in v1.17, I guess due to the recent changes around theming. The grey border is there to start with until you switch to another window and back (presumably it's part of the dark theme and doesn't get repainted until switching). Once you switch back it is white and will stay so even after closing/re-opening.

Steps to reproduce:

  • Delete settings.json and state.json
  • Open Terminal and switch to light theme
  • Border is grey
  • Alt tab to another window then back again
  • Border is now forever white

Originally posted by @harristom in https://github.com/microsoft/terminal/issues/6620#issuecomment-1406570765

zadjii-msft avatar Jan 27 '23 16:01 zadjii-msft

1000% this is due to one of the last commits in #14536.

@harristom Which OS version are you on? Just to make sure I fix it for the right platform(s)

zadjii-msft avatar Jan 27 '23 16:01 zadjii-msft

Windows 10

harristom avatar Jan 27 '23 16:01 harristom

Wait hold on, this is by design now? more or less. To try and tease this apart, I think there's two parts to this thread:

  • Part the first: The main issue here seems that when you switch to terminal light theme, the borders are white. Which is by-design now. We've got all these new toggles for different theme properties, we should probably actually respect the user's selected theme for the window. This is changed in 1.17, but seems like it changed to be more correct.
  • Part the second: "When I switch the terminal theme on Win10, the window border doesn't update till the window is unfocused/refocused", and I'm guessing that's a NCPAINT issue - we previously noted that the SetWindowAttribute thing in TerminalThemeHelpers needed to be before a NCPAINT. I'm betting that on win11, where that API is officially supported, they actually do force the NCPAINT when the API is called. On win 10, not so much.

Does all that make sense/?

zadjii-msft avatar Apr 04 '23 19:04 zadjii-msft

Is it definitely by design? As mentioned in the original report and the PR fixing it the white border makes the Terminal look out of place compared to other apps which all use a grey border. Obviously it's only a very minor issue but it makes the app look out of place and hard to tell whether it has focus (since for other apps a darker border means focussed and a lighter one means unfocussed whereas Terminal is doing the opposite).

harristom avatar Apr 07 '23 14:04 harristom

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

Sorry, the bot went haywire. As usual. I think this is worth talking about :)

DHowett avatar Apr 21 '23 21:04 DHowett

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

Hey just found this camping in the backlog. This has kinda turned into a complex space now.

  • Before 1.17, we'd always use the dark theme border for the Terminal window, even in light mode.
    • (personally, I agree that was preferable, but that's my opinion)
  • Now in 1.17, we'll use the border color that matches the window.applicationTheme. In light mode, that means a white border.
  • BUT NOW IN 1.18: we added a pair of window.frame and window.unfocusedFrame theme properties in #15441. This will let a user override the frame color and use whatever.
    • UNFORTUNATELY, that API doesn't work on Windows 10, so this isn't necessarily immediately helpful for you, but it does work

I dunno how I feel about reverting the "the window frame follows the OS theme" bit (back to "always use the dark border"). I feel like this is more correct now, even if UWPs and Win32 apps have been using different borders for years.

zadjii-msft avatar Sep 19 '23 14:09 zadjii-msft

discussion notes:

  • Sure, we should revert this back. So that null for window.border will always use the Dark theme border
  • While we're here, if people do use window.border on Windows 10, we should try to compute the lightness and use that to figure out what theme border to use (since the real RGB color API won't work there)

zadjii-msft avatar Oct 02 '23 20:10 zadjii-msft